Merge changes from topic "memcg_v2_soong_flags" into main

* changes:
  Add build flag to split the cgroup v2 hierarchy into apps/system
  Add build flag to force memcg to the v2 cgroup hierarchy
  Use ConvertUid{Pid}ToPath for all path generation
  Fix unused params and remove unneeded cflags

8 files changed, 227 insertions, 37 deletions

diff --git a/libprocessgroup/Android.bp b/libprocessgroup/Android.bp
index c6a073789..bb855d58c 100644
--- a/libprocessgroup/Android.bp
+++ b/libprocessgroup/Android.bp
@@ -2,17 +2,36 @@ package {
     default_applicable_licenses: ["Android-Apache-2.0"],
 }
 
-cc_defaults {
-    name: "libprocessgroup_defaults",
-    cpp_std: "gnu++20",
-    cflags: [
-        "-Wall",
-        "-Werror",
-        "-Wexit-time-destructors",
-        "-Wno-unused-parameter",
+soong_config_module_type {
+    name: "libprocessgroup_flag_aware_cc_defaults",
+    module_type: "cc_defaults",
+    config_namespace: "ANDROID",
+    bool_variables: [
+        "memcg_v2_force_enabled",
+        "cgroup_v2_sys_app_isolation",
+    ],
+    properties: [
+        "cflags",
     ],
 }
 
+libprocessgroup_flag_aware_cc_defaults {
+    name: "libprocessgroup_build_flags_cc",
+    cpp_std: "gnu++20",
+    soong_config_variables: {
+        memcg_v2_force_enabled: {
+            cflags: [
+                "-DMEMCG_V2_FORCE_ENABLED=true",
+            ],
+        },
+        cgroup_v2_sys_app_isolation: {
+            cflags: [
+                "-DCGROUP_V2_SYS_APP_ISOLATION=true",
+            ],
+        },
+    },
+}
+
 cc_library_headers {
     name: "libprocessgroup_headers",
     vendor_available: true,
@@ -73,7 +92,7 @@ cc_library {
     export_header_lib_headers: [
         "libprocessgroup_headers",
     ],
-    defaults: ["libprocessgroup_defaults"],
+    defaults: ["libprocessgroup_build_flags_cc"],
     apex_available: [
         "//apex_available:platform",
         "//apex_available:anyapex",
@@ -84,7 +103,7 @@ cc_library {
 cc_test {
     name: "task_profiles_test",
     host_supported: true,
-    defaults: ["libprocessgroup_defaults"],
+    defaults: ["libprocessgroup_build_flags_cc"],
     srcs: [
         "task_profiles_test.cpp",
     ],
diff --git a/libprocessgroup/build_flags.h b/libprocessgroup/build_flags.h
new file mode 100644
index 000000000..bc3e7dff1
--- /dev/null
+++ b/libprocessgroup/build_flags.h
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#ifndef MEMCG_V2_FORCE_ENABLED
+#define MEMCG_V2_FORCE_ENABLED false
+#endif
+
+#ifndef CGROUP_V2_SYS_APP_ISOLATION
+#define CGROUP_V2_SYS_APP_ISOLATION false
+#endif
+
+namespace android::libprocessgroup_flags {
+
+inline consteval bool force_memcg_v2() {
+    return MEMCG_V2_FORCE_ENABLED;
+}
+
+inline consteval bool cgroup_v2_sys_app_isolation() {
+    return CGROUP_V2_SYS_APP_ISOLATION;
+}
+
+}  // namespace android::libprocessgroup_flags
diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp
index 94d950209..8df2805d9 100644
--- a/libprocessgroup/processgroup.cpp
+++ b/libprocessgroup/processgroup.cpp
@@ -78,14 +78,6 @@ bool CgroupGetControllerPath(const std::string& cgroup_name, std::string* path)
     return true;
 }
 
-static std::string ConvertUidToPath(const char* cgroup, uid_t uid) {
-    return StringPrintf("%s/uid_%u", cgroup, uid);
-}
-
-static std::string ConvertUidPidToPath(const char* cgroup, uid_t uid, pid_t pid) {
-    return StringPrintf("%s/uid_%u/pid_%d", cgroup, uid, pid);
-}
-
 static bool CgroupKillAvailable() {
     static std::once_flag f;
     static bool cgroup_kill_available = false;
diff --git a/libprocessgroup/setup/Android.bp b/libprocessgroup/setup/Android.bp
index ea6c24706..1e0783ab0 100644
--- a/libprocessgroup/setup/Android.bp
+++ b/libprocessgroup/setup/Android.bp
@@ -41,8 +41,5 @@ cc_library_shared {
     export_header_lib_headers: [
         "libprocessgroup_headers",
     ],
-    cflags: [
-        "-Wall",
-        "-Werror",
-    ],
+    defaults: ["libprocessgroup_build_flags_cc"],
 }
diff --git a/libprocessgroup/setup/cgroup_map_write.cpp b/libprocessgroup/setup/cgroup_map_write.cpp
index 4e44c91be..1b26fbcc6 100644
--- a/libprocessgroup/setup/cgroup_map_write.cpp
+++ b/libprocessgroup/setup/cgroup_map_write.cpp
@@ -29,7 +29,7 @@
 #include <time.h>
 #include <unistd.h>
 
-#include <regex>
+#include <optional>
 
 #include <android-base/file.h>
 #include <android-base/logging.h>
@@ -43,6 +43,7 @@
 #include <processgroup/processgroup.h>
 #include <processgroup/setup.h>
 
+#include "../build_flags.h"
 #include "cgroup_descriptor.h"
 
 using android::base::GetUintProperty;
@@ -57,6 +58,8 @@ static constexpr const char* CGROUPS_DESC_VENDOR_FILE = "/vendor/etc/cgroups.jso
 
 static constexpr const char* TEMPLATE_CGROUPS_DESC_API_FILE = "/etc/task_profiles/cgroups_%u.json";
 
+static const std::string CGROUP_V2_ROOT_DEFAULT = "/sys/fs/cgroup";
+
 static bool ChangeDirModeAndOwner(const std::string& path, mode_t mode, const std::string& uid,
                                   const std::string& gid, bool permissive_mode = false) {
     uid_t pw_uid = -1;
@@ -182,6 +185,8 @@ static void MergeCgroupToDescriptors(std::map<std::string, CgroupDescriptor>* de
     }
 }
 
+static const bool force_memcg_v2 = android::libprocessgroup_flags::force_memcg_v2();
+
 static bool ReadDescriptorsFromFile(const std::string& file_name,
                                     std::map<std::string, CgroupDescriptor>* descriptors) {
     std::vector<CgroupDescriptor> result;
@@ -205,22 +210,41 @@ static bool ReadDescriptorsFromFile(const std::string& file_name,
         const Json::Value& cgroups = root["Cgroups"];
         for (Json::Value::ArrayIndex i = 0; i < cgroups.size(); ++i) {
             std::string name = cgroups[i]["Controller"].asString();
+
+            if (force_memcg_v2 && name == "memory") continue;
+
             MergeCgroupToDescriptors(descriptors, cgroups[i], name, "", 1);
         }
     }
 
+    bool memcgv2_present = false;
+    std::string root_path;
     if (root.isMember("Cgroups2")) {
         const Json::Value& cgroups2 = root["Cgroups2"];
-        std::string root_path = cgroups2["Path"].asString();
+        root_path = cgroups2["Path"].asString();
         MergeCgroupToDescriptors(descriptors, cgroups2, CGROUPV2_HIERARCHY_NAME, "", 2);
 
         const Json::Value& childGroups = cgroups2["Controllers"];
         for (Json::Value::ArrayIndex i = 0; i < childGroups.size(); ++i) {
             std::string name = childGroups[i]["Controller"].asString();
+
+            if (force_memcg_v2 && name == "memory") memcgv2_present = true;
+
             MergeCgroupToDescriptors(descriptors, childGroups[i], name, root_path, 2);
         }
     }
 
+    if (force_memcg_v2 && !memcgv2_present) {
+        LOG(INFO) << "Forcing memcg to v2 hierarchy";
+        Json::Value memcgv2;
+        memcgv2["Controller"] = "memory";
+        memcgv2["NeedsActivation"] = true;
+        memcgv2["Path"] = ".";
+        memcgv2["Optional"] = true;  // In case of cgroup_disabled=memory, so we can still boot
+        MergeCgroupToDescriptors(descriptors, memcgv2, "memory",
+                                 root_path.empty() ? CGROUP_V2_ROOT_DEFAULT : root_path, 2);
+    }
+
     return true;
 }
 
@@ -308,7 +332,8 @@ static bool ActivateV2CgroupController(const CgroupDescriptor& descriptor) {
 
         if (!base::WriteStringToFile(str, path)) {
             if (IsOptionalController(controller)) {
-                PLOG(INFO) << "Failed to activate optional controller " << controller->name();
+                PLOG(INFO) << "Failed to activate optional controller " << controller->name()
+                           << " at " << path;
                 return true;
             }
             PLOG(ERROR) << "Failed to activate controller " << controller->name();
@@ -424,6 +449,76 @@ void CgroupDescriptor::set_mounted(bool mounted) {
 }  // namespace cgrouprc
 }  // namespace android
 
+static std::optional<bool> MGLRUDisabled() {
+    const std::string file_name = "/sys/kernel/mm/lru_gen/enabled";
+    std::string content;
+    if (!android::base::ReadFileToString(file_name, &content)) {
+        PLOG(ERROR) << "Failed to read MGLRU state from " << file_name;
+        return {};
+    }
+
+    return content == "0x0000";
+}
+
+static std::optional<bool> MEMCGDisabled(
+        const std::map<std::string, android::cgrouprc::CgroupDescriptor>& descriptors) {
+    std::string cgroup_v2_root = android::cgrouprc::CGROUP_V2_ROOT_DEFAULT;
+    const auto it = descriptors.find(CGROUPV2_HIERARCHY_NAME);
+    if (it == descriptors.end()) {
+        LOG(WARNING) << "No Cgroups2 path found in cgroups.json. Vendor has modified Android, and "
+                     << "kernel memory use will be higher than intended.";
+    } else if (it->second.controller()->path() != cgroup_v2_root) {
+        cgroup_v2_root = it->second.controller()->path();
+    }
+
+    const std::string file_name = cgroup_v2_root + "/cgroup.controllers";
+    std::string content;
+    if (!android::base::ReadFileToString(file_name, &content)) {
+        PLOG(ERROR) << "Failed to read cgroup controllers from " << file_name;
+        return {};
+    }
+
+    // If we've forced memcg to v2 and it's not available, then it could only have been disabled
+    // on the kernel command line (GKI sets CONFIG_MEMCG).
+    return content.find("memory") == std::string::npos;
+}
+
+static bool CreateV2SubHierarchy(
+        const std::string& path,
+        const std::map<std::string, android::cgrouprc::CgroupDescriptor>& descriptors) {
+    using namespace android::cgrouprc;
+
+    const auto cgv2_iter = descriptors.find(CGROUPV2_HIERARCHY_NAME);
+    if (cgv2_iter == descriptors.end()) return false;
+    const android::cgrouprc::CgroupDescriptor cgv2_descriptor = cgv2_iter->second;
+
+    if (!Mkdir(path, cgv2_descriptor.mode(), cgv2_descriptor.uid(), cgv2_descriptor.gid())) {
+        PLOG(ERROR) << "Failed to create directory for " << path;
+        return false;
+    }
+
+    // Activate all v2 controllers in path so they can be activated in
+    // children as they are created.
+    for (const auto& [name, descriptor] : descriptors) {
+        const format::CgroupController* controller = descriptor.controller();
+        std::uint32_t flags = controller->flags();
+        if (controller->version() == 2 && name != CGROUPV2_HIERARCHY_NAME &&
+            flags & CGROUPRC_CONTROLLER_FLAG_NEEDS_ACTIVATION) {
+            std::string str("+");
+            str += controller->name();
+            if (!android::base::WriteStringToFile(str, path + "/cgroup.subtree_control")) {
+                if (flags & CGROUPRC_CONTROLLER_FLAG_OPTIONAL) {
+                    PLOG(WARNING) << "Activation of cgroup controller " << str << " failed in path "
+                                  << path;
+                } else {
+                    return false;
+                }
+            }
+        }
+    }
+    return true;
+}
+
 bool CgroupSetup() {
     using namespace android::cgrouprc;
 
@@ -457,6 +552,32 @@ bool CgroupSetup() {
         }
     }
 
+    if (force_memcg_v2) {
+        if (MGLRUDisabled().value_or(false)) {
+            LOG(WARNING) << "Memcg forced to v2 hierarchy with MGLRU disabled! "
+                         << "Global reclaim performance will suffer.";
+        }
+        if (MEMCGDisabled(descriptors).value_or(false)) {
+            LOG(WARNING) << "Memcg forced to v2 hierarchy while memcg is disabled by kernel "
+                         << "command line!";
+        }
+    }
+
+    // System / app isolation.
+    // This really belongs in early-init in init.rc, but we cannot use the flag there.
+    if (android::libprocessgroup_flags::cgroup_v2_sys_app_isolation()) {
+        const auto it = descriptors.find(CGROUPV2_HIERARCHY_NAME);
+        const std::string cgroup_v2_root = (it == descriptors.end())
+                                                   ? CGROUP_V2_ROOT_DEFAULT
+                                                   : it->second.controller()->path();
+
+        LOG(INFO) << "Using system/app isolation under: " << cgroup_v2_root;
+        if (!CreateV2SubHierarchy(cgroup_v2_root + "/apps", descriptors) ||
+            !CreateV2SubHierarchy(cgroup_v2_root + "/system", descriptors)) {
+            return false;
+        }
+    }
+
     // mkdir <CGROUPS_RC_DIR> 0711 system system
     if (!Mkdir(android::base::Dirname(CGROUPS_RC_PATH), 0711, "system", "system")) {
         LOG(ERROR) << "Failed to create directory for " << CGROUPS_RC_PATH << " file";
diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp
index 2353cf18a..0c2252b17 100644
--- a/libprocessgroup/task_profiles.cpp
+++ b/libprocessgroup/task_profiles.cpp
@@ -33,6 +33,8 @@
 #include <json/reader.h>
 #include <json/value.h>
 
+#include <build_flags.h>
+
 // To avoid issues in sdk_mac build
 #if defined(__ANDROID__)
 #include <sys/prctl.h>
@@ -126,11 +128,29 @@ void ProfileAttribute::Reset(const CgroupController& controller, const std::stri
     file_v2_name_ = file_v2_name;
 }
 
+static bool isSystemApp(uid_t uid) {
+    return uid < AID_APP_START;
+}
+
+std::string ConvertUidToPath(const char* root_cgroup_path, uid_t uid) {
+    if (android::libprocessgroup_flags::cgroup_v2_sys_app_isolation()) {
+        if (isSystemApp(uid))
+            return StringPrintf("%s/system/uid_%u", root_cgroup_path, uid);
+        else
+            return StringPrintf("%s/apps/uid_%u", root_cgroup_path, uid);
+    }
+    return StringPrintf("%s/uid_%u", root_cgroup_path, uid);
+}
+
+std::string ConvertUidPidToPath(const char* root_cgroup_path, uid_t uid, pid_t pid) {
+    const std::string uid_path = ConvertUidToPath(root_cgroup_path, uid);
+    return StringPrintf("%s/pid_%d", uid_path.c_str(), pid);
+}
+
 bool ProfileAttribute::GetPathForProcess(uid_t uid, pid_t pid, std::string* path) const {
     if (controller()->version() == 2) {
-        // all cgroup v2 attributes use the same process group hierarchy
-        *path = StringPrintf("%s/uid_%u/pid_%d/%s", controller()->path(), uid, pid,
-                             file_name().c_str());
+        const std::string cgroup_path = ConvertUidPidToPath(controller()->path(), uid, pid);
+        *path = cgroup_path + "/" + file_name();
         return true;
     }
     return GetPathForTask(pid, path);
@@ -155,12 +175,14 @@ bool ProfileAttribute::GetPathForTask(pid_t tid, std::string* path) const {
     return true;
 }
 
+// NOTE: This function is for cgroup v2 only
 bool ProfileAttribute::GetPathForUID(uid_t uid, std::string* path) const {
     if (path == nullptr) {
         return true;
     }
 
-    *path = StringPrintf("%s/uid_%u/%s", controller()->path(), uid, file_name().c_str());
+    const std::string cgroup_path = ConvertUidToPath(controller()->path(), uid);
+    *path = cgroup_path + "/" + file_name();
     return true;
 }
 
diff --git a/libprocessgroup/task_profiles.h b/libprocessgroup/task_profiles.h
index 2fa193177..7e3c50d9f 100644
--- a/libprocessgroup/task_profiles.h
+++ b/libprocessgroup/task_profiles.h
@@ -82,8 +82,8 @@ class ProfileAction {
 
     virtual void EnableResourceCaching(ResourceCacheType) {}
     virtual void DropResourceCaching(ResourceCacheType) {}
-    virtual bool IsValidForProcess(uid_t uid, pid_t pid) const { return false; }
-    virtual bool IsValidForTask(pid_t tid) const { return false; }
+    virtual bool IsValidForProcess(uid_t, pid_t) const { return false; }
+    virtual bool IsValidForTask(pid_t) const { return false; }
 
   protected:
     enum CacheUseResult { SUCCESS, FAIL, UNUSED };
@@ -109,8 +109,8 @@ class SetTimerSlackAction : public ProfileAction {
 
     const char* Name() const override { return "SetTimerSlack"; }
     bool ExecuteForTask(pid_t tid) const override;
-    bool IsValidForProcess(uid_t uid, pid_t pid) const override { return true; }
-    bool IsValidForTask(pid_t tid) const override { return true; }
+    bool IsValidForProcess(uid_t, pid_t) const override { return true; }
+    bool IsValidForTask(pid_t) const override { return true; }
 
   private:
     unsigned long slack_;
@@ -252,3 +252,6 @@ class TaskProfiles {
     std::map<std::string, std::shared_ptr<TaskProfile>, std::less<>> profiles_;
     std::map<std::string, std::unique_ptr<IProfileAttribute>, std::less<>> attributes_;
 };
+
+std::string ConvertUidToPath(const char* root_cgroup_path, uid_t uid);
+std::string ConvertUidPidToPath(const char* root_cgroup_path, uid_t uid, pid_t pid);
diff --git a/libprocessgroup/task_profiles_test.cpp b/libprocessgroup/task_profiles_test.cpp
index b17e695e5..d19da2b54 100644
--- a/libprocessgroup/task_profiles_test.cpp
+++ b/libprocessgroup/task_profiles_test.cpp
@@ -102,8 +102,7 @@ class ProfileAttributeMock : public IProfileAttribute {
   public:
     ProfileAttributeMock(const std::string& file_name) : file_name_(file_name) {}
     ~ProfileAttributeMock() override = default;
-    void Reset(const CgroupController& controller, const std::string& file_name,
-               const std::string& file_v2_name) override {
+    void Reset(const CgroupController&, const std::string&, const std::string&) override {
         CHECK(false);
     }
     const CgroupController* controller() const override {
@@ -111,10 +110,10 @@ class ProfileAttributeMock : public IProfileAttribute {
         return {};
     }
     const std::string& file_name() const override { return file_name_; }
-    bool GetPathForProcess(uid_t uid, pid_t pid, std::string* path) const override {
+    bool GetPathForProcess(uid_t, pid_t pid, std::string* path) const override {
         return GetPathForTask(pid, path);
     }
-    bool GetPathForTask(int tid, std::string* path) const override {
+    bool GetPathForTask(int, std::string* path) const override {
 #ifdef __ANDROID__
         CHECK(CgroupGetControllerPath(CGROUPV2_HIERARCHY_NAME, path));
         CHECK_GT(path->length(), 0);