From a42a789d2970c9209936a1f6a8ca7a2ba3a9ab78 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Fri, 9 Apr 2021 17:49:55 +0200 Subject: [PATCH] Refactor, clean up removed top-level cgroups The main purpose of this patch is to remove top-level cgroups when they are remvoed from finit.conf. However, this is tricky, becasue there may still be supervised services (and children of them) running. We must postpone removal until a cgroup.events change. To make matters worse, events may arrive out-of-order, i.e. the last-child event for the parent cgroup before the sub-group. A spin-off from this patch is that we can now support arbitrarily long group names and group config. (Only applies to top-level cgroups, not run/task/services.) Limitations: - Processes in "removed" cgroups are not migrated/orphaned, the reasons are several: overhead and complexity and the fact that memory cannot be migrated - A lingering child of a moved service may prevent an old top-level cgroup from being removed. Signed-off-by: Joachim Wiberg --- src/cgroup.c | 196 +++++++++++++++++++++++++++++++++++++++++++++------ src/cgroup.h | 8 ++- src/conf.c | 68 ++++-------------- src/conf.h | 4 -- 4 files changed, 194 insertions(+), 82 deletions(-) diff --git a/src/cgroup.c b/src/cgroup.c index a2600107..5140f023 100644 --- a/src/cgroup.c +++ b/src/cgroup.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include /* get_nprocs_conf() */ @@ -34,6 +35,18 @@ #include "log.h" #include "util.h" +struct cg { + TAILQ_ENTRY(cg) link; + + char *name; /* top-level group name */ + char *cfg; /* kernel settings */ + + int active; /* for mark & sweep */ + int protected; /* for init/, user/, & system/ */ +}; + +static TAILQ_HEAD(, cg) cgroups = TAILQ_HEAD_INITIALIZER(cgroups); + static char controllers[256]; static struct iwatch iw_cgroup; @@ -205,7 +218,17 @@ static void cgroup_handle_event(char *event, uint32_t mask) ptr = strrchr(path, '/'); if (ptr) { *ptr = 0; - rmdir(path); + if (!cgroup_del(path)) { + /* + * try with parent, top-level group, we + * may get events out-of-order *sigh* + */ + ptr = strrchr(path, '/'); + if (!ptr) + break; + *ptr = 0; + cgroup_del(path); + } } break; @@ -255,6 +278,151 @@ static void cgroup_events_cb(uev_t *w, void *arg, int events) #endif } +static struct cg *cgroup_find(char *name) +{ + struct cg *cg; + + TAILQ_FOREACH(cg, &cgroups, link) { + if (strcmp(cg->name, name)) + continue; + + return cg; + } + + return NULL; +} + +/* + * Marks all unprotected cgroups for deletion (during reload) + */ +void cgroup_mark_all(void) +{ + struct cg *cg; + + TAILQ_FOREACH(cg, &cgroups, link) { + if (cg->protected) + continue; + + cg->active = 0; + } +} + +/* + * Remove (try to) all unused cgroups + */ +void cgroup_cleanup(void) +{ + struct cg *cg, *tmp; + char path[256]; + + TAILQ_FOREACH_SAFE(cg, &cgroups, link, tmp) { + if (cg->active) + continue; + + snprintf(path, sizeof(path), FINIT_CGPATH "/%s", cg->name); + cgroup_del(path); + } +} + +/* + * Add, or update, settings for top-level cgroup + */ +int cgroup_add(char *name, char *cfg, int protected) +{ + struct cg *cg; + + if (!name) + return -1; + if (!cfg) + cfg = ""; + + cg = cgroup_find(name); + if (!cg) { + cg = malloc(sizeof(struct cg)); + if (!cg) { + _pe("Failed allocating 'struct cg' for %s", name); + return -1; + } + cg->name = strdup(name); + if (!cg->name) { + _pe("Failed setting cgroup name %s", name); + free(cg); + return -1; + } + TAILQ_INSERT_TAIL(&cgroups, cg, link); + } else + free(cg->cfg); + + cg->cfg = strdup(cfg); + if (!cg->cfg) { + _pe("Failed add/update of cgroup %s", name); + TAILQ_REMOVE(&cgroups, cg, link); + free(cg->name); + free(cg); + return -1; + } + cg->protected = protected; + cg->active = 1; + + return 0; +} + +/* + * Remove inactive top-level cgroup + */ +int cgroup_del(char *dir) +{ + struct cg *cg; + char path[256]; + + TAILQ_FOREACH(cg, &cgroups, link) { + snprintf(path, sizeof(path), FINIT_CGPATH "/%s", cg->name); + if (strcmp(path, dir)) + continue; + + if (cg->active) + return -1; + + break; + } + + if (rmdir(dir) && errno != ENOENT) { + _d("Failed removing %s: %s", dir, strerror(errno)); + return -1; + } + + if (cg) { + TAILQ_REMOVE(&cgroups, cg, link); + free(cg->name); + free(cg->cfg); + free(cg); + } + + return 0; +} + +/* the top-level init cgroup is a leaf, that's ensured in cgroup_init() */ +void cgroup_config(void) +{ + struct cg *cg; + + TAILQ_FOREACH(cg, &cgroups, link) { + char path[256]; + int leaf = 0; + + if (!cg->active) + continue; + if (!strcmp(cg->name, "init")) + leaf = 1; /* reserved */ + + snprintf(path, sizeof(path), "%s/%s", FINIT_CGPATH, cg->name); + group_init(path, leaf, cg->cfg); + + strlcat(path, "/cgroup.events", sizeof(path)); + iwatch_add(&iw_cgroup, path, 0); + } +} + /* * Called by Finit at early boot to mount initial cgroups */ @@ -293,10 +461,11 @@ void cgroup_init(uev_ctx_t *ctx) if (fnwrite(controllers, FINIT_CGPATH "/cgroup.subtree_control")) _pe("Failed enabling %s for %s", controllers, FINIT_CGPATH "/cgroup.subtree_control"); - /* Default groups, PID 1, services, and user/login processes */ - group_init(FINIT_CGPATH "/init", 1, "cpu.weight:100"); - group_init(FINIT_CGPATH "/user", 0, "cpu.weight:100"); - group_init(FINIT_CGPATH "/system", 0, "cpu.weight:9800"); + /* Default (protected) groups, PID 1, services, and user/login processes */ + cgroup_add("init", "cpu.weight:100", 1); + cgroup_add("system", "cpu.weight:9800", 1); + cgroup_add("user", "cpu.weight:100", 1); + cgroup_config(); /* Move ourselves to init (best effort, otherwise run in 'root' group */ if (fnwrite("1", FINIT_CGPATH "/init/cgroup.procs")) @@ -310,23 +479,6 @@ void cgroup_init(uev_ctx_t *ctx) } } -/* the top-level init cgroup is a leaf, that's ensured in cgroup_init() */ -void cgroup_config(size_t num, struct cgroup cg[]) -{ - size_t i; - - for (i = 0; i < num; i++) { - char path[256]; - int leaf = 0; - - if (!strcmp(cg[i].name, "init")) - leaf = 1; /* reserved */ - - snprintf(path, sizeof(path), "%s/%s", FINIT_CGPATH, cg[i].name); - group_init(path, leaf, cg[i].cfg); - } -} - /** * Local Variables: * indent-tabs-mode: t diff --git a/src/cgroup.h b/src/cgroup.h index 58e8b45a..f439dd47 100644 --- a/src/cgroup.h +++ b/src/cgroup.h @@ -31,8 +31,14 @@ struct cgroup { char cfg[128]; }; +void cgroup_mark_all(void); +void cgroup_cleanup (void); + +int cgroup_add (char *name, char *cfg, int protected); +int cgroup_del (char *dir); +void cgroup_config (void); + void cgroup_init (uev_ctx_t *ctx); -void cgroup_config (size_t num, struct cgroup cg[]); int cgroup_user (char *name, int pid); int cgroup_service (char *name, int pid, struct cgroup *cg); diff --git a/src/conf.c b/src/conf.c index 96af6671..123a7a18 100644 --- a/src/conf.c +++ b/src/conf.c @@ -51,8 +51,6 @@ int logfile_count_max = 5; struct rlimit initial_rlimit[RLIMIT_NLIMITS]; struct rlimit global_rlimit[RLIMIT_NLIMITS]; -struct cgroup cgroups[8]; -size_t cgroups_num; char cgroup_current[16]; /* cgroup.NAME sets current cgroup for a set of services */ struct conf_change { @@ -428,65 +426,27 @@ error: logit(LOG_WARNING, "rlimit: parse error"); } -/* reset defaults */ -static void init_cgroups(void) -{ - struct cgroup init[3] = { - { .name = "init", .cfg = "cpu.weight:100" }, - { .name = "user", .cfg = "cpu.weight:100" }, - { .name = "system", .cfg = "cpu.weight:9800" }, - }; - size_t i; - - for (i = 0; i < NELEMS(init); i++) - memcpy(&cgroups[i], &init[i], sizeof(struct cgroup)); - - cgroups_num = i; -} - -struct cgroup *conf_cgfind(char *name) -{ - size_t i; - - for (i = 0; i < cgroups_num; i++) { - if (strcmp(cgroups[i].name, name)) - continue; - - return &cgroups[i]; - } - - return NULL; -} - /* cgroup NAME ctrl.prop:value,ctrl.prop:value ... */ static void conf_parse_cgroup(char *line) { - struct cgroup *cg; + char config[strlen(line)]; char *ptr, *name; name = strtok(line, " \t"); if (!name) return; - cg = conf_cgfind(name); - if (!cg) { - if (strstr(name, "..") || strchr(name, '/')) - return; /* illegal */ - if (cgroups_num + 1 == NELEMS(cgroups)) - return; /* exhausted */ + if (strstr(name, "..") || strchr(name, '/')) + return; /* illegal */ - cg = &cgroups[cgroups_num++]; - strlcpy(cg->name, name, sizeof(cg->name)); - } - - cg->cfg[0] = 0; + config[0] = 0; while ((ptr = strtok(NULL, " \t"))) { - if (cg->cfg[0]) - strlcat(cg->cfg, ",", sizeof(cg->cfg)); - strlcat(cg->cfg, ptr, sizeof(cg->cfg)); + if (config[0]) + strlcat(config, ",", sizeof(config)); + strlcat(config, ptr, sizeof(config)); } - _d("cg->cfg: %s", cg->cfg); + cgroup_add(name, config, 0); } static void parse_static(char *line, int is_rcsd) @@ -702,6 +662,7 @@ int conf_reload(void) daylight, timezone, tzname[0], tzname[1]); /* Mark and sweep */ + cgroup_mark_all(); svc_mark_dynamic(); tty_mark(); @@ -710,9 +671,6 @@ int conf_reload(void) */ memcpy(global_rlimit, initial_rlimit, sizeof(global_rlimit)); - /* Initialize default cgroups */ - init_cgroups(); - if (rescue) { int rc; char line[80] = "tty [12345] @console noclear nologin"; @@ -784,8 +742,11 @@ int conf_reload(void) service_update_rdeps(); /* Set up top-level cgroups */ - cgroup_config(cgroups_num, cgroups); + cgroup_config(); done: + /* Remove all unused top-level cgroups */ + cgroup_cleanup(); + /* Drop record of all .conf changes */ drop_changes(); @@ -997,9 +958,6 @@ int conf_init(uev_ctx_t *ctx) /* Initialize global rlimits, e.g. for built-in services */ memcpy(global_rlimit, initial_rlimit, sizeof(global_rlimit)); - /* Initialize default cgroups */ - init_cgroups(); - /* Read global rlimits and global cgroup setup from /etc/finit.conf */ parse_conf(FINIT_CONF, 0); diff --git a/src/conf.h b/src/conf.h index 0b217fb6..1973203f 100644 --- a/src/conf.h +++ b/src/conf.h @@ -31,15 +31,11 @@ extern int logfile_size_max; extern int logfile_count_max; extern struct rlimit global_rlimit[]; -extern struct cgroup cgroups[]; -extern size_t cgroup_num; extern char cgroup_current[]; int str2rlim(char *str); char *rlim2str(int rlim); -struct cgroup *conf_cgfind(char *name); - int conf_init (uev_ctx_t *ctx); void conf_reload (void); int conf_any_change (void);