Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 99a096a9 authored by Girish Shetty's avatar Girish Shetty Committed by Gerrit Code Review
Browse files

Merge changes from topic "rm_changes" into main 

* changes:
  resourcemanager: protect resources from concurrent access
  resourcemanager: use std::vector for ResourceList
parents b7e34746 e8305279

services/mediaresourcemanager/ResourceManagerService.cpp

+25 −45
Original line number Diff line number Diff line
@@ -74,10 +74,7 @@ void ResourceManagerService::getResourceDump(std::string& resourceLog) const { 


            const ResourceList& resources = info.resources;

            resourceLog.append("        Resources:\n");

            for (auto it = resources.begin(); it != resources.end(); it++) {

                snprintf(buffer, SIZE, "          %s\n", toString(it->second).c_str());

                resourceLog.append(buffer);

            }

            resourceLog.append(resources.toString());

        }

    }
@@ -315,31 +312,21 @@ Status ResourceManagerService::addResource(const ClientInfoParcel& clientInfo, 


    for (size_t i = 0; i < resources.size(); ++i) {

        const auto &res = resources[i];

        const auto resType = std::tuple(res.type, res.subType, res.id);



        if (res.value < 0 && res.type != MediaResource::Type::kDrmSession) {

            ALOGW("Ignoring request to remove negative value of non-drm resource");

            continue;

        }

        if (info.resources.find(resType) == info.resources.end()) {

            if (res.value <= 0) {

                // We can't init a new entry with negative value, although it's allowed

                // to merge in negative values after the initial add.

                ALOGW("Ignoring request to add new resource entry with value <= 0");

        bool isNewEntry = false;

        if (!info.resources.add(res, &isNewEntry)) {

            continue;

        }

        if (isNewEntry) {

            onFirstAdded(res, info.uid);

            info.resources[resType] = res;

        } else {

            mergeResources(info.resources[resType], res);

        }



        // Add it to the list of added resources for observers.

        auto it = resourceAdded.find(resType);

        if (it == resourceAdded.end()) {

            resourceAdded[resType] = res;

        } else {

            mergeResources(it->second, res);

        }

        resourceAdded.add(res);

    }

    if (info.deathNotifier == nullptr && client != nullptr) {

        info.deathNotifier = DeathNotifier::Create(
@@ -386,31 +373,22 @@ Status ResourceManagerService::removeResource(const ClientInfoParcel& clientInfo 
    ResourceList resourceRemoved;

    for (size_t i = 0; i < resources.size(); ++i) {

        const auto &res = resources[i];

        const auto resType = std::tuple(res.type, res.subType, res.id);



        if (res.value < 0) {

            ALOGW("Ignoring request to remove negative value of resource");

            continue;

        }

        // ignore if we don't have it

        if (info.resources.find(resType) != info.resources.end()) {

            MediaResourceParcel &resource = info.resources[resType];



        long removedEntryValue = -1;

        if (info.resources.remove(res, &removedEntryValue)) {

            MediaResourceParcel actualRemoved = res;

            if (resource.value > res.value) {

                resource.value -= res.value;

            } else {

            if (removedEntryValue != -1) {

                onLastRemoved(res, info.uid);

                actualRemoved.value = resource.value;

                info.resources.erase(resType);

                actualRemoved.value = removedEntryValue;

            }



            // Add it to the list of removed resources for observers.

            auto it = resourceRemoved.find(resType);

            if (it == resourceRemoved.end()) {

                resourceRemoved[resType] = actualRemoved;

            } else {

                mergeResources(it->second, actualRemoved);

            }

            resourceRemoved.add(actualRemoved);

        }

    }

    if (mObserverService != nullptr && !resourceRemoved.empty()) {
@@ -453,8 +431,8 @@ Status ResourceManagerService::removeResource(const ClientInfoParcel& clientInfo 
    }



    const ResourceInfo& info = foundClient->second;

    for (auto it = info.resources.begin(); it != info.resources.end(); it++) {

        onLastRemoved(it->second, info.uid);

    for (const MediaResourceParcel& res : info.resources.getResources()) {

        onLastRemoved(res, info.uid);

    }



    // Since this client has been removed, update the metrics collector.
@@ -642,7 +620,7 @@ void ResourceManagerService::pushReclaimAtom(const ClientInfoParcel& clientInfo, 
    mResourceManagerMetrics->pushReclaimAtom(clientInfo, priorities, targetClients, reclaimed);

}



std::shared_ptr<IResourceManagerClient> ResourceManagerService::getClient(

std::shared_ptr<IResourceManagerClient> ResourceManagerService::getClient_l(

        int pid, const int64_t& clientId) const {

    std::map<int, ResourceInfos>::const_iterator found = mMap.find(pid);

    if (found == mMap.end()) {
@@ -660,7 +638,7 @@ std::shared_ptr<IResourceManagerClient> ResourceManagerService::getClient( 
    return foundClient->second.client;

}



bool ResourceManagerService::removeClient(int pid, const int64_t& clientId) {

bool ResourceManagerService::removeClient_l(int pid, const int64_t& clientId) {

    std::map<int, ResourceInfos>::iterator found = mMap.find(pid);

    if (found == mMap.end()) {

        ALOGV("%s: didn't find pid %d for clientId %lld", __func__, pid, (long long) clientId);
@@ -687,8 +665,11 @@ bool ResourceManagerService::reclaimUnconditionallyFrom( 
    int64_t failedClientId = -1;

    int32_t failedClientPid = -1;

    for (const ClientInfo& targetClient : targetClients) {

        std::shared_ptr<IResourceManagerClient> client = getClient(

            targetClient.mPid, targetClient.mClientId);

        std::shared_ptr<IResourceManagerClient> client = nullptr;

        {

            std::scoped_lock lock{mLock};

            client = getClient_l(targetClient.mPid, targetClient.mClientId);

        }

        if (client == nullptr) {

            // skip already released clients.

            continue;
@@ -710,7 +691,7 @@ bool ResourceManagerService::reclaimUnconditionallyFrom( 


    {

        std::scoped_lock lock{mLock};

        bool found = removeClient(failedClientPid, failedClientId);

        bool found = removeClient_l(failedClientPid, failedClientId);

        if (found) {

            ALOGW("Failed to reclaim resources from client with pid %d", failedClientPid);

        } else {
@@ -1056,8 +1037,7 @@ bool ResourceManagerService::getBiggestClient_l(int pid, MediaResource::Type typ 
        if (pendingRemovalOnly && !info.pendingRemoval) {

            continue;

        }

        for (auto it = resources.begin(); it != resources.end(); it++) {

            const MediaResourceParcel &resource = it->second;

        for (const MediaResourceParcel& resource : resources.getResources()) {

            if (hasResourceType(type, subType, resource)) {

                if (resource.value > largestValue) {

                    largestValue = resource.value;

services/mediaresourcemanager/ResourceManagerService.h

+2 −2
Original line number Diff line number Diff line
@@ -210,11 +210,11 @@ private: 
    virtual void removeProcessInfoOverride(int pid);



    // Get the client for given pid and the clientId from the map

    virtual std::shared_ptr<IResourceManagerClient> getClient(

    virtual std::shared_ptr<IResourceManagerClient> getClient_l(

        int pid, const int64_t& clientId) const;



    // Remove the client for given pid and the clientId from the map

    virtual bool removeClient(int pid, const int64_t& clientId);

    virtual bool removeClient_l(int pid, const int64_t& clientId);



    // Get all the resource status for dump

    virtual void getResourceDump(std::string& resourceLog) const;

services/mediaresourcemanager/ResourceManagerServiceNew.cpp

+2 −2
Original line number Diff line number Diff line
@@ -356,12 +356,12 @@ bool ResourceManagerServiceNew::getAllClients_l( 
    return true;

}



std::shared_ptr<IResourceManagerClient> ResourceManagerServiceNew::getClient(

std::shared_ptr<IResourceManagerClient> ResourceManagerServiceNew::getClient_l(

        int pid, const int64_t& clientId) const {

    return mResourceTracker->getClient(pid, clientId);

}



bool ResourceManagerServiceNew::removeClient(int pid, const int64_t& clientId) {

bool ResourceManagerServiceNew::removeClient_l(int pid, const int64_t& clientId) {

    return mResourceTracker->removeClient(pid, clientId);

}

services/mediaresourcemanager/ResourceManagerServiceNew.h

+2 −2
Original line number Diff line number Diff line
@@ -124,11 +124,11 @@ private: 
    bool getPriority_l(int pid, int* priority) const override;



    // Get the client for given pid and the clientId from the map

    std::shared_ptr<IResourceManagerClient> getClient(

    std::shared_ptr<IResourceManagerClient> getClient_l(

        int pid, const int64_t& clientId) const override;



    // Remove the client for given pid and the clientId from the map

    bool removeClient(int pid, const int64_t& clientId) override;

    bool removeClient_l(int pid, const int64_t& clientId) override;



    // Get all the resource status for dump

    void getResourceDump(std::string& resourceLog) const override;

services/mediaresourcemanager/ResourceManagerServiceUtils.cpp

+89 −2
Original line number Diff line number Diff line
@@ -27,6 +27,93 @@ 


namespace android {



bool ResourceList::add(const MediaResourceParcel& res, bool* isNewEntry) {

    // See if it's an existing entry, if so, merge it.

    for (MediaResourceParcel& item : mResourceList) {

        if (item.type == res.type && item.subType == res.subType && item.id == res.id) {

            // We already have an item. Merge them and return

            mergeResources(item, res);

            return true;

        }

    }



    // Since we have't found this resource yet, it is a new entry.

    // We can't init a new entry with negative value, although it's allowed

    // to merge in negative values after the initial add.

    if (res.value <= 0) {

        ALOGW("Ignoring request to add new resource entry with value <= 0");

        return false;

    }

    if (isNewEntry) {

        *isNewEntry = true;

    }

    mResourceList.push_back(res);

    return true;

}



void ResourceList::addOrUpdate(const MediaResourceParcel& res) {

    // See if it's an existing entry, just update the value.

    for (MediaResourceParcel& item : mResourceList) {

        if (item.type == res.type && item.subType == res.subType && item.id == res.id) {

            item.value = res.value;

            return;

        }

    }



    // Add the new entry.

    mResourceList.push_back(res);

}



bool ResourceList::remove(const MediaResourceParcel& res, long* removedEntryValue) {

    // Make sure we have an entry for this resource.

    for (std::vector<MediaResourceParcel>::iterator it = mResourceList.begin();

         it != mResourceList.end(); it++) {

        if (it->type == res.type && it->subType == res.subType && it->id == res.id) {

            if (it->value > res.value) {

                // Subtract the resource value by given value.

                it->value -= res.value;

            } else {

                // This entry will be removed.

                if (removedEntryValue) {

                    *removedEntryValue = it->value;

                }

                mResourceList.erase(it);

            }

            return true;

        }

    }



    // No such entry.

    return false;

}



std::string ResourceList::toString() const {

    std::string str;

    for (const ::aidl::android::media::MediaResourceParcel& res : mResourceList) {

        str.append(android::toString(res).c_str());

        str.append("\n");

    }



    return std::move(str);

}



bool ResourceList::operator==(const ResourceList& rhs) const {

    // Make sure the size is the same.

    if (mResourceList.size() != rhs.mResourceList.size()) {

        return false;

    }



    // Create a set from this object and check for the items from the rhs.

    std::set<::aidl::android::media::MediaResourceParcel> lhs(

            mResourceList.begin(), mResourceList.end());

    for (const ::aidl::android::media::MediaResourceParcel& res : rhs.mResourceList) {

        if (lhs.find(res) == lhs.end()) {

            return false;

        }

    }

    return true;

}



// Bunch of utility functions that looks for a specific Resource.

// Check whether a given resource (of type and subtype) is found in given resource parcel.

bool hasResourceType(MediaResource::Type type, MediaResource::SubType subType,
@@ -53,8 +140,8 @@ bool hasResourceType(MediaResource::Type type, MediaResource::SubType subType, 
// Check whether a given resource (of type and subtype) is found in given resource list.

bool hasResourceType(MediaResource::Type type, MediaResource::SubType subType,

                     const ResourceList& resources) {

    for (auto it = resources.begin(); it != resources.end(); it++) {

        if (hasResourceType(type, subType, it->second)) {

    for (const MediaResourceParcel& res : resources.getResources()) {

        if (hasResourceType(type, subType, res)) {

            return true;

        }

    }