libprocessgroup: Rework SetupCgroup()
Improve the readability of this function by splitting it.
This CL includes the following behavior changes:
- If changing the directory owner and/or mode fails for /sys/fs/cgroup,
this is considered as a fatal error instead of something that should
only fail if "Optional" has not been set.
- If mounting the v2 cgroup controller fails, this is considered as an
error.
- Activating/mounting a cgroup controller only fails if the controller
has not been marked as optional.
Bug: 213617178
Change-Id: If6908dfdbcb2e1c9637ab4ac8a7625f0a17dc9e0
Signed-off-by: Bart Van Assche <bvanassche@google.com>
diff --git a/libprocessgroup/setup/cgroup_map_write.cpp b/libprocessgroup/setup/cgroup_map_write.cpp
index 304248a..fbeedf9 100644
--- a/libprocessgroup/setup/cgroup_map_write.cpp
+++ b/libprocessgroup/setup/cgroup_map_write.cpp
@@ -254,86 +254,64 @@
// To avoid issues in sdk_mac build
#if defined(__ANDROID__)
-static bool SetupCgroup(const CgroupDescriptor& descriptor) {
+static bool IsOptionalController(const format::CgroupController* controller) {
+ return controller->flags() & CGROUPRC_CONTROLLER_FLAG_OPTIONAL;
+}
+
+static bool MountV2CgroupController(const CgroupDescriptor& descriptor) {
const format::CgroupController* controller = descriptor.controller();
- int result;
- if (controller->version() == 2) {
- result = 0;
- if (!strcmp(controller->name(), CGROUPV2_CONTROLLER_NAME)) {
- // /sys/fs/cgroup is created by cgroup2 with specific selinux permissions,
- // try to create again in case the mount point is changed
- if (!Mkdir(controller->path(), 0, "", "")) {
- LOG(ERROR) << "Failed to create directory for " << controller->name() << " cgroup";
- return false;
- }
+ // /sys/fs/cgroup is created by cgroup2 with specific selinux permissions,
+ // try to create again in case the mount point is changed
+ if (!Mkdir(controller->path(), 0, "", "")) {
+ LOG(ERROR) << "Failed to create directory for " << controller->name() << " cgroup";
+ return false;
+ }
- // The memory_recursiveprot mount option has been introduced by kernel commit
- // 8a931f801340 ("mm: memcontrol: recursive memory.low protection"; v5.7). Try first to
- // mount with that option enabled. If mounting fails because the kernel is too old,
- // retry without that mount option.
- if (mount("none", controller->path(), "cgroup2", MS_NODEV | MS_NOEXEC | MS_NOSUID,
- "memory_recursiveprot") < 0) {
- LOG(INFO) << "Mounting memcg with memory_recursiveprot failed. Retrying without.";
- if (mount("none", controller->path(), "cgroup2", MS_NODEV | MS_NOEXEC | MS_NOSUID,
- nullptr) < 0) {
- PLOG(ERROR) << "Failed to mount cgroup v2";
- }
- }
-
- // selinux permissions change after mounting, so it's ok to change mode and owner now
- if (!ChangeDirModeAndOwner(controller->path(), descriptor.mode(), descriptor.uid(),
- descriptor.gid())) {
- LOG(ERROR) << "Failed to create directory for " << controller->name() << " cgroup";
- result = -1;
- }
- } else {
- if (!Mkdir(controller->path(), descriptor.mode(), descriptor.uid(), descriptor.gid())) {
- LOG(ERROR) << "Failed to create directory for " << controller->name() << " cgroup";
- return false;
- }
-
- if (controller->flags() & CGROUPRC_CONTROLLER_FLAG_NEEDS_ACTIVATION) {
- std::string str = std::string("+") + controller->name();
- std::string path = std::string(controller->path()) + "/cgroup.subtree_control";
-
- if (!base::WriteStringToFile(str, path)) {
- LOG(ERROR) << "Failed to activate controller " << controller->name();
- return false;
- }
- }
- }
- } else {
- // mkdir <path> [mode] [owner] [group]
- if (!Mkdir(controller->path(), descriptor.mode(), descriptor.uid(), descriptor.gid())) {
- LOG(ERROR) << "Failed to create directory for " << controller->name() << " cgroup";
- return false;
- }
-
- // Unfortunately historically cpuset controller was mounted using a mount command
- // different from all other controllers. This results in controller attributes not
- // to be prepended with controller name. For example this way instead of
- // /dev/cpuset/cpuset.cpus the attribute becomes /dev/cpuset/cpus which is what
- // the system currently expects.
- if (!strcmp(controller->name(), "cpuset")) {
- // mount cpuset none /dev/cpuset nodev noexec nosuid
- result = mount("none", controller->path(), controller->name(),
- MS_NODEV | MS_NOEXEC | MS_NOSUID, nullptr);
- } else {
- // mount cgroup none <path> nodev noexec nosuid <controller>
- result = mount("none", controller->path(), "cgroup", MS_NODEV | MS_NOEXEC | MS_NOSUID,
- controller->name());
+ // The memory_recursiveprot mount option has been introduced by kernel commit
+ // 8a931f801340 ("mm: memcontrol: recursive memory.low protection"; v5.7). Try first to
+ // mount with that option enabled. If mounting fails because the kernel is too old,
+ // retry without that mount option.
+ if (mount("none", controller->path(), "cgroup2", MS_NODEV | MS_NOEXEC | MS_NOSUID,
+ "memory_recursiveprot") < 0) {
+ LOG(INFO) << "Mounting memcg with memory_recursiveprot failed. Retrying without.";
+ if (mount("none", controller->path(), "cgroup2", MS_NODEV | MS_NOEXEC | MS_NOSUID,
+ nullptr) < 0) {
+ PLOG(ERROR) << "Failed to mount cgroup v2";
+ return IsOptionalController(controller);
}
}
- if (result < 0) {
- bool optional = controller->flags() & CGROUPRC_CONTROLLER_FLAG_OPTIONAL;
+ // selinux permissions change after mounting, so it's ok to change mode and owner now
+ if (!ChangeDirModeAndOwner(controller->path(), descriptor.mode(), descriptor.uid(),
+ descriptor.gid())) {
+ PLOG(ERROR) << "Change of ownership or mode failed for controller " << controller->name();
+ return IsOptionalController(controller);
+ }
- if (optional && errno == EINVAL) {
- // Optional controllers are allowed to fail to mount if kernel does not support them
- LOG(INFO) << "Optional " << controller->name() << " cgroup controller is not mounted";
- } else {
- PLOG(ERROR) << "Failed to mount " << controller->name() << " cgroup";
+ return true;
+}
+
+static bool ActivateV2CgroupController(const CgroupDescriptor& descriptor) {
+ const format::CgroupController* controller = descriptor.controller();
+
+ if (!Mkdir(controller->path(), descriptor.mode(), descriptor.uid(), descriptor.gid())) {
+ LOG(ERROR) << "Failed to create directory for " << controller->name() << " cgroup";
+ return false;
+ }
+
+ if (controller->flags() & CGROUPRC_CONTROLLER_FLAG_NEEDS_ACTIVATION) {
+ std::string str = "+";
+ str += controller->name();
+ std::string path = controller->path();
+ path += "/cgroup.subtree_control";
+
+ if (!base::WriteStringToFile(str, path)) {
+ if (IsOptionalController(controller)) {
+ PLOG(INFO) << "Failed to activate optional controller " << controller->name();
+ return true;
+ }
+ PLOG(ERROR) << "Failed to activate controller " << controller->name();
return false;
}
}
@@ -341,6 +319,55 @@
return true;
}
+static bool MountV1CgroupController(const CgroupDescriptor& descriptor) {
+ const format::CgroupController* controller = descriptor.controller();
+
+ // mkdir <path> [mode] [owner] [group]
+ if (!Mkdir(controller->path(), descriptor.mode(), descriptor.uid(), descriptor.gid())) {
+ LOG(ERROR) << "Failed to create directory for " << controller->name() << " cgroup";
+ return false;
+ }
+
+ // Unfortunately historically cpuset controller was mounted using a mount command
+ // different from all other controllers. This results in controller attributes not
+ // to be prepended with controller name. For example this way instead of
+ // /dev/cpuset/cpuset.cpus the attribute becomes /dev/cpuset/cpus which is what
+ // the system currently expects.
+ int res;
+ if (!strcmp(controller->name(), "cpuset")) {
+ // mount cpuset none /dev/cpuset nodev noexec nosuid
+ res = mount("none", controller->path(), controller->name(),
+ MS_NODEV | MS_NOEXEC | MS_NOSUID, nullptr);
+ } else {
+ // mount cgroup none <path> nodev noexec nosuid <controller>
+ res = mount("none", controller->path(), "cgroup", MS_NODEV | MS_NOEXEC | MS_NOSUID,
+ controller->name());
+ }
+ if (res != 0) {
+ if (IsOptionalController(controller)) {
+ PLOG(INFO) << "Failed to mount optional controller " << controller->name();
+ return true;
+ }
+ PLOG(ERROR) << "Failed to mount controller " << controller->name();
+ return false;
+ }
+ return true;
+}
+
+static bool SetupCgroup(const CgroupDescriptor& descriptor) {
+ const format::CgroupController* controller = descriptor.controller();
+
+ if (controller->version() == 2) {
+ if (!strcmp(controller->name(), CGROUPV2_CONTROLLER_NAME)) {
+ return MountV2CgroupController(descriptor);
+ } else {
+ return ActivateV2CgroupController(descriptor);
+ }
+ } else {
+ return MountV1CgroupController(descriptor);
+ }
+}
+
#else
// Stubs for non-Android targets.