From 7961e3557dfc32353f78231fd6599c45899f9943 Mon Sep 17 00:00:00 2001 From: Edgar Arriaga Date: Mon, 5 Apr 2021 10:57:37 -0700 Subject: [PATCH] cleanups and fixes for process_madvise compaction some fix suggestions that came up on ag/13665789 and some other cleanups 1. Use unique_fd instead of raw int to keep pidfd 2. Return the total compacted bytes on success for compactProcess 3. Fix for error potentially returning a random value 4. Fix truncation that could happen when calling madvise 5. Fail fast after encountering an error instead of silently advancing to other VMAs when compacting. Bug: 162993824 Test: Manual Signed-off-by: Edgar Arriaga Change-Id: Ide644f66cf0ebdea570dcb365d6a2400ffb18f4e --- ...m_android_server_am_CachedAppOptimizer.cpp | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp index a4a74af8a1d9c..e319e3febc212 100644 --- a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp +++ b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp @@ -18,8 +18,11 @@ //#define LOG_NDEBUG 0 #include +#include #include +#include #include +#include #include #include #include @@ -27,9 +30,11 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -37,29 +42,16 @@ #include -#include -#include -#include -#include -#include - using android::base::StringPrintf; using android::base::WriteStringToFile; using android::meminfo::ProcMemInfo; using namespace android::meminfo; -// This is temporarily hard-coded and should be removed once -// bionic/libc/kernel/uapi/asm-generic/unistd.h are updated with process_madvise syscall header -#ifndef __NR_process_madvise -#define __NR_process_madvise 440 -#define MADV_COLD 20 /* deactivate these pages */ -#define MADV_PAGEOUT 21 -#endif - #define COMPACT_ACTION_FILE_FLAG 1 #define COMPACT_ACTION_ANON_FLAG 2 using VmaToAdviseFunc = std::function; +using android::base::unique_fd; #define SYNC_RECEIVED_WHILE_FROZEN (1) #define ASYNC_RECEIVED_WHILE_FROZEN (2) @@ -73,24 +65,25 @@ static inline void compactProcessProcfs(int pid, const std::string& compactionTy WriteStringToFile(compactionType, reclaim_path); } -static int compactMemory(const std::vector& vmas, int pid, int madviseType) { +// Compacts a set of VMAs for pid using an madviseType accepted by process_madvise syscall +// On success returns the total bytes that where compacted. On failure it returns +// a negative error code from the standard linux error codes. +static int64_t compactMemory(const std::vector& vmas, int pid, int madviseType) { // UIO_MAXIOV is currently a small value and we might have more addresses // we do multiple syscalls if we exceed its maximum static struct iovec vmasToKernel[UIO_MAXIOV]; - int err = 0; - if (vmas.empty()) { - return err; + return 0; } - int pidfd = syscall(__NR_pidfd_open, pid, 0); - err = -errno; + unique_fd pidfd(pidfd_open(pid, 0)); if (pidfd < 0) { // Skip compaction if failed to open pidfd with any error - return err; + return -errno; } + int64_t totalBytesCompacted = 0; for (int iBase = 0; iBase < vmas.size(); iBase += UIO_MAXIOV) { int totalVmasToKernel = std::min(UIO_MAXIOV, (int)(vmas.size() - iBase)); for (int iVec = 0, iVma = iBase; iVec < totalVmasToKernel; ++iVec, ++iVma) { @@ -98,17 +91,16 @@ static int compactMemory(const std::vector& vmas, int pid, int madviseType) vmasToKernel[iVec].iov_len = vmas[iVma].end - vmas[iVma].start; } - process_madvise(pidfd, vmasToKernel, totalVmasToKernel, madviseType, 0); - err = -errno; - if (CC_UNLIKELY(err == -ENOSYS)) { - // Syscall does not exist, skip trying more calls process_madvise - break; + auto bytesCompacted = + process_madvise(pidfd, vmasToKernel, totalVmasToKernel, madviseType, 0); + if (CC_UNLIKELY(bytesCompacted == -1)) { + return -errno; } + + totalBytesCompacted += bytesCompacted; } - close(pidfd); - - return err; + return totalBytesCompacted; } static int getFilePageAdvice(const Vma& vma) { @@ -132,7 +124,7 @@ static int getAnyPageAdvice(const Vma& vma) { // Perform a full process compaction using process_madvise syscall // reading all filtering VMAs and filtering pages as specified by pageFilter -static int compactProcess(int pid, VmaToAdviseFunc vmaToAdviseFunc) { +static int64_t compactProcess(int pid, VmaToAdviseFunc vmaToAdviseFunc) { ProcMemInfo meminfo(pid); std::vector pageoutVmas, coldVmas; auto vmaCollectorCb = [&coldVmas,&pageoutVmas,&vmaToAdviseFunc](const Vma& vma) { @@ -148,11 +140,19 @@ static int compactProcess(int pid, VmaToAdviseFunc vmaToAdviseFunc) { }; meminfo.ForEachVmaFromMaps(vmaCollectorCb); - int err = compactMemory(pageoutVmas, pid, MADV_PAGEOUT); - if (!err) { - err = compactMemory(coldVmas, pid, MADV_COLD); + int64_t pageoutBytes = compactMemory(pageoutVmas, pid, MADV_PAGEOUT); + if (pageoutBytes < 0) { + // Error, just forward it. + return pageoutBytes; } - return err; + + int64_t coldBytes = compactMemory(coldVmas, pid, MADV_COLD); + if (coldBytes < 0) { + // Error, just forward it. + return coldBytes; + } + + return pageoutBytes + coldBytes; } // Compact process using process_madvise syscall or fallback to procfs in