From 9b5a232083d8dea613cd875712f8b5b9e5b72375 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 22 Mar 2022 16:15:00 -0700 Subject: [PATCH 1/3] Fix support for optional cgroup attributes The Linux kernel returns error code EACCES (Permission denied) if either a cgroup attribute does not exist or if the process that is trying to write into a cgroup attribute does not have permission to write. In other words, it is not possible to tell the difference between these two scenarios by checking the value of 'errno'. Hence this patch that adds a stat() call to check whether or not a cgroup attribute exists. This patch makes lines like the following disappear from logcat for optional cgroup attributes: 03-22 23:16:04.816 616 649 E libprocessgroup: Failed to write '0' to /sys/fs/cgroup/./uid_10077/p id_1695/io.group_idle: Permission denied Bug: 213617178 Test: Booted Android in Cuttlefish and inspected logcat. Change-Id: I6e041dfc34a52c9bdb75a8c70d99ad79b06eee06 Signed-off-by: Bart Van Assche --- libprocessgroup/task_profiles.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp index 78a316a6f..7a04100ac 100644 --- a/libprocessgroup/task_profiles.cpp +++ b/libprocessgroup/task_profiles.cpp @@ -207,7 +207,7 @@ bool SetAttributeAction::ExecuteForTask(int tid) const { } if (!WriteStringToFile(value_, path)) { - if (errno == ENOENT) { + if (access(path.c_str(), F_OK) < 0) { if (optional_) { return true; } else { From ebea9097a82d152d2f75b332da75217a70e8c2b0 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 25 Mar 2022 16:10:08 -0700 Subject: [PATCH 2/3] Micro-optimize MergeCgroupToDescriptors() Introduce a local variable for an expression that occurs twice. Use string append instead of string concatenation because the former is more efficient. Bug: 213617178 Test: Compile-tested only. Change-Id: I6e8b9d63b10accb220216dc484dffd18f5c54ce7 Signed-off-by: Bart Van Assche --- libprocessgroup/setup/cgroup_map_write.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libprocessgroup/setup/cgroup_map_write.cpp b/libprocessgroup/setup/cgroup_map_write.cpp index 992cc2e23..d5bce8fc5 100644 --- a/libprocessgroup/setup/cgroup_map_write.cpp +++ b/libprocessgroup/setup/cgroup_map_write.cpp @@ -147,12 +147,15 @@ static bool Mkdir(const std::string& path, mode_t mode, const std::string& uid, static void MergeCgroupToDescriptors(std::map* descriptors, const Json::Value& cgroup, const std::string& name, const std::string& root_path, int cgroups_version) { + const std::string cgroup_path = cgroup["Path"].asString(); std::string path; if (!root_path.empty()) { - path = root_path + "/" + cgroup["Path"].asString(); + path = root_path; + path += "/"; + path += cgroup_path; } else { - path = cgroup["Path"].asString(); + path = cgroup_path; } uint32_t controller_flags = 0; From d31602063eba491d19fa463fe1b7aa42596df26b Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 25 Mar 2022 16:03:54 -0700 Subject: [PATCH 3/3] Canonicalize cgroup paths Make it possible to verify whether the memory cgroup controller has been mounted in the v2 hierarchy by comparing its path with the cgroup v2 path. This patch changes the path of cgroup controllers mounted in the v2 hierarchy from /sys/fs/cgroup/. into /sys/fs/cgroup (no trailing /.). Bug: 213617178 Test: Added debug code that prints the path for the memory cgroup in the Test: v2 hierarchy. Change-Id: I8a2764a6daae84caecf360a918eab62778e3f546 Signed-off-by: Bart Van Assche --- libprocessgroup/setup/cgroup_map_write.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libprocessgroup/setup/cgroup_map_write.cpp b/libprocessgroup/setup/cgroup_map_write.cpp index d5bce8fc5..3831ef20a 100644 --- a/libprocessgroup/setup/cgroup_map_write.cpp +++ b/libprocessgroup/setup/cgroup_map_write.cpp @@ -152,8 +152,10 @@ static void MergeCgroupToDescriptors(std::map* de if (!root_path.empty()) { path = root_path; - path += "/"; - path += cgroup_path; + if (cgroup_path != ".") { + path += "/"; + path += cgroup_path; + } } else { path = cgroup_path; }