From a3cf826de61ce3eafa4410092e5ae6ed83b932b1 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Thu, 31 Oct 2024 22:38:50 +0000 Subject: [PATCH] Set input thread priority to RT - try 2 This reverts commit de6707df0c0decfeb92a1e73a64d3a610e16d061. Reason for revert: changing code without modifying JSON file now Original description: To improve input latency, set the critical input threads to RT priority. This will use RT priority on AOSP devices by default. OEMs can still choose to customize what "input policy" means for their device, which may not necessarily mean RT. For example, on device with multiple small / big cores, input task affinity could be changed to prioritize big cores + higher CPU frequency / voltage, but still keep the standard / default input thread priority. With this patch, I'm finding that sometimes, one of the critical input threads has priority 100 instead of the expected 98. Still looking into that specific issue, but the issue is already present with the existing "input policy" code. Bug: 330719044 Flag: com.android.input.flags.enable_input_policy_profile Test: took perfetto trace and checked the priority on InputDispatcher and InputReader threads. Change-Id: I3dabf4da0398324cf542e701c103551343b883cf --- libprocessgroup/task_profiles.cpp | 56 ++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp index cc675b3fe..f3594e345 100644 --- a/libprocessgroup/task_profiles.cpp +++ b/libprocessgroup/task_profiles.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -54,6 +55,7 @@ static constexpr const char* TASK_PROFILE_DB_VENDOR_FILE = "/vendor/etc/task_pro static constexpr const char* TEMPLATE_TASK_PROFILE_API_FILE = "/etc/task_profiles/task_profiles_%u.json"; +namespace { class FdCacheHelper { public: @@ -64,8 +66,11 @@ class FdCacheHelper { }; static void Cache(const std::string& path, android::base::unique_fd& fd); + static void Drop(android::base::unique_fd& fd); + static void Init(const std::string& path, android::base::unique_fd& fd); + static bool IsCached(const android::base::unique_fd& fd) { return fd > FDS_INACCESSIBLE; } private: @@ -116,6 +121,17 @@ bool FdCacheHelper::IsAppDependentPath(const std::string& path) { return path.find("", 0) != std::string::npos || path.find("", 0) != std::string::npos; } +std::optional readLong(const std::string& str) { + char* end; + const long result = strtol(str.c_str(), &end, 10); + if (end > str.c_str()) { + return result; + } + return std::nullopt; +} + +} // namespace + IProfileAttribute::~IProfileAttribute() = default; const std::string& ProfileAttribute::file_name() const { @@ -924,15 +940,12 @@ bool TaskProfiles::Load(const CgroupMap& cg_map, const std::string& file_name) { LOG(WARNING) << "JoinCgroup: controller " << controller_name << " is not found"; } } else if (action_name == "SetTimerSlack") { - std::string slack_value = params_val["Slack"].asString(); - char* end; - unsigned long slack; - - slack = strtoul(slack_value.c_str(), &end, 10); - if (end > slack_value.c_str()) { - profile->Add(std::make_unique(slack)); + const std::string slack_string = params_val["Slack"].asString(); + std::optional slack = readLong(slack_string); + if (slack && *slack >= 0) { + profile->Add(std::make_unique(*slack)); } else { - LOG(WARNING) << "SetTimerSlack: invalid parameter: " << slack_value; + LOG(WARNING) << "SetTimerSlack: invalid parameter: " << slack_string; } } else if (action_name == "SetAttribute") { std::string attr_name = params_val["Name"].asString(); @@ -991,15 +1004,19 @@ bool TaskProfiles::Load(const CgroupMap& cg_map, const std::string& file_name) { // If present, this optional value will be passed in an additional syscall // to setpriority(), since the sched_priority value must be 0 for calls to // sched_setscheduler() with "normal" policies. - const int nice = params_val["Nice"].asInt(); + const std::string nice_string = params_val["Nice"].asString(); + const std::optional nice = readLong(nice_string); + if (!nice) { + LOG(FATAL) << "Invalid nice value specified: " << nice_string; + } const int LINUX_MIN_NICE = -20; const int LINUX_MAX_NICE = 19; - if (nice < LINUX_MIN_NICE || nice > LINUX_MAX_NICE) { - LOG(WARNING) << "SetSchedulerPolicy: Provided nice (" << nice + if (*nice < LINUX_MIN_NICE || *nice > LINUX_MAX_NICE) { + LOG(WARNING) << "SetSchedulerPolicy: Provided nice (" << *nice << ") appears out of range."; } - profile->Add(std::make_unique(policy, nice)); + profile->Add(std::make_unique(policy, *nice)); } else { profile->Add(std::make_unique(policy)); } @@ -1012,11 +1029,18 @@ bool TaskProfiles::Load(const CgroupMap& cg_map, const std::string& file_name) { // This is a "virtual priority" as described by `man 2 sched_get_priority_min` // that will be mapped onto the following range for the provided policy: // [sched_get_priority_min(), sched_get_priority_max()] - const int virtual_priority = params_val["Priority"].asInt(); - int priority; - if (SetSchedulerPolicyAction::toPriority(policy, virtual_priority, priority)) { - profile->Add(std::make_unique(policy, priority)); + const std::string priority_string = params_val["Priority"].asString(); + std::optional virtual_priority = readLong(priority_string); + if (virtual_priority && *virtual_priority > 0) { + int priority; + if (SetSchedulerPolicyAction::toPriority(policy, *virtual_priority, + priority)) { + profile->Add( + std::make_unique(policy, priority)); + } + } else { + LOG(WARNING) << "Invalid priority value: " << priority_string; } } } else {