commit 4c95712c9f24f0387055bc4801f4014bea82f830
author Bart Van Assche <bvanassche@google.com> Fri Feb 04 18:19:45 2022 +0000
committer Bart Van Assche <bvanassche@google.com> Wed Mar 23 14:12:43 2022 -0700
tree de059d7f72aa4435b23206e58f2ab6dd15c9478a
parent 3e9eeb8329f5d3f9b8df329077149161ba5c540d

Use the proper memcg directory for the v2 hierarchy

When using the v1 hierarchy per process memcg directories exist under
/dev/memcg/apps. When using the v2 hierarchy per process memcg
directories exist under /sys/fs/cgroup. Hence this patch that selects
the proper top-level directory depending on the memcg version.

Bug: 213617178
Test: Verified Android operation inside the Cuttlefish emulator.
Change-Id: I7373fb407cb6ad2b1181579691ff54886fd36c24
Signed-off-by: Bart Van Assche <bvanassche@google.com>

libprocessgroup/processgroup.cpp

diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp
index 96b5537..b0fcb5f 100644
--- a/libprocessgroup/processgroup.cpp
+++ b/libprocessgroup/processgroup.cpp
@@ -69,6 +69,23 @@
     return true;
 }
 
+static bool CgroupGetMemcgAppsPath(std::string* path) {
+    CgroupController controller = CgroupMap::GetInstance().FindController("memory");
+
+    if (!controller.HasValue()) {
+        return false;
+    }
+
+    if (path) {
+        *path = controller.path();
+        if (controller.version() == 1) {
+            *path += "/apps";
+        }
+    }
+
+    return true;
+}
+
 bool CgroupGetControllerFromPath(const std::string& path, std::string* cgroup_name) {
     auto controller = CgroupMap::GetInstance().FindControllerByPath(path);
 
@@ -199,13 +216,13 @@
     LOG(VERBOSE) << "removeAllProcessGroups()";
 
     std::vector<std::string> cgroups;
-    std::string path;
+    std::string path, memcg_apps_path;
 
     if (CgroupGetControllerPath(CGROUPV2_CONTROLLER_NAME, &path)) {
         cgroups.push_back(path);
     }
-    if (CgroupGetControllerPath("memory", &path)) {
-        cgroups.push_back(path + "/apps");
+    if (CgroupGetMemcgAppsPath(&memcg_apps_path) && memcg_apps_path != path) {
+        cgroups.push_back(memcg_apps_path);
     }
 
     for (std::string cgroup_root_path : cgroups) {
@@ -411,10 +428,11 @@
         int err = RemoveProcessGroup(cgroup, uid, initialPid, retries);
 
         if (isMemoryCgroupSupported() && UsePerAppMemcg()) {
-            std::string memory_path;
-            CgroupGetControllerPath("memory", &memory_path);
-            memory_path += "/apps";
-            if (RemoveProcessGroup(memory_path.c_str(), uid, initialPid, retries)) return -1;
+            std::string memcg_apps_path;
+            if (CgroupGetMemcgAppsPath(&memcg_apps_path) &&
+                RemoveProcessGroup(memcg_apps_path.c_str(), uid, initialPid, retries) < 0) {
+                return -1;
+            }
         }
 
         return err;
@@ -491,10 +509,12 @@
         return -EINVAL;
     }
 
-    if (isMemoryCgroupSupported() && UsePerAppMemcg()) {
-        CgroupGetControllerPath("memory", &cgroup);
-        cgroup += "/apps";
-        int ret = createProcessGroupInternal(uid, initialPid, cgroup, false);
+    if (std::string memcg_apps_path;
+        isMemoryCgroupSupported() && UsePerAppMemcg() && CgroupGetMemcgAppsPath(&memcg_apps_path)) {
+        // Note by bvanassche: passing 'false' as fourth argument below implies that the v1
+        // hierarchy is used. It is not clear to me whether the above conditions guarantee that the
+        // v1 hierarchy is used.
+        int ret = createProcessGroupInternal(uid, initialPid, memcg_apps_path, false);
         if (ret != 0) {
             return ret;
         }