Make Mtp FFS allocations per session rather than per file
Workloads that send a large number of small files could
repeatedly allocate and free the buffer, causing fragmentation
and eventually running out of allocable memory. Instead
have the allocation be once per MTP session, and retry
with smaller buffers if it fails initially.
Bug: 34741015
Bug: 34822471
Test: Transfer files via MTP
Change-Id: I775376076d3a0c26765b211100830ea0c08450ef
(cherry picked from commit cc9d0fdd302cf997607055e0a8b5559509cba766)
diff --git a/media/mtp/MtpFfsHandle.cpp b/media/mtp/MtpFfsHandle.cpp
index 9f0f8bf..6df2065 100644
--- a/media/mtp/MtpFfsHandle.cpp
+++ b/media/mtp/MtpFfsHandle.cpp
@@ -35,6 +35,7 @@
#include "AsyncIO.h"
#include "MtpFfsHandle.h"
+#include "mtp.h"
#define cpu_to_le16(x) htole16(x)
#define cpu_to_le32(x) htole32(x)
@@ -58,8 +59,8 @@
// To get good performance, override these with
// higher values per device using the properties
// sys.usb.ffs.max_read and sys.usb.ffs.max_write
-constexpr int USB_FFS_MAX_WRITE = 32768;
-constexpr int USB_FFS_MAX_READ = 32768;
+constexpr int USB_FFS_MAX_WRITE = MTP_BUFFER_SIZE;
+constexpr int USB_FFS_MAX_READ = MTP_BUFFER_SIZE;
constexpr unsigned int MAX_MTP_FILE_SIZE = 0xFFFFFFFF;
@@ -344,28 +345,9 @@
if (mBulkIn > -1 || mBulkOut > -1 || mIntr > -1)
LOG(WARNING) << "Endpoints were not closed before configure!";
- mBulkIn.reset(TEMP_FAILURE_RETRY(open(FFS_MTP_EP_IN, O_RDWR)));
- if (mBulkIn < 0) {
- PLOG(ERROR) << FFS_MTP_EP_IN << ": cannot open bulk in ep";
- goto err;
- }
-
- mBulkOut.reset(TEMP_FAILURE_RETRY(open(FFS_MTP_EP_OUT, O_RDWR)));
- if (mBulkOut < 0) {
- PLOG(ERROR) << FFS_MTP_EP_OUT << ": cannot open bulk out ep";
- goto err;
- }
-
- mIntr.reset(TEMP_FAILURE_RETRY(open(FFS_MTP_EP_INTR, O_RDWR)));
- if (mIntr < 0) {
- PLOG(ERROR) << FFS_MTP_EP0 << ": cannot open intr ep";
- goto err;
- }
-
return true;
err:
- closeEndpoints();
closeConfig();
return false;
}
@@ -447,7 +429,49 @@
int MtpFfsHandle::start() {
mLock.lock();
- return 0;
+
+ mBulkIn.reset(TEMP_FAILURE_RETRY(open(FFS_MTP_EP_IN, O_RDWR)));
+ if (mBulkIn < 0) {
+ PLOG(ERROR) << FFS_MTP_EP_IN << ": cannot open bulk in ep";
+ return -1;
+ }
+
+ mBulkOut.reset(TEMP_FAILURE_RETRY(open(FFS_MTP_EP_OUT, O_RDWR)));
+ if (mBulkOut < 0) {
+ PLOG(ERROR) << FFS_MTP_EP_OUT << ": cannot open bulk out ep";
+ return -1;
+ }
+
+ mIntr.reset(TEMP_FAILURE_RETRY(open(FFS_MTP_EP_INTR, O_RDWR)));
+ if (mIntr < 0) {
+ PLOG(ERROR) << FFS_MTP_EP0 << ": cannot open intr ep";
+ return -1;
+ }
+
+ mBuffer1.resize(MAX_FILE_CHUNK_SIZE);
+ mBuffer2.resize(MAX_FILE_CHUNK_SIZE);
+ posix_madvise(mBuffer1.data(), MAX_FILE_CHUNK_SIZE,
+ POSIX_MADV_SEQUENTIAL | POSIX_MADV_WILLNEED);
+ posix_madvise(mBuffer2.data(), MAX_FILE_CHUNK_SIZE,
+ POSIX_MADV_SEQUENTIAL | POSIX_MADV_WILLNEED);
+
+ // Get device specific r/w size
+ mMaxWrite = android::base::GetIntProperty("sys.usb.ffs.max_write", USB_FFS_MAX_WRITE);
+ mMaxRead = android::base::GetIntProperty("sys.usb.ffs.max_read", USB_FFS_MAX_READ);
+
+ while (mMaxWrite > USB_FFS_MAX_WRITE && mMaxRead > USB_FFS_MAX_READ) {
+ // If larger contiguous chunks of memory aren't available, attempt to try
+ // smaller allocations.
+ if (ioctl(mBulkIn, FUNCTIONFS_ENDPOINT_ALLOC, static_cast<__u32>(mMaxWrite)) ||
+ ioctl(mBulkOut, FUNCTIONFS_ENDPOINT_ALLOC, static_cast<__u32>(mMaxRead))) {
+ mMaxWrite /= 2;
+ mMaxRead /=2;
+ } else {
+ return 0;
+ }
+ }
+ PLOG(ERROR) << "Functionfs could not allocate any memory!";
+ return -1;
}
int MtpFfsHandle::configure(bool usePtp) {
@@ -465,13 +489,6 @@
return -1;
}
- // Get device specific r/w size
- mMaxWrite = android::base::GetIntProperty("sys.usb.ffs.max_write", 0);
- mMaxRead = android::base::GetIntProperty("sys.usb.ffs.max_read", 0);
- if (!mMaxWrite)
- mMaxWrite = USB_FFS_MAX_WRITE;
- if (!mMaxRead)
- mMaxRead = USB_FFS_MAX_READ;
return 0;
}
@@ -480,26 +497,6 @@
mLock.unlock();
}
-class ScopedEndpointBufferAlloc {
-private:
- const int mFd;
- const unsigned int mAllocSize;
-public:
- ScopedEndpointBufferAlloc(int fd, unsigned alloc_size) :
- mFd(fd),
- mAllocSize(alloc_size) {
- if (ioctl(mFd, FUNCTIONFS_ENDPOINT_ALLOC, static_cast<__u32>(mAllocSize)))
- PLOG(ERROR) << "FFS endpoint alloc failed!";
- }
-
- ~ScopedEndpointBufferAlloc() {
- if (ioctl(mFd, FUNCTIONFS_ENDPOINT_ALLOC, static_cast<__u32>(0)))
- PLOG(ERROR) << "FFS endpoint alloc reset failed!";
- }
-
- DISALLOW_COPY_AND_ASSIGN(ScopedEndpointBufferAlloc);
-};
-
/* Read from USB and write to a local file. */
int MtpFfsHandle::receiveFile(mtp_file_range mfr) {
// When receiving files, the incoming length is given in 32 bits.
@@ -507,15 +504,8 @@
uint32_t file_length = mfr.length;
uint64_t offset = lseek(mfr.fd, 0, SEEK_CUR);
- int buf1_len = std::min(static_cast<uint32_t>(MAX_FILE_CHUNK_SIZE), file_length);
- std::vector<char> buf1(buf1_len);
- char* data = buf1.data();
-
- // If necessary, allocate a second buffer for background r/w
- int buf2_len = std::min(static_cast<uint32_t>(MAX_FILE_CHUNK_SIZE),
- file_length - MAX_FILE_CHUNK_SIZE);
- std::vector<char> buf2(std::max(0, buf2_len));
- char *data2 = buf2.data();
+ char *data = mBuffer1.data();
+ char *data2 = mBuffer2.data();
struct aiocb aio;
aio.aio_fildes = mfr.fd;
@@ -527,9 +517,6 @@
bool write = false;
posix_fadvise(mfr.fd, 0, 0, POSIX_FADV_SEQUENTIAL | POSIX_FADV_NOREUSE);
- posix_madvise(data, buf1_len, POSIX_MADV_SEQUENTIAL | POSIX_MADV_WILLNEED);
- posix_madvise(data2, buf2_len, POSIX_MADV_SEQUENTIAL | POSIX_MADV_WILLNEED);
- ScopedEndpointBufferAlloc scoped_alloc(mBulkOut, mMaxRead);
// Break down the file into pieces that fit in buffers
while (file_length > 0 || write) {
@@ -608,20 +595,11 @@
}
int init_read_len = packet_size - sizeof(mtp_data_header);
- int buf1_len = std::max(static_cast<uint64_t>(packet_size), std::min(
- static_cast<uint64_t>(MAX_FILE_CHUNK_SIZE), file_length - init_read_len));
- std::vector<char> buf1(buf1_len);
- char *data = buf1.data();
- // If necessary, allocate a second buffer for background r/w
- int buf2_len = std::min(static_cast<uint64_t>(MAX_FILE_CHUNK_SIZE),
- file_length - MAX_FILE_CHUNK_SIZE - init_read_len);
- std::vector<char> buf2(std::max(0, buf2_len));
- char *data2 = buf2.data();
+ char *data = mBuffer1.data();
+ char *data2 = mBuffer2.data();
posix_fadvise(mfr.fd, 0, 0, POSIX_FADV_SEQUENTIAL | POSIX_FADV_NOREUSE);
- posix_madvise(data, buf1_len, POSIX_MADV_SEQUENTIAL | POSIX_MADV_WILLNEED);
- posix_madvise(data2, buf2_len, POSIX_MADV_SEQUENTIAL | POSIX_MADV_WILLNEED);
struct aiocb aio;
aio.aio_fildes = mfr.fd;
@@ -647,8 +625,6 @@
if (writeHandle(mBulkIn, data, packet_size) == -1) return -1;
if (file_length == 0) return 0;
- ScopedEndpointBufferAlloc scoped_alloc(mBulkIn, mMaxWrite);
-
// Break down the file into pieces that fit in buffers
while(file_length > 0) {
if (read) {
@@ -672,7 +648,7 @@
}
if (file_length > 0) {
- length = std::min((uint64_t) MAX_FILE_CHUNK_SIZE, file_length);
+ length = std::min(static_cast<uint64_t>(MAX_FILE_CHUNK_SIZE), file_length);
// Queue up another read
aio.aio_buf = data;
aio.aio_offset = offset;
diff --git a/media/mtp/MtpFfsHandle.h b/media/mtp/MtpFfsHandle.h
index 9cd4dcf..44ff0f3 100644
--- a/media/mtp/MtpFfsHandle.h
+++ b/media/mtp/MtpFfsHandle.h
@@ -48,6 +48,9 @@
int mMaxWrite;
int mMaxRead;
+ std::vector<char> mBuffer1;
+ std::vector<char> mBuffer2;
+
public:
int read(void *data, int len);
int write(const void *data, int len);
diff --git a/media/mtp/MtpServer.cpp b/media/mtp/MtpServer.cpp
index 8d56c16..753d833 100644
--- a/media/mtp/MtpServer.cpp
+++ b/media/mtp/MtpServer.cpp
@@ -179,6 +179,7 @@
if (sHandle->start()) {
ALOGE("Failed to start usb driver!");
+ sHandle->close();
return;
}
