From: Michael Tremer Date: Mon, 9 Dec 2024 16:17:46 +0000 (+0000) Subject: cgroups: Remove the overly complicated handling of controllers X-Git-Tag: 0.9.30~740 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c2ad23f2573f2bcb23ba8a56f58f00fd2e188448;p=pakfire.git cgroups: Remove the overly complicated handling of controllers Signed-off-by: Michael Tremer --- diff --git a/src/libpakfire/build.c b/src/libpakfire/build.c index 5f9e2c524..6d205a3af 100644 --- a/src/libpakfire/build.c +++ b/src/libpakfire/build.c @@ -1348,8 +1348,7 @@ static int pakfire_build_setup_cgroup(struct pakfire_build* build) { } // Create a new cgroup - r = pakfire_cgroup_open(&build->cgroup, build->ctx, path, - PAKFIRE_CGROUP_ENABLE_ACCOUNTING); + r = pakfire_cgroup_open(&build->cgroup, build->ctx, path, 0); if (r) { ERROR(build->ctx, "Could not create cgroup for build %s: %m\n", build->_id); goto ERROR; diff --git a/src/libpakfire/cgroup.c b/src/libpakfire/cgroup.c index bf12b5b98..1550c93a5 100644 --- a/src/libpakfire/cgroup.c +++ b/src/libpakfire/cgroup.c @@ -57,19 +57,6 @@ .imm = 0 \ }) -enum pakfire_cgroup_controllers { - PAKFIRE_CGROUP_CONTROLLER_CPU = (1 << 0), - PAKFIRE_CGROUP_CONTROLLER_MEMORY = (1 << 1), - PAKFIRE_CGROUP_CONTROLLER_PIDS = (1 << 2), - PAKFIRE_CGROUP_CONTROLLER_IO = (1 << 3), -}; - -static const enum pakfire_cgroup_controllers pakfire_cgroup_accounting_controllers = - PAKFIRE_CGROUP_CONTROLLER_CPU | - PAKFIRE_CGROUP_CONTROLLER_MEMORY | - PAKFIRE_CGROUP_CONTROLLER_PIDS | - PAKFIRE_CGROUP_CONTROLLER_IO; - struct pakfire_cgroup { struct pakfire_ctx* ctx; int nrefs; @@ -95,10 +82,6 @@ static int pakfire_cgroup_is_root(struct pakfire_cgroup* cgroup) { return !*cgroup->path; } -static int pakfire_cgroup_has_flag(struct pakfire_cgroup* cgroup, int flag) { - return cgroup->flags & flag; -} - static int pakfire_cgroup_set_root(struct pakfire_cgroup* cgroup) { int r; @@ -131,44 +114,6 @@ static const char* pakfire_cgroup_name(struct pakfire_cgroup* cgroup) { return cgroup->path; } -static const char* pakfire_cgroup_controller_name( - enum pakfire_cgroup_controllers controller) { - switch (controller) { - case PAKFIRE_CGROUP_CONTROLLER_CPU: - return "cpu"; - - case PAKFIRE_CGROUP_CONTROLLER_MEMORY: - return "memory"; - - case PAKFIRE_CGROUP_CONTROLLER_PIDS: - return "pids"; - - case PAKFIRE_CGROUP_CONTROLLER_IO: - return "io"; - } - - return NULL; -} - -static enum pakfire_cgroup_controllers pakfire_cgroup_find_controller_by_name( - const char* name) { - const char* n = NULL; - - // Walk through the bitmap - for (unsigned int i = 1; i; i <<= 1) { - n = pakfire_cgroup_controller_name(i); - if (!n) - break; - - // Match - if (strcmp(name, n) == 0) - return i; - } - - // Nothing found - return 0; -} - static struct pakfire_cgroup* pakfire_cgroup_parent(struct pakfire_cgroup* cgroup) { struct pakfire_cgroup* parent = NULL; char path[PATH_MAX]; @@ -366,7 +311,7 @@ static int pakfire_cgroup_write(struct pakfire_cgroup* cgroup, if (fd < 0) { DEBUG(cgroup->ctx, "Could not open %s/%s for writing: %m\n", pakfire_cgroup_name(cgroup), path); - return 1; + return -errno; } // Write buffer @@ -378,7 +323,7 @@ static int pakfire_cgroup_write(struct pakfire_cgroup* cgroup, if (bytes_written < 0) { DEBUG(cgroup->ctx, "Could not write to %s/%s: %m\n", pakfire_cgroup_name(cgroup), path); - r = 1; + r = -errno; } // Close fd @@ -387,135 +332,37 @@ static int pakfire_cgroup_write(struct pakfire_cgroup* cgroup, return r; } -static int pakfire_cgroup_read_controllers( - struct pakfire_cgroup* cgroup, const char* name) { - char buffer[BUFFER_SIZE]; - char* p = NULL; - - // Discovered controllers - int controllers = 0; - - // Read cgroup.controllers file - ssize_t bytes_read = pakfire_cgroup_read(cgroup, name, buffer, sizeof(buffer)); - if (bytes_read < 0) - return -1; - - // If the file was empty, there is nothing more to do - if (bytes_read == 0) - return 0; - - char* token = strtok_r(buffer, " \n", &p); - - while (token) { - DEBUG(cgroup->ctx, "Found controller '%s'\n", token); - - // Try finding this controller - int controller = pakfire_cgroup_find_controller_by_name(token); - if (controller) - controllers |= controller; - - // Move on to next token - token = strtok_r(NULL, " \n", &p); - } - - // Return discovered controllers - return controllers; -} - /* - Returns a bitmap of all available controllers + Enables a cgroup controller. */ -static int pakfire_cgroup_available_controllers(struct pakfire_cgroup* cgroup) { - return pakfire_cgroup_read_controllers(cgroup, "cgroup.controllers"); -} +static int pakfire_cgroup_enable_controller(struct pakfire_cgroup* cgroup, const char* name) { + int r; -/* - Returns a bitmap of all enabled controllers -*/ -static int pakfire_cgroup_enabled_controllers(struct pakfire_cgroup* cgroup) { - return pakfire_cgroup_read_controllers(cgroup, "cgroup.subtree_control"); + // Try to enable the controller + r = pakfire_cgroup_write(cgroup, "cgroup.subtree_control", "+%s", name); + if (r < 0) + ERROR(cgroup->ctx, "Could not enable controller '%s': %s\n", name, strerror(-r)); + + return r; } /* - This function takes a bitmap of controllers that should be enabled. + Enables all controllers that we need. */ -static int pakfire_cgroup_enable_controllers(struct pakfire_cgroup* cgroup, - enum pakfire_cgroup_controllers controllers) { - struct pakfire_cgroup* parent = NULL; - int r = 1; - - // Find all enabled controllers - const int enabled_controllers = pakfire_cgroup_enabled_controllers(cgroup); - if (enabled_controllers < 0) { - ERROR(cgroup->ctx, "Could not fetch enabled controllers: %m\n"); - goto ERROR; - } - - // Filter out anything that is already enabled - controllers = (controllers & ~enabled_controllers); - - // Exit if everything is already enabled - if (!controllers) { - DEBUG(cgroup->ctx, "All controllers are already enabled\n"); - return 0; - } - - // Find all available controllers - const int available_controllers = pakfire_cgroup_available_controllers(cgroup); - if (available_controllers < 0) { - ERROR(cgroup->ctx, "Could not fetch available controllers: %m\n"); - goto ERROR; - } - - // Are all controllers we need available, yet? - if (controllers & ~available_controllers) { - DEBUG(cgroup->ctx, "Not all controllers are available, yet\n"); - - parent = pakfire_cgroup_parent(cgroup); - - // Enable everything we need on the parent group - if (parent) { - r = pakfire_cgroup_enable_controllers(parent, controllers); - if (r) - goto ERROR; - } - } - - // Determine how many iterations we will need - const int iterations = 1 << (sizeof(controllers) * 8 - __builtin_clz(controllers)); - - // Iterate over all known controllers - for (int controller = 1; controller < iterations; controller <<= 1) { - // Skip enabling this controller if not requested - if (!(controller & controllers)) - continue; - - // Fetch name - const char* name = pakfire_cgroup_controller_name(controller); - - DEBUG(cgroup->ctx, "Enabling controller %s in cgroup %s\n", - name, pakfire_cgroup_name(cgroup)); +static int pakfire_cgroup_enable_controllers(struct pakfire_cgroup* cgroup) { + static const char* controllers[] = { + "cpu", "memory", "pids", "io", NULL, + }; + int r; - // Try enabling the controller (this will succeed if it already is enabled) - r = pakfire_cgroup_write(cgroup, "cgroup.subtree_control", "+%s\n", name); - if (r) { - ERROR(cgroup->ctx, "Could not enable controller %s in cgroup %s\n", - name, pakfire_cgroup_name(cgroup)); - goto ERROR; - } + // Enable all controllers + for (const char** controller = controllers; *controller; controller++) { + r = pakfire_cgroup_enable_controller(cgroup, *controller); + if (r < 0) + return r; } -ERROR: - if (parent) - pakfire_cgroup_unref(parent); - - return r; -} - -static int pakfire_cgroup_enable_accounting(struct pakfire_cgroup* cgroup) { - // Enable all accounting controllers - return pakfire_cgroup_enable_controllers(cgroup, - pakfire_cgroup_accounting_controllers); + return 0; } /* @@ -562,12 +409,10 @@ int pakfire_cgroup_open(struct pakfire_cgroup** cgroup, goto ERROR; } - // Enable accounting if requested - if (pakfire_cgroup_has_flag(c, PAKFIRE_CGROUP_ENABLE_ACCOUNTING)) { - r = pakfire_cgroup_enable_accounting(c); - if (r) - goto ERROR; - } + // Enable all controllers + r = pakfire_cgroup_enable_controllers(c); + if (r < 0) + goto ERROR; // Setup the devices filter r = pakfire_cgroup_setup_devices(c); @@ -686,11 +531,6 @@ int pakfire_cgroup_fd(struct pakfire_cgroup* cgroup) { int pakfire_cgroup_set_guaranteed_memory(struct pakfire_cgroup* cgroup, size_t mem) { int r; - // Enable memory controller - r = pakfire_cgroup_enable_controllers(cgroup, PAKFIRE_CGROUP_CONTROLLER_MEMORY); - if (r) - return r; - DEBUG(cgroup->ctx, "%s: Setting guaranteed memory to %zu byte(s)\n", pakfire_cgroup_name(cgroup), mem); @@ -706,11 +546,6 @@ int pakfire_cgroup_set_guaranteed_memory(struct pakfire_cgroup* cgroup, size_t m int pakfire_cgroup_set_memory_limit(struct pakfire_cgroup* cgroup, size_t mem) { int r; - // Enable memory controller - r = pakfire_cgroup_enable_controllers(cgroup, PAKFIRE_CGROUP_CONTROLLER_MEMORY); - if (r) - return r; - DEBUG(cgroup->ctx, "%s: Setting memory limit to %zu byte(s)\n", pakfire_cgroup_name(cgroup), mem); @@ -728,11 +563,6 @@ int pakfire_cgroup_set_memory_limit(struct pakfire_cgroup* cgroup, size_t mem) { int pakfire_cgroup_set_pid_limit(struct pakfire_cgroup* cgroup, size_t limit) { int r; - // Enable PID controller - r = pakfire_cgroup_enable_controllers(cgroup, PAKFIRE_CGROUP_CONTROLLER_PIDS); - if (r) - return r; - DEBUG(cgroup->ctx, "%s: Setting PID limit to %zu\n", pakfire_cgroup_name(cgroup), limit); diff --git a/src/libpakfire/include/pakfire/cgroup.h b/src/libpakfire/include/pakfire/cgroup.h index 53621e88a..1b4b881bc 100644 --- a/src/libpakfire/include/pakfire/cgroup.h +++ b/src/libpakfire/include/pakfire/cgroup.h @@ -195,10 +195,6 @@ struct pakfire_cgroup_stats { } memory; }; -enum pakfire_cgroup_flags { - PAKFIRE_CGROUP_ENABLE_ACCOUNTING = (1 << 0), -}; - int pakfire_cgroup_open(struct pakfire_cgroup** cgroup, struct pakfire_ctx* ctx, const char* path, int flags); diff --git a/tests/libpakfire/cgroup.c b/tests/libpakfire/cgroup.c index 9cc411334..cb5d1de2a 100644 --- a/tests/libpakfire/cgroup.c +++ b/tests/libpakfire/cgroup.c @@ -57,14 +57,7 @@ static int test_stats(const struct test* t) { }; // Open the cgroup - ASSERT_SUCCESS(pakfire_cgroup_open(&cgroup, t->ctx, "pakfire-test", - PAKFIRE_CGROUP_ENABLE_ACCOUNTING)); - -#if 0 - // Enable CPU & Memory controllers - ASSERT_SUCCESS(pakfire_cgroup_enable_controllers(cgroup, - PAKFIRE_CGROUP_CONTROLLER_CPU|PAKFIRE_CGROUP_CONTROLLER_MEMORY)); -#endif + ASSERT_SUCCESS(pakfire_cgroup_open(&cgroup, t->ctx, "pakfire-test", 0)); // Create a new jail ASSERT_SUCCESS(pakfire_jail_create(&jail, t->pakfire));