cameraserver: Allow clients to specify lower oom scores for camera arbitration.
Bug: 187429135
Test: atest CameraEvictionTest.java
Change-Id: I7b54a399f87f57a428cc9da382bcc51ebf6adfa6
Signed-off-by: Jayant Chowdhary <jchowdhary@google.com>
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index a0448b4..329400d 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -945,7 +945,7 @@
if (!(ret = connectHelper<ICameraClient,Client>(
sp<ICameraClient>{nullptr}, id, cameraId,
internalPackageName, {}, uid, USE_CALLING_PID,
- API_1, /*shimUpdateOnly*/ true, /*out*/ tmp)
+ API_1, /*shimUpdateOnly*/ true, /*oomScoreOffset*/ 0, /*out*/ tmp)
).isOk()) {
ALOGE("%s: Error initializing shim metadata: %s", __FUNCTION__, ret.toString8().string());
}
@@ -1217,10 +1217,12 @@
}
void CameraService::finishConnectLocked(const sp<BasicClient>& client,
- const CameraService::DescriptorPtr& desc) {
+ const CameraService::DescriptorPtr& desc, int oomScoreOffset) {
// Make a descriptor for the incoming client
- auto clientDescriptor = CameraService::CameraClientManager::makeClientDescriptor(client, desc);
+ auto clientDescriptor =
+ CameraService::CameraClientManager::makeClientDescriptor(client, desc,
+ oomScoreOffset);
auto evicted = mActiveClientManager.addAndEvict(clientDescriptor);
logConnected(desc->getKey(), static_cast<int>(desc->getOwnerId()),
@@ -1250,6 +1252,7 @@
status_t CameraService::handleEvictionsLocked(const String8& cameraId, int clientPid,
apiLevel effectiveApiLevel, const sp<IBinder>& remoteCallback, const String8& packageName,
+ int oomScoreOffset,
/*out*/
sp<BasicClient>* client,
std::shared_ptr<resource_policy::ClientDescriptor<String8, sp<BasicClient>>>* partial) {
@@ -1301,7 +1304,8 @@
for (size_t i = 0; i < ownerPids.size() - 1; i++) {
pidToPriorityMap.emplace(ownerPids[i],
resource_policy::ClientPriority(priorityScores[i], states[i],
- /* isVendorClient won't get copied over*/ false));
+ /* isVendorClient won't get copied over*/ false,
+ /* oomScoreOffset won't get copied over*/ 0));
}
mActiveClientManager.updatePriorities(pidToPriorityMap);
@@ -1314,13 +1318,16 @@
return BAD_VALUE;
}
- // Make descriptor for incoming client
+ int32_t actualScore = priorityScores[priorityScores.size() - 1];
+ int32_t actualState = states[states.size() - 1];
+
+ // Make descriptor for incoming client. We store the oomScoreOffset
+ // since we might need it later on new handleEvictionsLocked and
+ // ProcessInfoService would not take that into account.
clientDescriptor = CameraClientManager::makeClientDescriptor(cameraId,
sp<BasicClient>{nullptr}, static_cast<int32_t>(state->getCost()),
- state->getConflicting(),
- priorityScores[priorityScores.size() - 1],
- clientPid,
- states[states.size() - 1]);
+ state->getConflicting(), actualScore, clientPid, actualState,
+ oomScoreOffset);
resource_policy::ClientPriority clientPriority = clientDescriptor->getPriority();
@@ -1463,7 +1470,7 @@
sp<Client> client = nullptr;
ret = connectHelper<ICameraClient,Client>(cameraClient, id, api1CameraId,
clientPackageName, {}, clientUid, clientPid, API_1,
- /*shimUpdateOnly*/ false, /*out*/client);
+ /*shimUpdateOnly*/ false, /*oomScoreOffset*/ 0, /*out*/client);
if(!ret.isOk()) {
logRejected(id, CameraThreadState::getCallingPid(), String8(clientPackageName),
@@ -1539,7 +1546,7 @@
const String16& cameraId,
const String16& clientPackageName,
const std::optional<String16>& clientFeatureId,
- int clientUid,
+ int clientUid, int oomScoreOffset,
/*out*/
sp<hardware::camera2::ICameraDeviceUser>* device) {
@@ -1548,19 +1555,41 @@
String8 id = String8(cameraId);
sp<CameraDeviceClient> client = nullptr;
String16 clientPackageNameAdj = clientPackageName;
+ int callingPid = CameraThreadState::getCallingPid();
if (getCurrentServingCall() == BinderCallType::HWBINDER) {
std::string vendorClient =
StringPrintf("vendor.client.pid<%d>", CameraThreadState::getCallingPid());
clientPackageNameAdj = String16(vendorClient.c_str());
}
+
+ if (oomScoreOffset < 0) {
+ String8 msg =
+ String8::format("Cannot increase the priority of a client %s pid %d for "
+ "camera id %s", String8(clientPackageNameAdj).string(), callingPid,
+ id.string());
+ ALOGE("%s: %s", __FUNCTION__, msg.string());
+ return STATUS_ERROR(ERROR_ILLEGAL_ARGUMENT, msg.string());
+ }
+
+ // enforce system camera permissions
+ if (oomScoreOffset > 0 &&
+ !hasPermissionsForSystemCamera(callingPid, CameraThreadState::getCallingUid())) {
+ String8 msg =
+ String8::format("Cannot change the priority of a client %s pid %d for "
+ "camera id %s without SYSTEM_CAMERA permissions",
+ String8(clientPackageNameAdj).string(), callingPid, id.string());
+ ALOGE("%s: %s", __FUNCTION__, msg.string());
+ return STATUS_ERROR(ERROR_PERMISSION_DENIED, msg.string());
+ }
+
ret = connectHelper<hardware::camera2::ICameraDeviceCallbacks,CameraDeviceClient>(cameraCb, id,
/*api1CameraId*/-1, clientPackageNameAdj, clientFeatureId,
- clientUid, USE_CALLING_PID, API_2, /*shimUpdateOnly*/ false, /*out*/client);
+ clientUid, USE_CALLING_PID, API_2, /*shimUpdateOnly*/ false, oomScoreOffset,
+ /*out*/client);
if(!ret.isOk()) {
- logRejected(id, CameraThreadState::getCallingPid(), String8(clientPackageNameAdj),
- ret.toString8());
+ logRejected(id, callingPid, String8(clientPackageNameAdj), ret.toString8());
return ret;
}
@@ -1572,7 +1601,7 @@
Status CameraService::connectHelper(const sp<CALLBACK>& cameraCb, const String8& cameraId,
int api1CameraId, const String16& clientPackageName,
const std::optional<String16>& clientFeatureId, int clientUid, int clientPid,
- apiLevel effectiveApiLevel, bool shimUpdateOnly,
+ apiLevel effectiveApiLevel, bool shimUpdateOnly, int oomScoreOffset,
/*out*/sp<CLIENT>& device) {
binder::Status ret = binder::Status::ok();
@@ -1623,7 +1652,7 @@
sp<BasicClient> clientTmp = nullptr;
std::shared_ptr<resource_policy::ClientDescriptor<String8, sp<BasicClient>>> partial;
if ((err = handleEvictionsLocked(cameraId, originalClientPid, effectiveApiLevel,
- IInterface::asBinder(cameraCb), clientName8, /*out*/&clientTmp,
+ IInterface::asBinder(cameraCb), clientName8, oomScoreOffset, /*out*/&clientTmp,
/*out*/&partial)) != NO_ERROR) {
switch (err) {
case -ENODEV:
@@ -1741,7 +1770,7 @@
mServiceLock.lock();
} else {
// Otherwise, add client to active clients list
- finishConnectLocked(client, partial);
+ finishConnectLocked(client, partial, oomScoreOffset);
}
client->setImageDumpMask(mImageDumpMask);
@@ -1788,7 +1817,8 @@
auto offlineClientDesc = CameraClientManager::makeClientDescriptor(
kOfflineDevice + onlineClientDesc->getKey(), offlineClient, /*cost*/ 0,
/*conflictingKeys*/ std::set<String8>(), onlinePriority.getScore(),
- onlineClientDesc->getOwnerId(), onlinePriority.getState());
+ onlineClientDesc->getOwnerId(), onlinePriority.getState(),
+ /*ommScoreOffset*/ 0);
// Allow only one offline device per camera
auto incompatibleClients = mActiveClientManager.getIncompatibleClients(offlineClientDesc);
@@ -3662,21 +3692,23 @@
CameraService::DescriptorPtr CameraService::CameraClientManager::makeClientDescriptor(
const String8& key, const sp<BasicClient>& value, int32_t cost,
const std::set<String8>& conflictingKeys, int32_t score, int32_t ownerId,
- int32_t state) {
+ int32_t state, int32_t oomScoreOffset) {
bool isVendorClient = getCurrentServingCall() == BinderCallType::HWBINDER;
int32_t score_adj = isVendorClient ? kVendorClientScore : score;
int32_t state_adj = isVendorClient ? kVendorClientState: state;
return std::make_shared<resource_policy::ClientDescriptor<String8, sp<BasicClient>>>(
- key, value, cost, conflictingKeys, score_adj, ownerId, state_adj, isVendorClient);
+ key, value, cost, conflictingKeys, score_adj, ownerId, state_adj, isVendorClient,
+ oomScoreOffset);
}
CameraService::DescriptorPtr CameraService::CameraClientManager::makeClientDescriptor(
- const sp<BasicClient>& value, const CameraService::DescriptorPtr& partial) {
+ const sp<BasicClient>& value, const CameraService::DescriptorPtr& partial,
+ int32_t oomScoreOffset) {
return makeClientDescriptor(partial->getKey(), value, partial->getCost(),
partial->getConflicting(), partial->getPriority().getScore(),
- partial->getOwnerId(), partial->getPriority().getState());
+ partial->getOwnerId(), partial->getPriority().getState(), oomScoreOffset);
}
// ----------------------------------------------------------------------------
diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index 10e1748..22dd0db 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -138,7 +138,7 @@
virtual binder::Status connectDevice(
const sp<hardware::camera2::ICameraDeviceCallbacks>& cameraCb, const String16& cameraId,
const String16& clientPackageName, const std::optional<String16>& clientFeatureId,
- int32_t clientUid,
+ int32_t clientUid, int scoreOffset,
/*out*/
sp<hardware::camera2::ICameraDeviceUser>* device);
@@ -510,14 +510,14 @@
*/
static DescriptorPtr makeClientDescriptor(const String8& key, const sp<BasicClient>& value,
int32_t cost, const std::set<String8>& conflictingKeys, int32_t score,
- int32_t ownerId, int32_t state);
+ int32_t ownerId, int32_t state, int oomScoreOffset);
/**
* Make a ClientDescriptor object wrapping the given BasicClient strong pointer with
* values intialized from a prior ClientDescriptor.
*/
static DescriptorPtr makeClientDescriptor(const sp<BasicClient>& value,
- const CameraService::DescriptorPtr& partial);
+ const CameraService::DescriptorPtr& partial, int oomScoreOffset);
}; // class CameraClientManager
@@ -739,6 +739,7 @@
// Only call with with mServiceLock held.
status_t handleEvictionsLocked(const String8& cameraId, int clientPid,
apiLevel effectiveApiLevel, const sp<IBinder>& remoteCallback, const String8& packageName,
+ int scoreOffset,
/*out*/
sp<BasicClient>* client,
std::shared_ptr<resource_policy::ClientDescriptor<String8, sp<BasicClient>>>* partial);
@@ -772,7 +773,8 @@
binder::Status connectHelper(const sp<CALLBACK>& cameraCb, const String8& cameraId,
int api1CameraId, const String16& clientPackageName,
const std::optional<String16>& clientFeatureId, int clientUid, int clientPid,
- apiLevel effectiveApiLevel, bool shimUpdateOnly, /*out*/sp<CLIENT>& device);
+ apiLevel effectiveApiLevel, bool shimUpdateOnly, int scoreOffset,
+ /*out*/sp<CLIENT>& device);
// Lock guarding camera service state
Mutex mServiceLock;
@@ -831,7 +833,8 @@
*
* This method must be called with mServiceLock held.
*/
- void finishConnectLocked(const sp<BasicClient>& client, const DescriptorPtr& desc);
+ void finishConnectLocked(const sp<BasicClient>& client, const DescriptorPtr& desc,
+ int oomScoreOffset);
/**
* Returns the underlying camera Id string mapped to a camera id int
diff --git a/services/camera/libcameraservice/hidl/HidlCameraService.cpp b/services/camera/libcameraservice/hidl/HidlCameraService.cpp
index aa1e95a..88d2f72 100644
--- a/services/camera/libcameraservice/hidl/HidlCameraService.cpp
+++ b/services/camera/libcameraservice/hidl/HidlCameraService.cpp
@@ -104,7 +104,7 @@
sp<hardware::camera2::ICameraDeviceCallbacks> callbacks = hybridCallbacks;
binder::Status serviceRet = mAidlICameraService->connectDevice(
callbacks, String16(cameraId.c_str()), String16(""), {},
- hardware::ICameraService::USE_CALLING_UID, /*out*/&deviceRemote);
+ hardware::ICameraService::USE_CALLING_UID, 0/*oomScoreOffset*/, /*out*/&deviceRemote);
HStatus status = HStatus::NO_ERROR;
if (!serviceRet.isOk()) {
ALOGE("%s: Unable to connect to camera device", __FUNCTION__);
diff --git a/services/camera/libcameraservice/libcameraservice_fuzzer/camera_service_fuzzer.cpp b/services/camera/libcameraservice/libcameraservice_fuzzer/camera_service_fuzzer.cpp
index 985b2f8..8bd4ed7 100644
--- a/services/camera/libcameraservice/libcameraservice_fuzzer/camera_service_fuzzer.cpp
+++ b/services/camera/libcameraservice/libcameraservice_fuzzer/camera_service_fuzzer.cpp
@@ -526,7 +526,7 @@
sp<TestCameraDeviceCallbacks> callbacks(new TestCameraDeviceCallbacks());
sp<hardware::camera2::ICameraDeviceUser> device;
mCameraService->connectDevice(callbacks, String16(s.cameraId), String16(), {},
- android::CameraService::USE_CALLING_UID, &device);
+ android::CameraService::USE_CALLING_UID, 0/*oomScoreDiff*/, &device);
if (device == nullptr) {
continue;
}
diff --git a/services/camera/libcameraservice/tests/ClientManagerTest.cpp b/services/camera/libcameraservice/tests/ClientManagerTest.cpp
index 037c5c2..c79ef45 100644
--- a/services/camera/libcameraservice/tests/ClientManagerTest.cpp
+++ b/services/camera/libcameraservice/tests/ClientManagerTest.cpp
@@ -44,7 +44,7 @@
TestDescriptorPtr makeDescFromTestClient(const TestClient& tc) {
return std::make_shared<TestClientDescriptor>(/*ID*/tc.mId, tc, tc.mCost, tc.mConflictingKeys,
- tc.mScore, tc.mOwnerId, tc.mState, tc.mIsVendorClient);
+ tc.mScore, tc.mOwnerId, tc.mState, tc.mIsVendorClient, /*oomScoreOffset*/0);
}
class TestClientManager : public ClientManager<int, TestClient> {
diff --git a/services/camera/libcameraservice/utils/ClientManager.h b/services/camera/libcameraservice/utils/ClientManager.h
index 09258ef..d164885 100644
--- a/services/camera/libcameraservice/utils/ClientManager.h
+++ b/services/camera/libcameraservice/utils/ClientManager.h
@@ -64,20 +64,26 @@
* case where the construction is offloaded to another thread which isn't a
* hwbinder thread.
*/
- ClientPriority(int32_t score, int32_t state, bool isVendorClient) :
- mScore((score == INVALID_ADJ) ? UNKNOWN_ADJ : score),
- mState(state),
- mIsVendorClient(isVendorClient) { }
+ ClientPriority(int32_t score, int32_t state, bool isVendorClient, int32_t scoreOffset = 0) :
+ mIsVendorClient(isVendorClient), mScoreOffset(scoreOffset) {
+ setScore(score);
+ setState(state);
+ }
int32_t getScore() const { return mScore; }
int32_t getState() const { return mState; }
+ int32_t isVendorClient() const { return mIsVendorClient; }
void setScore(int32_t score) {
// For vendor clients, the score is set once and for all during
// construction. Otherwise, it can get reset each time cameraserver
// queries ActivityManagerService for oom_adj scores / states .
- if (!mIsVendorClient) {
- mScore = (score == INVALID_ADJ) ? UNKNOWN_ADJ : score;
+ // For clients where the score offset is set by the app, add it to the
+ // score provided by ActivityManagerService.
+ if (score == INVALID_ADJ) {
+ mScore = UNKNOWN_ADJ;
+ } else {
+ mScore = mScoreOffset + score;
}
}
@@ -87,9 +93,7 @@
// queries ActivityManagerService for oom_adj scores / states
// (ActivityManagerService returns a vendor process' state as
// PROCESS_STATE_NONEXISTENT.
- if (!mIsVendorClient) {
- mState = state;
- }
+ mState = state;
}
bool operator==(const ClientPriority& rhs) const {
@@ -120,6 +124,7 @@
int32_t mScore;
int32_t mState;
bool mIsVendorClient = false;
+ int32_t mScoreOffset = 0;
};
// --------------------------------------------------------------------------------
@@ -137,9 +142,10 @@
public:
ClientDescriptor(const KEY& key, const VALUE& value, int32_t cost,
const std::set<KEY>& conflictingKeys, int32_t score, int32_t ownerId, int32_t state,
- bool isVendorClient);
+ bool isVendorClient, int32_t oomScoreOffset);
ClientDescriptor(KEY&& key, VALUE&& value, int32_t cost, std::set<KEY>&& conflictingKeys,
- int32_t score, int32_t ownerId, int32_t state, bool isVendorClient);
+ int32_t score, int32_t ownerId, int32_t state, bool isVendorClient,
+ int32_t oomScoreOffset);
~ClientDescriptor();
@@ -204,18 +210,18 @@
template<class KEY, class VALUE>
ClientDescriptor<KEY, VALUE>::ClientDescriptor(const KEY& key, const VALUE& value, int32_t cost,
const std::set<KEY>& conflictingKeys, int32_t score, int32_t ownerId, int32_t state,
- bool isVendorClient) :
+ bool isVendorClient, int32_t scoreOffset) :
mKey{key}, mValue{value}, mCost{cost}, mConflicting{conflictingKeys},
- mPriority(score, state, isVendorClient),
+ mPriority(score, state, isVendorClient, scoreOffset),
mOwnerId{ownerId} {}
template<class KEY, class VALUE>
ClientDescriptor<KEY, VALUE>::ClientDescriptor(KEY&& key, VALUE&& value, int32_t cost,
std::set<KEY>&& conflictingKeys, int32_t score, int32_t ownerId, int32_t state,
- bool isVendorClient) :
+ bool isVendorClient, int32_t scoreOffset) :
mKey{std::forward<KEY>(key)}, mValue{std::forward<VALUE>(value)}, mCost{cost},
mConflicting{std::forward<std::set<KEY>>(conflictingKeys)},
- mPriority(score, state, isVendorClient), mOwnerId{ownerId} {}
+ mPriority(score, state, isVendorClient, scoreOffset), mOwnerId{ownerId} {}
template<class KEY, class VALUE>
ClientDescriptor<KEY, VALUE>::~ClientDescriptor() {}
@@ -266,6 +272,9 @@
// off in the incoming priority argument since an AIDL thread might have
// called getCurrentServingCall() == BinderCallType::HWBINDER after refreshing
// priorities for old clients through ProcessInfoService::getProcessStatesScoresFromPids().
+ if (mPriority.isVendorClient()) {
+ return;
+ }
mPriority.setScore(priority.getScore());
mPriority.setState(priority.getState());
}