From c673f1f06eaea82e34a502a59376597ddb7a0f32 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Thu, 7 Oct 2021 18:23:35 -0700 Subject: libbinder: do not always compute open ashmem size There is only one caller of this, but it adds system calls to every pass of every FD using binder. Bug: 195752513 Test: binderHostTest Change-Id: Ieb2cfbd1f17055af2cbb0747ee4af0df42e04551 --- libs/binder/Parcel.cpp | 59 +++++++++++++++++-------------------- libs/binder/include/binder/Parcel.h | 15 ++++++---- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 07555f6eee..41ca60564a 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -98,9 +98,8 @@ enum { BLOB_ASHMEM_MUTABLE = 2, }; -static void acquire_object(const sp& proc, - const flat_binder_object& obj, const void* who, size_t* outAshmemSize) -{ +static void acquire_object(const sp& proc, const flat_binder_object& obj, + const void* who) { switch (obj.hdr.type) { case BINDER_TYPE_BINDER: if (obj.binder) { @@ -117,13 +116,6 @@ static void acquire_object(const sp& proc, return; } case BINDER_TYPE_FD: { - if ((obj.cookie != 0) && (outAshmemSize != nullptr) && ashmem_valid(obj.handle)) { - // If we own an ashmem fd, keep track of how much memory it refers to. - int size = ashmem_get_size_region(obj.handle); - if (size > 0) { - *outAshmemSize += size; - } - } return; } } @@ -131,9 +123,8 @@ static void acquire_object(const sp& proc, ALOGD("Invalid object type 0x%08x", obj.hdr.type); } -static void release_object(const sp& proc, - const flat_binder_object& obj, const void* who, size_t* outAshmemSize) -{ +static void release_object(const sp& proc, const flat_binder_object& obj, + const void* who) { switch (obj.hdr.type) { case BINDER_TYPE_BINDER: if (obj.binder) { @@ -151,16 +142,6 @@ static void release_object(const sp& proc, } case BINDER_TYPE_FD: { if (obj.cookie != 0) { // owned - if ((outAshmemSize != nullptr) && ashmem_valid(obj.handle)) { - int size = ashmem_get_size_region(obj.handle); - if (size > 0) { - // ashmem size might have changed since last time it was accounted for, e.g. - // in acquire_object(). Value of *outAshmemSize is not critical since we are - // releasing the object anyway. Check for integer overflow condition. - *outAshmemSize -= std::min(*outAshmemSize, static_cast(size)); - } - } - close(obj.handle); } return; @@ -512,7 +493,7 @@ status_t Parcel::appendFrom(const Parcel *parcel, size_t offset, size_t len) flat_binder_object* flat = reinterpret_cast(mData + off); - acquire_object(proc, *flat, this, &mOpenAshmemSize); + acquire_object(proc, *flat, this); if (flat->hdr.type == BINDER_TYPE_FD) { // If this is a file descriptor, we need to dup it so the @@ -1335,7 +1316,7 @@ restart_write: // Need to write meta-data? if (nullMetaData || val.binder != 0) { mObjects[mObjectsSize] = mDataPos; - acquire_object(ProcessState::self(), val, this, &mOpenAshmemSize); + acquire_object(ProcessState::self(), val, this); mObjectsSize++; } @@ -2225,7 +2206,7 @@ void Parcel::releaseObjects() i--; const flat_binder_object* flat = reinterpret_cast(data+objects[i]); - release_object(proc, *flat, this, &mOpenAshmemSize); + release_object(proc, *flat, this); } } @@ -2242,7 +2223,7 @@ void Parcel::acquireObjects() i--; const flat_binder_object* flat = reinterpret_cast(data+objects[i]); - acquire_object(proc, *flat, this, &mOpenAshmemSize); + acquire_object(proc, *flat, this); } } @@ -2447,7 +2428,7 @@ status_t Parcel::continueWrite(size_t desired) // will need to rescan because we may have lopped off the only FDs mFdsKnown = false; } - release_object(proc, *flat, this, &mOpenAshmemSize); + release_object(proc, *flat, this); } if (objectsSize == 0) { @@ -2540,7 +2521,6 @@ void Parcel::initState() mAllowFds = true; mDeallocZero = false; mOwner = nullptr; - mOpenAshmemSize = 0; mWorkSourceRequestHeaderPosition = 0; mRequestHeaderPresent = false; @@ -2576,13 +2556,28 @@ size_t Parcel::getBlobAshmemSize() const { // This used to return the size of all blobs that were written to ashmem, now we're returning // the ashmem currently referenced by this Parcel, which should be equivalent. - // TODO: Remove method once ABI can be changed. - return mOpenAshmemSize; + // TODO(b/202029388): Remove method once ABI can be changed. + return getOpenAshmemSize(); } size_t Parcel::getOpenAshmemSize() const { - return mOpenAshmemSize; + size_t openAshmemSize = 0; + for (size_t i = 0; i < mObjectsSize; i++) { + const flat_binder_object* flat = + reinterpret_cast(mData + mObjects[i]); + + // cookie is compared against zero for historical reasons + // > obj.cookie = takeOwnership ? 1 : 0; + if (flat->hdr.type == BINDER_TYPE_FD && flat->cookie != 0 && ashmem_valid(flat->handle)) { + int size = ashmem_get_size_region(flat->handle); + if (__builtin_add_overflow(openAshmemSize, size, &openAshmemSize)) { + ALOGE("Overflow when computing ashmem size."); + return SIZE_MAX; + } + } + } + return openAshmemSize; } // --- Parcel::Blob --- diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h index fd8ac622a5..3e172d3474 100644 --- a/libs/binder/include/binder/Parcel.h +++ b/libs/binder/include/binder/Parcel.h @@ -1141,6 +1141,7 @@ private: release_func mOwner; sp mSession; + size_t mReserved; class Blob { public: @@ -1225,13 +1226,17 @@ public: inline void* data() { return mData; } }; -private: - size_t mOpenAshmemSize; + /** + * Returns the total amount of ashmem memory owned by this object. + * + * Note: for historical reasons, this does not include ashmem memory which + * is referenced by this Parcel, but which this parcel doesn't own (e.g. + * writeFileDescriptor is called without 'takeOwnership' true). + */ + size_t getOpenAshmemSize() const; -public: - // TODO: Remove once ABI can be changed. + // TODO(b/202029388): Remove 'getBlobAshmemSize' once ABI can be changed. size_t getBlobAshmemSize() const; - size_t getOpenAshmemSize() const; }; // --------------------------------------------------------------------------- -- cgit v1.2.3-59-g8ed1b