Add cpuset switching to scheduler policy mechanism.
Add the ability to add a cpuset to the supported scheduler policies.
This enables services that use the scheduler policy API to run in
the correct cpuset without explicitly requesting a cpuset via the
deprecated API.
Add tests to confirm the correct operation and fix flaky test due
to racing with thread.join().
Also fix log messages on performanced startup related to failure to
access /proc/<pid> directories for certain tasks: this failure is
expected due to sepolicy settings and the log spam is unnecessary.
Bug: 64337476
Test: performance_service_tests passes.
Change-Id: I8a82852e63e39fe72fa3b731e03237f3652cddc7
diff --git a/services/vr/performanced/performance_service.cpp b/services/vr/performanced/performance_service.cpp
index 4b9fbe0..4c26671 100644
--- a/services/vr/performanced/performance_service.cpp
+++ b/services/vr/performanced/performance_service.cpp
@@ -22,13 +22,15 @@
using android::pdx::ErrorStatus;
using android::pdx::Message;
using android::pdx::Status;
-using android::pdx::rpc::DispatchRemoteMethod;
using android::pdx::default_transport::Endpoint;
+using android::pdx::rpc::DispatchRemoteMethod;
namespace {
const char kCpuSetBasePath[] = "/dev/cpuset";
+const char kRootCpuSet[] = "/";
+
constexpr unsigned long kTimerSlackForegroundNs = 50000;
constexpr unsigned long kTimerSlackBackgroundNs = 40000000;
@@ -123,22 +125,22 @@
// hack for now to put some form of permission logic in place while a longer
// term solution is developed.
using AllowRootSystem =
- CheckAnd<SameProcess, CheckOr<UserId<AID_ROOT, AID_SYSTEM>,
- GroupId<AID_SYSTEM>>>;
+ CheckAnd<SameProcess,
+ CheckOr<UserId<AID_ROOT, AID_SYSTEM>, GroupId<AID_SYSTEM>>>;
using AllowRootSystemGraphics =
CheckAnd<SameProcess, CheckOr<UserId<AID_ROOT, AID_SYSTEM, AID_GRAPHICS>,
GroupId<AID_SYSTEM, AID_GRAPHICS>>>;
using AllowRootSystemAudio =
CheckAnd<SameProcess, CheckOr<UserId<AID_ROOT, AID_SYSTEM, AID_AUDIO>,
GroupId<AID_SYSTEM, AID_AUDIO>>>;
- using AllowRootSystemTrusted = CheckOr<Trusted, UserId<AID_ROOT, AID_SYSTEM>,
- GroupId<AID_SYSTEM>>;
+ using AllowRootSystemTrusted =
+ CheckOr<Trusted, UserId<AID_ROOT, AID_SYSTEM>, GroupId<AID_SYSTEM>>;
partition_permission_check_ = AllowRootSystemTrusted::Check;
// Setup the scheduler classes.
// TODO(eieio): Replace this with a device-specific config file.
- scheduler_classes_ = {
+ scheduler_policies_ = {
{"audio:low",
{.timer_slack = kTimerSlackForegroundNs,
.scheduler_policy = SCHED_FIFO | SCHED_RESET_ON_FORK,
@@ -183,12 +185,14 @@
{.timer_slack = kTimerSlackForegroundNs,
.scheduler_policy = SCHED_FIFO | SCHED_RESET_ON_FORK,
.priority = fifo_medium + 2,
- .permission_check = AllowRootSystemTrusted::Check}},
+ .permission_check = AllowRootSystemTrusted::Check,
+ "/system/performance"}},
{"vr:app:render",
{.timer_slack = kTimerSlackForegroundNs,
.scheduler_policy = SCHED_FIFO | SCHED_RESET_ON_FORK,
.priority = fifo_medium + 1,
- .permission_check = AllowRootSystemTrusted::Check}},
+ .permission_check = AllowRootSystemTrusted::Check,
+ "/application/performance"}},
{"normal",
{.timer_slack = kTimerSlackForegroundNs,
.scheduler_policy = SCHED_NORMAL,
@@ -219,14 +223,80 @@
Status<void> PerformanceService::OnSetSchedulerPolicy(
Message& message, pid_t task_id, const std::string& scheduler_policy) {
- // Forward to scheduler class handler for now. In the future this method will
- // subsume the others by unifying both scheduler class and cpu partiton into a
- // single policy concept.
ALOGI(
"PerformanceService::OnSetSchedulerPolicy: task_id=%d "
"scheduler_policy=%s",
task_id, scheduler_policy.c_str());
- return OnSetSchedulerClass(message, task_id, scheduler_policy);
+
+ Task task(task_id);
+ if (!task) {
+ ALOGE(
+ "PerformanceService::OnSetSchedulerPolicy: Unable to access /proc/%d "
+ "to gather task information.",
+ task_id);
+ return ErrorStatus(EINVAL);
+ }
+
+ auto search = scheduler_policies_.find(scheduler_policy);
+ if (search != scheduler_policies_.end()) {
+ auto config = search->second;
+
+ // Make sure the sending process is allowed to make the requested change to
+ // this task.
+ if (!config.IsAllowed(message, task))
+ return ErrorStatus(EINVAL);
+
+ // Get the thread group's cpu set. Policies that do not specify a cpuset
+ // should default to this cpuset.
+ std::string thread_group_cpuset;
+ Task thread_group{task.thread_group_id()};
+ if (thread_group) {
+ thread_group_cpuset = thread_group.GetCpuSetPath();
+ } else {
+ ALOGE(
+ "PerformanceService::OnSetSchedulerPolicy: Failed to get thread "
+ "group tgid=%d for task_id=%d",
+ task.thread_group_id(), task_id);
+ thread_group_cpuset = kRootCpuSet;
+ }
+
+ std::string target_cpuset;
+ if (config.cpuset.empty()) {
+ target_cpuset = thread_group_cpuset;
+ } else {
+ target_cpuset = config.cpuset;
+ }
+ ALOGI("PerformanceService::OnSetSchedulerPolicy: Using cpuset=%s",
+ target_cpuset.c_str());
+
+ auto target_set = cpuset_.Lookup(target_cpuset);
+ if (target_set) {
+ auto attach_status = target_set->AttachTask(task_id);
+ ALOGW_IF(!attach_status,
+ "PerformanceService::OnSetSchedulerPolicy: Failed to attach "
+ "task=%d to cpuset=%s: %s",
+ task_id, target_cpuset.c_str(),
+ attach_status.GetErrorMessage().c_str());
+ } else {
+ ALOGW(
+ "PerformanceService::OnSetSchedulerPolicy: Failed to lookup "
+ "cpuset=%s",
+ target_cpuset.c_str());
+ }
+
+ struct sched_param param;
+ param.sched_priority = config.priority;
+
+ sched_setscheduler(task_id, config.scheduler_policy, ¶m);
+ prctl(PR_SET_TIMERSLACK_PID, config.timer_slack, task_id);
+ return {};
+ } else {
+ ALOGE(
+ "PerformanceService::OnSetSchedulerPolicy: Invalid scheduler_policy=%s "
+ "requested by task=%d.",
+ scheduler_policy.c_str(), task_id);
+ return ErrorStatus(EINVAL);
+ }
}
Status<void> PerformanceService::OnSetCpuPartition(
@@ -259,8 +329,8 @@
if (!task)
return ErrorStatus(EINVAL);
- auto search = scheduler_classes_.find(scheduler_class);
- if (search != scheduler_classes_.end()) {
+ auto search = scheduler_policies_.find(scheduler_class);
+ if (search != scheduler_policies_.end()) {
auto config = search->second;
// Make sure the sending process is allowed to make the requested change to
diff --git a/services/vr/performanced/performance_service.h b/services/vr/performanced/performance_service.h
index b28d94a..6b519ab 100644
--- a/services/vr/performanced/performance_service.h
+++ b/services/vr/performanced/performance_service.h
@@ -44,13 +44,13 @@
int sched_fifo_min_priority_;
int sched_fifo_max_priority_;
- // Scheduler class config type.
- struct SchedulerClassConfig {
+ struct SchedulerPolicyConfig {
unsigned long timer_slack;
int scheduler_policy;
int priority;
std::function<bool(const pdx::Message& message, const Task& task)>
permission_check;
+ std::string cpuset;
// Check the permisison of the given task to use this scheduler class. If a
// permission check function is not set then operations are only allowed on
@@ -65,7 +65,7 @@
}
};
- std::unordered_map<std::string, SchedulerClassConfig> scheduler_classes_;
+ std::unordered_map<std::string, SchedulerPolicyConfig> scheduler_policies_;
std::function<bool(const pdx::Message& message, const Task& task)>
partition_permission_check_;
diff --git a/services/vr/performanced/performance_service_tests.cpp b/services/vr/performanced/performance_service_tests.cpp
index 274a1b3..4065785 100644
--- a/services/vr/performanced/performance_service_tests.cpp
+++ b/services/vr/performanced/performance_service_tests.cpp
@@ -1,24 +1,65 @@
#include <errno.h>
#include <sched.h>
+#include <stdio.h>
#include <sys/types.h>
#include <unistd.h>
#include <condition_variable>
#include <cstdlib>
+#include <iostream>
#include <mutex>
+#include <sstream>
#include <thread>
+#include <utility>
+#include <android-base/unique_fd.h>
#include <dvr/performance_client_api.h>
#include <gtest/gtest.h>
#include <private/android_filesystem_config.h>
+#include "stdio_filebuf.h"
+#include "string_trim.h"
+#include "unique_file.h"
+
+using android::dvr::Trim;
+using android::dvr::UniqueFile;
+using android::dvr::stdio_filebuf;
+
namespace {
const char kTrustedUidEnvironmentVariable[] = "GTEST_TRUSTED_UID";
+const char kProcBase[] = "/proc";
+
+std::pair<UniqueFile, int> OpenTaskFile(pid_t task_id,
+ const std::string& name) {
+ std::ostringstream stream;
+ stream << kProcBase << "/" << task_id << "/" << name;
+
+ UniqueFile file{fopen(stream.str().c_str(), "r")};
+ const int error = file ? 0 : errno;
+ return {std::move(file), error};
+}
+
+std::string GetTaskCpuSet(pid_t task_id) {
+ int error;
+ UniqueFile file;
+
+ std::tie(file, error) = OpenTaskFile(task_id, "cpuset");
+ if (!file)
+ return std::string("errno:") + strerror(error);
+
+ stdio_filebuf<char> filebuf(file.get());
+ std::istream file_stream(&filebuf);
+
+ std::string line;
+ std::getline(file_stream, line);
+ return Trim(line);
+}
+
} // anonymous namespace
-TEST(DISABLED_PerformanceTest, SetCpuPartition) {
+TEST(PerformanceTest, SetCpuPartition) {
int error;
// Test setting the the partition for the current task.
@@ -59,13 +100,6 @@
}
thread.join();
- // Test setting the partition for a task that isn't valid using
- // the task id of the thread that we just joined. Technically the
- // id could wrap around by the time we get here, but this is
- // extremely unlikely.
- error = dvrSetCpuPartition(task_id, "/application");
- EXPECT_EQ(-EINVAL, error);
-
// Test setting the partition for a task that doesn't belong to us.
error = dvrSetCpuPartition(1, "/application");
EXPECT_EQ(-EINVAL, error);
@@ -73,6 +107,10 @@
// Test setting the partition to one that doesn't exist.
error = dvrSetCpuPartition(0, "/foobar");
EXPECT_EQ(-ENOENT, error);
+
+ // Set the test back to the root partition.
+ error = dvrSetCpuPartition(0, "/");
+ EXPECT_EQ(0, error);
}
TEST(PerformanceTest, SetSchedulerClass) {
@@ -96,8 +134,6 @@
EXPECT_EQ(-EINVAL, error);
}
-// This API mirrors SetSchedulerClass for now. Replace with with a more specific
-// test once the policy API is fully implemented.
TEST(PerformanceTest, SetSchedulerPolicy) {
int error;
@@ -115,6 +151,50 @@
error = dvrSetSchedulerPolicy(0, "foobar");
EXPECT_EQ(-EINVAL, error);
+
+ // Set the test back to the root partition.
+ error = dvrSetCpuPartition(0, "/");
+ EXPECT_EQ(0, error);
+
+ const std::string original_cpuset = GetTaskCpuSet(gettid());
+ EXPECT_EQ("/", original_cpuset);
+
+ error = dvrSetSchedulerPolicy(0, "vr:system:arp");
+ EXPECT_EQ(0, error);
+ EXPECT_EQ(SCHED_FIFO | SCHED_RESET_ON_FORK, sched_getscheduler(0));
+
+ const std::string new_cpuset = GetTaskCpuSet(gettid());
+ EXPECT_NE(original_cpuset, new_cpuset);
+
+ // The cpuset for the thread group is now new_cpuset. Scheduler profiles that
+ // do not specify a cpuset should not change the cpuset of a thread, except to
+ // restore it to the thread group cpuset.
+ std::string thread_original_cpuset;
+ std::string thread_new_cpuset;
+ std::string thread_final_cpuset;
+
+ std::thread thread{
+ [&thread_original_cpuset, &thread_new_cpuset, &thread_final_cpuset]() {
+ thread_original_cpuset = GetTaskCpuSet(gettid());
+
+ int error = dvrSetSchedulerPolicy(0, "vr:app:render");
+ EXPECT_EQ(0, error);
+
+ thread_new_cpuset = GetTaskCpuSet(gettid());
+
+ error = dvrSetSchedulerPolicy(0, "normal");
+ EXPECT_EQ(0, error);
+
+ thread_final_cpuset = GetTaskCpuSet(gettid());
+ }};
+ thread.join();
+
+ EXPECT_EQ(new_cpuset, thread_original_cpuset);
+ EXPECT_NE(new_cpuset, thread_new_cpuset);
+ EXPECT_EQ(new_cpuset, thread_final_cpuset);
+
+ error = dvrSetCpuPartition(0, original_cpuset.c_str());
+ EXPECT_EQ(0, error);
}
TEST(PerformanceTest, SchedulerClassResetOnFork) {
@@ -424,11 +504,11 @@
error = dvrSetSchedulerPolicy(0, "audio:high");
EXPECT_EQ(-EINVAL, error);
error = dvrSetSchedulerPolicy(0, "graphics");
- EXPECT_EQ(0, error);
+ EXPECT_EQ(-EINVAL, error);
error = dvrSetSchedulerPolicy(0, "graphics:low");
- EXPECT_EQ(0, error);
+ EXPECT_EQ(-EINVAL, error);
error = dvrSetSchedulerPolicy(0, "graphics:high");
- EXPECT_EQ(0, error);
+ EXPECT_EQ(-EINVAL, error);
error = dvrSetSchedulerPolicy(0, "sensors");
EXPECT_EQ(-EINVAL, error);
error = dvrSetSchedulerPolicy(0, "sensors:low");
diff --git a/services/vr/performanced/task.cpp b/services/vr/performanced/task.cpp
index 1175a7b..c2f078e 100644
--- a/services/vr/performanced/task.cpp
+++ b/services/vr/performanced/task.cpp
@@ -48,15 +48,18 @@
thread_count_(0),
cpus_allowed_mask_(0) {
task_fd_ = OpenTaskDirectory(task_id_);
- ALOGE_IF(task_fd_.get() < 0,
+ const int error = errno;
+ ALOGE_IF(task_fd_.get() < 0 && error != EACCES,
"Task::Task: Failed to open task directory for task_id=%d: %s",
- task_id, strerror(errno));
+ task_id, strerror(error));
- ReadStatusFields();
-
- ALOGD_IF(TRACE, "Task::Task: task_id=%d name=%s tgid=%d ppid=%d cpu_mask=%x",
- task_id_, name_.c_str(), thread_group_id_, parent_process_id_,
- cpus_allowed_mask_);
+ if (IsValid()) {
+ ReadStatusFields();
+ ALOGD_IF(TRACE,
+ "Task::Task: task_id=%d name=%s tgid=%d ppid=%d cpu_mask=%x",
+ task_id_, name_.c_str(), thread_group_id_, parent_process_id_,
+ cpus_allowed_mask_);
+ }
}
base::unique_fd Task::OpenTaskFile(const std::string& name) const {