From fd285d2b70537fd3ef239ae3843c30ccf07ac161 Mon Sep 17 00:00:00 2001 From: Suren Baghdasaryan Date: Wed, 8 May 2019 17:59:55 -0700 Subject: [PATCH] libprocessgroup: add flags to indicate when a controller failed to mount Controllers listed in cgroups.json file might fail to mount if kernel is not configured to support them. We need a way to indicate whether a controller was successfully mounted and is usable to avoid logging errors and warnings when a controller that failed to mount is being used. Add flags bitmask to cgrouprc controller descriptor and use a bit to indicate that controller is successfully mounted. Modify cpusets_enabled() and schedboost_enabled() functions to use this bit and report the actual availability of the controller. Bug: 124080437 Test: libcutils_test with cpuset and schedtune controllers disabled Change-Id: I770cc39fe50465146e3205aacf77dc3c56923c5d Signed-off-by: Suren Baghdasaryan --- libprocessgroup/cgroup_map.cpp | 8 ++++++++ libprocessgroup/cgroup_map.h | 1 + libprocessgroup/cgrouprc/Android.bp | 6 +++--- .../cgrouprc/cgroup_controller.cpp | 5 +++++ .../cgrouprc/include/android/cgrouprc.h | 12 +++++++++++ ...cgrouprc.map.txt => libcgrouprc.llndk.txt} | 1 + .../cgrouprc_format/cgroup_controller.cpp | 19 ++++++++++++++++-- .../processgroup/format/cgroup_controller.h | 9 +++++++-- libprocessgroup/processgroup.cpp | 3 +-- libprocessgroup/sched_policy.cpp | 6 +++--- libprocessgroup/setup/cgroup_descriptor.h | 2 ++ libprocessgroup/setup/cgroup_map_write.cpp | 20 +++++++++++++++---- 12 files changed, 76 insertions(+), 16 deletions(-) rename libprocessgroup/cgrouprc/{libcgrouprc.map.txt => libcgrouprc.llndk.txt} (88%) diff --git a/libprocessgroup/cgroup_map.cpp b/libprocessgroup/cgroup_map.cpp index 6cd6b6e50..92fcd1e71 100644 --- a/libprocessgroup/cgroup_map.cpp +++ b/libprocessgroup/cgroup_map.cpp @@ -67,6 +67,13 @@ bool CgroupController::HasValue() const { return controller_ != nullptr; } +bool CgroupController::IsUsable() const { + if (!HasValue()) return false; + + uint32_t flags = ACgroupController_getFlags(controller_); + return (flags & CGROUPRC_CONTROLLER_FLAG_MOUNTED) != 0; +} + std::string CgroupController::GetTasksFilePath(const std::string& rel_path) const { std::string tasks_path = path(); @@ -153,6 +160,7 @@ void CgroupMap::Print() const { const ACgroupController* controller = ACgroupFile_getController(i); LOG(INFO) << "\t" << ACgroupController_getName(controller) << " ver " << ACgroupController_getVersion(controller) << " path " + << ACgroupController_getFlags(controller) << " flags " << ACgroupController_getPath(controller); } } diff --git a/libprocessgroup/cgroup_map.h b/libprocessgroup/cgroup_map.h index d765e600c..935041242 100644 --- a/libprocessgroup/cgroup_map.h +++ b/libprocessgroup/cgroup_map.h @@ -38,6 +38,7 @@ class CgroupController { const char* path() const; bool HasValue() const; + bool IsUsable() const; std::string GetTasksFilePath(const std::string& path) const; std::string GetProcsFilePath(const std::string& path, uid_t uid, pid_t pid) const; diff --git a/libprocessgroup/cgrouprc/Android.bp b/libprocessgroup/cgrouprc/Android.bp index 6848620f9..9d5afebd3 100644 --- a/libprocessgroup/cgrouprc/Android.bp +++ b/libprocessgroup/cgrouprc/Android.bp @@ -42,19 +42,19 @@ cc_library { "libcgrouprc_format", ], stubs: { - symbol_file: "libcgrouprc.map.txt", + symbol_file: "libcgrouprc.llndk.txt", versions: ["29"], }, target: { linux: { - version_script: "libcgrouprc.map.txt", + version_script: "libcgrouprc.llndk.txt", }, }, } llndk_library { name: "libcgrouprc", - symbol_file: "libcgrouprc.map.txt", + symbol_file: "libcgrouprc.llndk.txt", export_include_dirs: [ "include", ], diff --git a/libprocessgroup/cgrouprc/cgroup_controller.cpp b/libprocessgroup/cgrouprc/cgroup_controller.cpp index d064d312e..5a326e55d 100644 --- a/libprocessgroup/cgrouprc/cgroup_controller.cpp +++ b/libprocessgroup/cgrouprc/cgroup_controller.cpp @@ -27,6 +27,11 @@ uint32_t ACgroupController_getVersion(const ACgroupController* controller) { return controller->version(); } +uint32_t ACgroupController_getFlags(const ACgroupController* controller) { + CHECK(controller != nullptr); + return controller->flags(); +} + const char* ACgroupController_getName(const ACgroupController* controller) { CHECK(controller != nullptr); return controller->name(); diff --git a/libprocessgroup/cgrouprc/include/android/cgrouprc.h b/libprocessgroup/cgrouprc/include/android/cgrouprc.h index 4edd239e2..ffc9f0b60 100644 --- a/libprocessgroup/cgrouprc/include/android/cgrouprc.h +++ b/libprocessgroup/cgrouprc/include/android/cgrouprc.h @@ -65,6 +65,18 @@ __attribute__((warn_unused_result)) const ACgroupController* ACgroupFile_getCont __attribute__((warn_unused_result)) uint32_t ACgroupController_getVersion(const ACgroupController*) __INTRODUCED_IN(29); +/** + * Flag bitmask used in ACgroupController_getFlags + */ +#define CGROUPRC_CONTROLLER_FLAG_MOUNTED 0x1 + +/** + * Returns the flags bitmask of the given controller. + * If the given controller is null, return 0. + */ +__attribute__((warn_unused_result)) uint32_t ACgroupController_getFlags(const ACgroupController*) + __INTRODUCED_IN(29); + /** * Returns the name of the given controller. * If the given controller is null, return nullptr. diff --git a/libprocessgroup/cgrouprc/libcgrouprc.map.txt b/libprocessgroup/cgrouprc/libcgrouprc.llndk.txt similarity index 88% rename from libprocessgroup/cgrouprc/libcgrouprc.map.txt rename to libprocessgroup/cgrouprc/libcgrouprc.llndk.txt index 91df3929d..ea3df33e6 100644 --- a/libprocessgroup/cgrouprc/libcgrouprc.map.txt +++ b/libprocessgroup/cgrouprc/libcgrouprc.llndk.txt @@ -4,6 +4,7 @@ LIBCGROUPRC { # introduced=29 ACgroupFile_getControllerCount; ACgroupFile_getController; ACgroupController_getVersion; + ACgroupController_getFlags; ACgroupController_getName; ACgroupController_getPath; local: diff --git a/libprocessgroup/cgrouprc_format/cgroup_controller.cpp b/libprocessgroup/cgrouprc_format/cgroup_controller.cpp index 877eed872..202b23eed 100644 --- a/libprocessgroup/cgrouprc_format/cgroup_controller.cpp +++ b/libprocessgroup/cgrouprc_format/cgroup_controller.cpp @@ -20,12 +20,19 @@ namespace android { namespace cgrouprc { namespace format { -CgroupController::CgroupController(uint32_t version, const std::string& name, - const std::string& path) { +CgroupController::CgroupController() : version_(0), flags_(0) { + memset(name_, 0, sizeof(name_)); + memset(path_, 0, sizeof(path_)); +} + +CgroupController::CgroupController(uint32_t version, uint32_t flags, const std::string& name, + const std::string& path) + : CgroupController() { // strlcpy isn't available on host. Although there is an implementation // in licutils, libcutils itself depends on libcgrouprc_format, causing // a circular dependency. version_ = version; + flags_ = flags; strncpy(name_, name.c_str(), sizeof(name_) - 1); name_[sizeof(name_) - 1] = '\0'; strncpy(path_, path.c_str(), sizeof(path_) - 1); @@ -36,6 +43,10 @@ uint32_t CgroupController::version() const { return version_; } +uint32_t CgroupController::flags() const { + return flags_; +} + const char* CgroupController::name() const { return name_; } @@ -44,6 +55,10 @@ const char* CgroupController::path() const { return path_; } +void CgroupController::set_flags(uint32_t flags) { + flags_ = flags; +} + } // namespace format } // namespace cgrouprc } // namespace android diff --git a/libprocessgroup/cgrouprc_format/include/processgroup/format/cgroup_controller.h b/libprocessgroup/cgrouprc_format/include/processgroup/format/cgroup_controller.h index 64c7532d9..40d85480b 100644 --- a/libprocessgroup/cgrouprc_format/include/processgroup/format/cgroup_controller.h +++ b/libprocessgroup/cgrouprc_format/include/processgroup/format/cgroup_controller.h @@ -26,18 +26,23 @@ namespace format { // Minimal controller description to be mmapped into process address space struct CgroupController { public: - CgroupController() {} - CgroupController(uint32_t version, const std::string& name, const std::string& path); + CgroupController(); + CgroupController(uint32_t version, uint32_t flags, const std::string& name, + const std::string& path); uint32_t version() const; + uint32_t flags() const; const char* name() const; const char* path() const; + void set_flags(uint32_t flags); + private: static constexpr size_t CGROUP_NAME_BUF_SZ = 16; static constexpr size_t CGROUP_PATH_BUF_SZ = 32; uint32_t version_; + uint32_t flags_; char name_[CGROUP_NAME_BUF_SZ]; char path_[CGROUP_PATH_BUF_SZ]; }; diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp index 1485ae986..d3ac26bd1 100644 --- a/libprocessgroup/processgroup.cpp +++ b/libprocessgroup/processgroup.cpp @@ -106,8 +106,7 @@ bool UsePerAppMemcg() { } static bool isMemoryCgroupSupported() { - std::string cgroup_name; - static bool memcg_supported = CgroupMap::GetInstance().FindController("memory").HasValue(); + static bool memcg_supported = CgroupMap::GetInstance().FindController("memory").IsUsable(); return memcg_supported; } diff --git a/libprocessgroup/sched_policy.cpp b/libprocessgroup/sched_policy.cpp index fe4f93b1e..15f8139b1 100644 --- a/libprocessgroup/sched_policy.cpp +++ b/libprocessgroup/sched_policy.cpp @@ -151,19 +151,19 @@ int set_sched_policy(int tid, SchedPolicy policy) { } bool cpusets_enabled() { - static bool enabled = (CgroupMap::GetInstance().FindController("cpuset").HasValue()); + static bool enabled = (CgroupMap::GetInstance().FindController("cpuset").IsUsable()); return enabled; } bool schedboost_enabled() { - static bool enabled = (CgroupMap::GetInstance().FindController("schedtune").HasValue()); + static bool enabled = (CgroupMap::GetInstance().FindController("schedtune").IsUsable()); return enabled; } static int getCGroupSubsys(int tid, const char* subsys, std::string& subgroup) { auto controller = CgroupMap::GetInstance().FindController(subsys); - if (!controller.HasValue()) return -1; + if (!controller.IsUsable()) return -1; if (!controller.GetTaskGroup(tid, &subgroup)) { LOG(ERROR) << "Failed to find cgroup for tid " << tid; diff --git a/libprocessgroup/setup/cgroup_descriptor.h b/libprocessgroup/setup/cgroup_descriptor.h index 597060e35..f029c4f12 100644 --- a/libprocessgroup/setup/cgroup_descriptor.h +++ b/libprocessgroup/setup/cgroup_descriptor.h @@ -32,6 +32,8 @@ class CgroupDescriptor { std::string uid() const { return uid_; } std::string gid() const { return gid_; } + void set_mounted(bool mounted); + private: format::CgroupController controller_; mode_t mode_ = 0; diff --git a/libprocessgroup/setup/cgroup_map_write.cpp b/libprocessgroup/setup/cgroup_map_write.cpp index da6094879..17ea06e09 100644 --- a/libprocessgroup/setup/cgroup_map_write.cpp +++ b/libprocessgroup/setup/cgroup_map_write.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -267,7 +268,17 @@ static bool WriteRcFile(const std::map& descripto CgroupDescriptor::CgroupDescriptor(uint32_t version, const std::string& name, const std::string& path, mode_t mode, const std::string& uid, const std::string& gid) - : controller_(version, name, path), mode_(mode), uid_(uid), gid_(gid) {} + : controller_(version, 0, name, path), mode_(mode), uid_(uid), gid_(gid) {} + +void CgroupDescriptor::set_mounted(bool mounted) { + uint32_t flags = controller_.flags(); + if (mounted) { + flags |= CGROUPRC_CONTROLLER_FLAG_MOUNTED; + } else { + flags &= ~CGROUPRC_CONTROLLER_FLAG_MOUNTED; + } + controller_.set_flags(flags); +} } // namespace cgrouprc } // namespace android @@ -296,10 +307,11 @@ bool CgroupSetup() { } // setup cgroups - for (const auto& [name, descriptor] : descriptors) { - if (!SetupCgroup(descriptor)) { + for (auto& [name, descriptor] : descriptors) { + if (SetupCgroup(descriptor)) { + descriptor.set_mounted(true); + } else { // issue a warning and proceed with the next cgroup - // TODO: mark the descriptor as invalid and skip it in WriteRcFile() LOG(WARNING) << "Failed to setup " << name << " cgroup"; } }