diff options
| -rw-r--r-- | services/vr/performanced/performance_service.cpp | 98 | ||||
| -rw-r--r-- | services/vr/performanced/performance_service.h | 6 | ||||
| -rw-r--r-- | services/vr/performanced/performance_service_tests.cpp | 106 | ||||
| -rw-r--r-- | services/vr/performanced/task.cpp | 19 |
4 files changed, 191 insertions, 38 deletions
diff --git a/services/vr/performanced/performance_service.cpp b/services/vr/performanced/performance_service.cpp index 4b9fbe047d..4c26671b11 100644 --- a/services/vr/performanced/performance_service.cpp +++ b/services/vr/performanced/performance_service.cpp @@ -22,13 +22,15 @@ using android::dvr::Task; 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 @@ PerformanceService::PerformanceService() // 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 @@ PerformanceService::PerformanceService() {.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 @@ std::string PerformanceService::DumpState(size_t /*max_length*/) { 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 @@ Status<void> PerformanceService::OnSetSchedulerClass( 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 b28d94addb..6b519abac3 100644 --- a/services/vr/performanced/performance_service.h +++ b/services/vr/performanced/performance_service.h @@ -44,13 +44,13 @@ class PerformanceService : public pdx::ServiceBase<PerformanceService> { 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 @@ class PerformanceService : public pdx::ServiceBase<PerformanceService> { } }; - 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 274a1b36d4..4065785426 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 @@ TEST(DISABLED_PerformanceTest, SetCpuPartition) { } 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(DISABLED_PerformanceTest, SetCpuPartition) { // 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 @@ TEST(PerformanceTest, SetSchedulerClass) { 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 @@ TEST(PerformanceTest, SetSchedulerPolicy) { 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 @@ TEST(PerformanceTest, Permissions) { 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 1175a7b12b..c2f078efb4 100644 --- a/services/vr/performanced/task.cpp +++ b/services/vr/performanced/task.cpp @@ -48,15 +48,18 @@ Task::Task(pid_t task_id) 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)); - - 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_); + task_id, strerror(error)); + + 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 { |