From 94f0b13b1623c083ece9c0381bd1e65500dc1363 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 6 Oct 2023 18:13:16 +0200 Subject: [PATCH 1/6] cgroup: rename TasksMax structure to CGroupTasksMax Almost all our enums/structs/funcs carry the CGroup prefix if they are defined in cgroup.h, TasksMax so far does not, even though it is exclusively used in cgroup context. Change that. --- src/core/cgroup.c | 16 ++++++++-------- src/core/cgroup.h | 12 ++++++------ src/core/dbus-cgroup.c | 10 +++++----- src/core/load-fragment.c | 10 +++++----- src/core/manager.c | 2 +- src/core/manager.h | 2 +- src/test/test-cgroup-mask.c | 2 +- 7 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 29905b08ad..56c90f0460 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -52,7 +52,7 @@ * out specific attributes from us. */ #define LOG_LEVEL_CGROUP_WRITE(r) (IN_SET(abs(r), ENOENT, EROFS, EACCES, EPERM) ? LOG_DEBUG : LOG_WARNING) -uint64_t tasks_max_resolve(const TasksMax *tasks_max) { +uint64_t cgroup_tasks_max_resolve(const CGroupTasksMax *tasks_max) { if (tasks_max->scale == 0) return tasks_max->value; @@ -171,7 +171,7 @@ void cgroup_context_init(CGroupContext *c) { .blockio_weight = CGROUP_BLKIO_WEIGHT_INVALID, .startup_blockio_weight = CGROUP_BLKIO_WEIGHT_INVALID, - .tasks_max = TASKS_MAX_UNSET, + .tasks_max = CGROUP_TASKS_MAX_UNSET, .moom_swap = MANAGED_OOM_AUTO, .moom_mem_pressure = MANAGED_OOM_AUTO, @@ -561,7 +561,7 @@ void cgroup_context_dump(Unit *u, FILE* f, const char *prefix) { prefix, c->memory_zswap_max, format_cgroup_memory_limit_comparison(cdj, sizeof(cdj), u, "MemoryZSwapMax"), prefix, c->startup_memory_zswap_max, format_cgroup_memory_limit_comparison(cdk, sizeof(cdk), u, "StartupMemoryZSwapMax"), prefix, c->memory_limit, - prefix, tasks_max_resolve(&c->tasks_max), + prefix, cgroup_tasks_max_resolve(&c->tasks_max), prefix, cgroup_device_policy_to_string(c->device_policy), prefix, strempty(disable_controllers_str), prefix, delegate_str, @@ -1816,9 +1816,9 @@ static void cgroup_context_apply( * which is desirable so that there's an official way to release control of the sysctl from * systemd: set the limit to unbounded and reload. */ - if (tasks_max_isset(&c->tasks_max)) { + if (cgroup_tasks_max_isset(&c->tasks_max)) { u->manager->sysctl_pid_max_changed = true; - r = procfs_tasks_set_limit(tasks_max_resolve(&c->tasks_max)); + r = procfs_tasks_set_limit(cgroup_tasks_max_resolve(&c->tasks_max)); } else if (u->manager->sysctl_pid_max_changed) r = procfs_tasks_set_limit(TASKS_MAX); else @@ -1831,10 +1831,10 @@ static void cgroup_context_apply( /* The attribute itself is not available on the host root cgroup, and in the container case we want to * leave it for the container manager. */ if (!is_local_root) { - if (tasks_max_isset(&c->tasks_max)) { + if (cgroup_tasks_max_isset(&c->tasks_max)) { char buf[DECIMAL_STR_MAX(uint64_t) + 1]; - xsprintf(buf, "%" PRIu64 "\n", tasks_max_resolve(&c->tasks_max)); + xsprintf(buf, "%" PRIu64 "\n", cgroup_tasks_max_resolve(&c->tasks_max)); (void) set_attribute_and_warn(u, "pids", "pids.max", buf); } else (void) set_attribute_and_warn(u, "pids", "pids.max", "max\n"); @@ -1952,7 +1952,7 @@ static CGroupMask unit_get_cgroup_mask(Unit *u) { mask |= CGROUP_MASK_DEVICES | CGROUP_MASK_BPF_DEVICES; if (c->tasks_accounting || - tasks_max_isset(&c->tasks_max)) + cgroup_tasks_max_isset(&c->tasks_max)) mask |= CGROUP_MASK_PIDS; return CGROUP_MASK_EXTEND_JOINED(mask); diff --git a/src/core/cgroup.h b/src/core/cgroup.h index 05dfd48277..0d113a3de9 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -11,20 +11,20 @@ #include "pidref.h" #include "time-util.h" -typedef struct TasksMax { +typedef struct CGroupTasksMax { /* If scale == 0, just use value; otherwise, value / scale. * See tasks_max_resolve(). */ uint64_t value; uint64_t scale; -} TasksMax; +} CGroupTasksMax; -#define TASKS_MAX_UNSET ((TasksMax) { .value = UINT64_MAX, .scale = 0 }) +#define CGROUP_TASKS_MAX_UNSET ((CGroupTasksMax) { .value = UINT64_MAX, .scale = 0 }) -static inline bool tasks_max_isset(const TasksMax *tasks_max) { +static inline bool cgroup_tasks_max_isset(const CGroupTasksMax *tasks_max) { return tasks_max->value != UINT64_MAX || tasks_max->scale != 0; } -uint64_t tasks_max_resolve(const TasksMax *tasks_max); +uint64_t cgroup_tasks_max_resolve(const CGroupTasksMax *tasks_max); typedef struct CGroupContext CGroupContext; typedef struct CGroupDeviceAllow CGroupDeviceAllow; @@ -212,7 +212,7 @@ struct CGroupContext { LIST_HEAD(CGroupSocketBindItem, socket_bind_deny); /* Common */ - TasksMax tasks_max; + CGroupTasksMax tasks_max; /* Settings for systemd-oomd */ ManagedOOMMode moom_swap; diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 5dd34d1c20..32f93b8e8d 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -27,7 +27,7 @@ #include "percent-util.h" #include "socket-util.h" -BUS_DEFINE_PROPERTY_GET(bus_property_get_tasks_max, "t", TasksMax, tasks_max_resolve); +BUS_DEFINE_PROPERTY_GET(bus_property_get_tasks_max, "t", CGroupTasksMax, cgroup_tasks_max_resolve); BUS_DEFINE_PROPERTY_GET_ENUM(bus_property_get_cgroup_pressure_watch, cgroup_pressure_watch, CGroupPressureWatch); static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_cgroup_device_policy, cgroup_device_policy, CGroupDevicePolicy); @@ -1025,7 +1025,7 @@ static int bus_cgroup_set_cpu_weight( static int bus_cgroup_set_tasks_max( Unit *u, const char *name, - TasksMax *p, + CGroupTasksMax *p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error) { @@ -1044,7 +1044,7 @@ static int bus_cgroup_set_tasks_max( "Value specified in %s is out of range", name); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - *p = (TasksMax) { .value = v, .scale = 0 }; /* When .scale==0, .value is the absolute value */ + *p = (CGroupTasksMax) { .value = v, .scale = 0 }; /* When .scale==0, .value is the absolute value */ unit_invalidate_cgroup(u, CGROUP_MASK_PIDS); if (v == CGROUP_LIMIT_MAX) @@ -1061,7 +1061,7 @@ static int bus_cgroup_set_tasks_max( static int bus_cgroup_set_tasks_max_scale( Unit *u, const char *name, - TasksMax *p, + CGroupTasksMax *p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error) { @@ -1080,7 +1080,7 @@ static int bus_cgroup_set_tasks_max_scale( "Value specified in %s is out of range", name); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - *p = (TasksMax) { v, UINT32_MAX }; /* .scale is not 0, so this is interpreted as v/UINT32_MAX. */ + *p = (CGroupTasksMax) { v, UINT32_MAX }; /* .scale is not 0, so this is interpreted as v/UINT32_MAX. */ unit_invalidate_cgroup(u, CGROUP_MASK_PIDS); uint32_t scaled = DIV_ROUND_UP((uint64_t) v * 100U, (uint64_t) UINT32_MAX); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 935a4ef35d..1d230080bc 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3917,23 +3917,23 @@ int config_parse_tasks_max( void *userdata) { const Unit *u = userdata; - TasksMax *tasks_max = data; + CGroupTasksMax *tasks_max = data; uint64_t v; int r; if (isempty(rvalue)) { - *tasks_max = u ? u->manager->defaults.tasks_max : TASKS_MAX_UNSET; + *tasks_max = u ? u->manager->defaults.tasks_max : CGROUP_TASKS_MAX_UNSET; return 0; } if (streq(rvalue, "infinity")) { - *tasks_max = TASKS_MAX_UNSET; + *tasks_max = CGROUP_TASKS_MAX_UNSET; return 0; } r = parse_permyriad(rvalue); if (r >= 0) - *tasks_max = (TasksMax) { r, 10000U }; /* r‱ */ + *tasks_max = (CGroupTasksMax) { r, 10000U }; /* r‱ */ else { r = safe_atou64(rvalue, &v); if (r < 0) { @@ -3946,7 +3946,7 @@ int config_parse_tasks_max( return 0; } - *tasks_max = (TasksMax) { v }; + *tasks_max = (CGroupTasksMax) { v }; } return 0; diff --git a/src/core/manager.c b/src/core/manager.c index 07a95fe752..3068f633d7 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -112,7 +112,7 @@ /* How many units and jobs to process of the bus queue before returning to the event loop. */ #define MANAGER_BUS_MESSAGE_BUDGET 100U -#define DEFAULT_TASKS_MAX ((TasksMax) { 15U, 100U }) /* 15% */ +#define DEFAULT_TASKS_MAX ((CGroupTasksMax) { 15U, 100U }) /* 15% */ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata); static int manager_dispatch_cgroups_agent_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata); diff --git a/src/core/manager.h b/src/core/manager.h index 05697ce4e8..4fa54bb3c1 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -167,7 +167,7 @@ typedef struct UnitDefaults { bool tasks_accounting; bool ip_accounting; - TasksMax tasks_max; + CGroupTasksMax tasks_max; usec_t timer_accuracy_usec; OOMPolicy oom_policy; diff --git a/src/test/test-cgroup-mask.c b/src/test/test-cgroup-mask.c index 9ab253a65c..9d7c29f348 100644 --- a/src/test/test-cgroup-mask.c +++ b/src/test/test-cgroup-mask.c @@ -58,7 +58,7 @@ TEST_RET(cgroup_mask, .sd_booted = true) { m->defaults.blockio_accounting = m->defaults.io_accounting = m->defaults.tasks_accounting = false; - m->defaults.tasks_max = TASKS_MAX_UNSET; + m->defaults.tasks_max = CGROUP_TASKS_MAX_UNSET; assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); From c6f2dca68c698065df7ba7bf44a5b01aa710427e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 6 Oct 2023 18:15:56 +0200 Subject: [PATCH 2/6] cgroup: rename cgroup_add_* to cgroup_context_* They add settings to a CGroupContext, hence give them the expected context, to make clear they do not operate on anything else than the structure (i.e. not on a kernel cgroup or so). --- src/core/cgroup.c | 4 ++-- src/core/cgroup.h | 4 ++-- src/core/dbus-cgroup.c | 2 +- src/core/load-fragment.c | 4 ++-- src/core/unit.c | 8 ++++---- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 56c90f0460..f982526f26 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -695,7 +695,7 @@ void cgroup_context_dump_socket_bind_item(const CGroupSocketBindItem *item, FILE } } -int cgroup_add_device_allow(CGroupContext *c, const char *dev, const char *mode) { +int cgroup_context_add_device_allow(CGroupContext *c, const char *dev, const char *mode) { _cleanup_free_ CGroupDeviceAllow *a = NULL; _cleanup_free_ char *d = NULL; @@ -724,7 +724,7 @@ int cgroup_add_device_allow(CGroupContext *c, const char *dev, const char *mode) return 0; } -int cgroup_add_bpf_foreign_program(CGroupContext *c, uint32_t attach_type, const char *bpffs_path) { +int cgroup_context_add_bpf_foreign_program(CGroupContext *c, uint32_t attach_type, const char *bpffs_path) { CGroupBPFForeignProgram *p; _cleanup_free_ char *d = NULL; diff --git a/src/core/cgroup.h b/src/core/cgroup.h index 0d113a3de9..abc8957902 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -275,8 +275,8 @@ static inline bool cgroup_context_want_memory_pressure(const CGroupContext *c) { (c->memory_pressure_watch == CGROUP_PRESSURE_WATCH_AUTO && c->memory_accounting); } -int cgroup_add_device_allow(CGroupContext *c, const char *dev, const char *mode); -int cgroup_add_bpf_foreign_program(CGroupContext *c, uint32_t attach_type, const char *path); +int cgroup_context_add_device_allow(CGroupContext *c, const char *dev, const char *mode); +int cgroup_context_add_bpf_foreign_program(CGroupContext *c, uint32_t attach_type, const char *path); void cgroup_oomd_xattr_apply(Unit *u, const char *cgroup_path); int cgroup_log_xattr_apply(Unit *u, const char *cgroup_path); diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 32f93b8e8d..d38e8a6aeb 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -746,7 +746,7 @@ static int bus_cgroup_set_transient_property( name); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - r = cgroup_add_bpf_foreign_program(c, attach_type, p); + r = cgroup_context_add_bpf_foreign_program(c, attach_type, p); if (r < 0) return r; } diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 1d230080bc..3b737bf2f3 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -4195,7 +4195,7 @@ int config_parse_device_allow( return 0; } - return cgroup_add_device_allow(c, resolved, p); + return cgroup_context_add_device_allow(c, resolved, p); } int config_parse_io_device_weight( @@ -5978,7 +5978,7 @@ int config_parse_bpf_foreign_program( if (r < 0) return 0; - r = cgroup_add_bpf_foreign_program(c, attach_type, resolved); + r = cgroup_context_add_bpf_foreign_program(c, attach_type, resolved); if (r < 0) return log_error_errno(r, "Failed to add foreign BPF program to cgroup context: %m"); diff --git a/src/core/unit.c b/src/core/unit.c index 2d096cbaef..eed092b4dc 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4385,12 +4385,12 @@ int unit_patch_contexts(Unit *u) { /* When RootImage= or MountImages= is specified, the following devices are touched. */ FOREACH_STRING(p, "/dev/loop-control", "/dev/mapper/control") { - r = cgroup_add_device_allow(cc, p, "rw"); + r = cgroup_context_add_device_allow(cc, p, "rw"); if (r < 0) return r; } FOREACH_STRING(p, "block-loop", "block-blkext", "block-device-mapper") { - r = cgroup_add_device_allow(cc, p, "rwm"); + r = cgroup_context_add_device_allow(cc, p, "rwm"); if (r < 0) return r; } @@ -4405,14 +4405,14 @@ int unit_patch_contexts(Unit *u) { } if (ec->protect_clock) { - r = cgroup_add_device_allow(cc, "char-rtc", "r"); + r = cgroup_context_add_device_allow(cc, "char-rtc", "r"); if (r < 0) return r; } /* If there are encrypted credentials we might need to access the TPM. */ if (exec_context_has_encrypted_credentials(ec)) { - r = cgroup_add_device_allow(cc, "char-tpm", "rw"); + r = cgroup_context_add_device_allow(cc, "char-tpm", "rw"); if (r < 0) return r; } From 17d047f5070edea4426af75ec6f038000adada59 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 6 Oct 2023 18:20:08 +0200 Subject: [PATCH 3/6] cgroup: drop cgroup path parameter from xattr calls We only pass the same thing there: u->cgroup_path or NULL (which is ultimately the same as u->cgroup_path). Hence let's simplify things, and simply drop the whole parameter, and imply u->cgroup_pat. --- src/core/cgroup.c | 60 ++++++++++++++++++++--------------------------- src/core/cgroup.h | 4 ++-- 2 files changed, 28 insertions(+), 36 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index f982526f26..39e670091f 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -782,43 +782,35 @@ UNIT_DEFINE_ANCESTOR_MEMORY_LOOKUP(memory_low); UNIT_DEFINE_ANCESTOR_MEMORY_LOOKUP(startup_memory_low); UNIT_DEFINE_ANCESTOR_MEMORY_LOOKUP(memory_min); -static void unit_set_xattr_graceful(Unit *u, const char *cgroup_path, const char *name, const void *data, size_t size) { +static void unit_set_xattr_graceful(Unit *u, const char *name, const void *data, size_t size) { int r; assert(u); assert(name); - if (!cgroup_path) { - if (!u->cgroup_path) - return; + if (!u->cgroup_path) + return; - cgroup_path = u->cgroup_path; - } - - r = cg_set_xattr(cgroup_path, name, data, size, 0); + r = cg_set_xattr(u->cgroup_path, name, data, size, 0); if (r < 0) - log_unit_debug_errno(u, r, "Failed to set '%s' xattr on control group %s, ignoring: %m", name, empty_to_root(cgroup_path)); + log_unit_debug_errno(u, r, "Failed to set '%s' xattr on control group %s, ignoring: %m", name, empty_to_root(u->cgroup_path)); } -static void unit_remove_xattr_graceful(Unit *u, const char *cgroup_path, const char *name) { +static void unit_remove_xattr_graceful(Unit *u, const char *name) { int r; assert(u); assert(name); - if (!cgroup_path) { - if (!u->cgroup_path) - return; + if (!u->cgroup_path) + return; - cgroup_path = u->cgroup_path; - } - - r = cg_remove_xattr(cgroup_path, name); + r = cg_remove_xattr(u->cgroup_path, name); if (r < 0 && !ERRNO_IS_XATTR_ABSENT(r)) - log_unit_debug_errno(u, r, "Failed to remove '%s' xattr flag on control group %s, ignoring: %m", name, empty_to_root(cgroup_path)); + log_unit_debug_errno(u, r, "Failed to remove '%s' xattr flag on control group %s, ignoring: %m", name, empty_to_root(u->cgroup_path)); } -void cgroup_oomd_xattr_apply(Unit *u, const char *cgroup_path) { +void cgroup_oomd_xattr_apply(Unit *u) { CGroupContext *c; assert(u); @@ -828,19 +820,19 @@ void cgroup_oomd_xattr_apply(Unit *u, const char *cgroup_path) { return; if (c->moom_preference == MANAGED_OOM_PREFERENCE_OMIT) - unit_set_xattr_graceful(u, cgroup_path, "user.oomd_omit", "1", 1); + unit_set_xattr_graceful(u, "user.oomd_omit", "1", 1); if (c->moom_preference == MANAGED_OOM_PREFERENCE_AVOID) - unit_set_xattr_graceful(u, cgroup_path, "user.oomd_avoid", "1", 1); + unit_set_xattr_graceful(u, "user.oomd_avoid", "1", 1); if (c->moom_preference != MANAGED_OOM_PREFERENCE_AVOID) - unit_remove_xattr_graceful(u, cgroup_path, "user.oomd_avoid"); + unit_remove_xattr_graceful(u, "user.oomd_avoid"); if (c->moom_preference != MANAGED_OOM_PREFERENCE_OMIT) - unit_remove_xattr_graceful(u, cgroup_path, "user.oomd_omit"); + unit_remove_xattr_graceful(u, "user.oomd_omit"); } -int cgroup_log_xattr_apply(Unit *u, const char *cgroup_path) { +int cgroup_log_xattr_apply(Unit *u) { ExecContext *c; size_t len, allowed_patterns_len, denied_patterns_len; _cleanup_free_ char *patterns = NULL, *allowed_patterns = NULL, *denied_patterns = NULL; @@ -856,7 +848,7 @@ int cgroup_log_xattr_apply(Unit *u, const char *cgroup_path) { return 0; if (set_isempty(c->log_filter_allowed_patterns) && set_isempty(c->log_filter_denied_patterns)) { - unit_remove_xattr_graceful(u, cgroup_path, "user.journald_log_filter_patterns"); + unit_remove_xattr_graceful(u, "user.journald_log_filter_patterns"); return 0; } @@ -881,7 +873,7 @@ int cgroup_log_xattr_apply(Unit *u, const char *cgroup_path) { *(last++) = '\xff'; memcpy_safe(last, denied_patterns, denied_patterns_len); - unit_set_xattr_graceful(u, cgroup_path, "user.journald_log_filter_patterns", patterns, len); + unit_set_xattr_graceful(u, "user.journald_log_filter_patterns", patterns, len); return 0; } @@ -893,8 +885,8 @@ static void cgroup_xattr_apply(Unit *u) { assert(u); /* The 'user.*' xattrs can be set from a user manager. */ - cgroup_oomd_xattr_apply(u, u->cgroup_path); - cgroup_log_xattr_apply(u, u->cgroup_path); + cgroup_oomd_xattr_apply(u); + cgroup_log_xattr_apply(u); if (!MANAGER_IS_SYSTEM(u->manager)) return; @@ -902,9 +894,9 @@ static void cgroup_xattr_apply(Unit *u) { b = !sd_id128_is_null(u->invocation_id); FOREACH_STRING(xn, "trusted.invocation_id", "user.invocation_id") { if (b) - unit_set_xattr_graceful(u, NULL, xn, SD_ID128_TO_STRING(u->invocation_id), 32); + unit_set_xattr_graceful(u, xn, SD_ID128_TO_STRING(u->invocation_id), 32); else - unit_remove_xattr_graceful(u, NULL, xn); + unit_remove_xattr_graceful(u, xn); } /* Indicate on the cgroup whether delegation is on, via an xattr. This is best-effort, as old kernels @@ -918,9 +910,9 @@ static void cgroup_xattr_apply(Unit *u) { b = unit_cgroup_delegate(u); FOREACH_STRING(xn, "trusted.delegate", "user.delegate") { if (b) - unit_set_xattr_graceful(u, NULL, xn, "1", 1); + unit_set_xattr_graceful(u, xn, "1", 1); else - unit_remove_xattr_graceful(u, NULL, xn); + unit_remove_xattr_graceful(u, xn); } if (u->survive_final_kill_signal) { @@ -945,8 +937,8 @@ static void cgroup_xattr_apply(Unit *u) { "group %s, ignoring: %m", empty_to_root(u->cgroup_path)); } else { - unit_remove_xattr_graceful(u, /* cgroup_path= */ NULL, "user.survive_final_kill_signal"); - unit_remove_xattr_graceful(u, /* cgroup_path= */ NULL, "trusted.survive_final_kill_signal"); + unit_remove_xattr_graceful(u, "user.survive_final_kill_signal"); + unit_remove_xattr_graceful(u, "trusted.survive_final_kill_signal"); } } diff --git a/src/core/cgroup.h b/src/core/cgroup.h index abc8957902..868d83f43b 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -278,8 +278,8 @@ static inline bool cgroup_context_want_memory_pressure(const CGroupContext *c) { int cgroup_context_add_device_allow(CGroupContext *c, const char *dev, const char *mode); int cgroup_context_add_bpf_foreign_program(CGroupContext *c, uint32_t attach_type, const char *path); -void cgroup_oomd_xattr_apply(Unit *u, const char *cgroup_path); -int cgroup_log_xattr_apply(Unit *u, const char *cgroup_path); +void cgroup_oomd_xattr_apply(Unit *u); +int cgroup_log_xattr_apply(Unit *u); void cgroup_modify_nft_set(Unit *u, bool add); From d46510de2fe7ce23a17efab6a881b53b7f4a9b38 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 6 Oct 2023 18:24:46 +0200 Subject: [PATCH 4/6] cgroup: refactor cgroup_xattr_apply() Split the function up, so that each set of xattrs is applied separately. --- src/core/cgroup.c | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 39e670091f..5f8407c480 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -878,19 +878,11 @@ int cgroup_log_xattr_apply(Unit *u) { return 0; } -static void cgroup_xattr_apply(Unit *u) { +static void cgroup_invocation_id_xattr_apply(Unit *u) { bool b; - int r; assert(u); - /* The 'user.*' xattrs can be set from a user manager. */ - cgroup_oomd_xattr_apply(u); - cgroup_log_xattr_apply(u); - - if (!MANAGER_IS_SYSTEM(u->manager)) - return; - b = !sd_id128_is_null(u->invocation_id); FOREACH_STRING(xn, "trusted.invocation_id", "user.invocation_id") { if (b) @@ -898,6 +890,12 @@ static void cgroup_xattr_apply(Unit *u) { else unit_remove_xattr_graceful(u, xn); } +} + +static void cgroup_delegate_xattr_apply(Unit *u) { + bool b; + + assert(u); /* Indicate on the cgroup whether delegation is on, via an xattr. This is best-effort, as old kernels * didn't support xattrs on cgroups at all. Later they got support for setting 'trusted.*' xattrs, @@ -914,6 +912,12 @@ static void cgroup_xattr_apply(Unit *u) { else unit_remove_xattr_graceful(u, xn); } +} + +static void cgroup_survive_xattr_apply(Unit *u) { + int r; + + assert(u); if (u->survive_final_kill_signal) { r = cg_set_xattr( @@ -942,6 +946,21 @@ static void cgroup_xattr_apply(Unit *u) { } } +static void cgroup_xattr_apply(Unit *u) { + assert(u); + + /* The 'user.*' xattrs can be set from a user manager. */ + cgroup_oomd_xattr_apply(u); + cgroup_log_xattr_apply(u); + + if (!MANAGER_IS_SYSTEM(u->manager)) + return; + + cgroup_invocation_id_xattr_apply(u); + cgroup_delegate_xattr_apply(u); + cgroup_survive_xattr_apply(u); +} + static int lookup_block_device(const char *p, dev_t *ret) { dev_t rdev, dev = 0; mode_t mode; From 64c71f4fd7ad032c8e221bcbe18b4928345db4c8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 6 Oct 2023 18:36:03 +0200 Subject: [PATCH 5/6] cgroup: un-export two functions --- src/core/cgroup.c | 4 ++-- src/core/cgroup.h | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 5f8407c480..e2046f2c86 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -810,7 +810,7 @@ static void unit_remove_xattr_graceful(Unit *u, const char *name) { log_unit_debug_errno(u, r, "Failed to remove '%s' xattr flag on control group %s, ignoring: %m", name, empty_to_root(u->cgroup_path)); } -void cgroup_oomd_xattr_apply(Unit *u) { +static void cgroup_oomd_xattr_apply(Unit *u) { CGroupContext *c; assert(u); @@ -832,7 +832,7 @@ void cgroup_oomd_xattr_apply(Unit *u) { unit_remove_xattr_graceful(u, "user.oomd_omit"); } -int cgroup_log_xattr_apply(Unit *u) { +static int cgroup_log_xattr_apply(Unit *u) { ExecContext *c; size_t len, allowed_patterns_len, denied_patterns_len; _cleanup_free_ char *patterns = NULL, *allowed_patterns = NULL, *denied_patterns = NULL; diff --git a/src/core/cgroup.h b/src/core/cgroup.h index 868d83f43b..bd6345820c 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -278,9 +278,6 @@ static inline bool cgroup_context_want_memory_pressure(const CGroupContext *c) { int cgroup_context_add_device_allow(CGroupContext *c, const char *dev, const char *mode); int cgroup_context_add_bpf_foreign_program(CGroupContext *c, uint32_t attach_type, const char *path); -void cgroup_oomd_xattr_apply(Unit *u); -int cgroup_log_xattr_apply(Unit *u); - void cgroup_modify_nft_set(Unit *u, bool add); CGroupMask unit_get_own_mask(Unit *u); From 49b6babb76c306ccf9a06845cd6a4cb2083e051a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 6 Oct 2023 18:38:22 +0200 Subject: [PATCH 6/6] =?UTF-8?q?cgroup:=20rename=20cgroup=5Fmodify=5Fnft=5F?= =?UTF-8?q?set()=20=E2=86=92=20unit=5Fmodify=5Fnft=5Fset()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the only function that cgroup.h exports that is prefixed with cgroup_ where this does not refer to some type such as CGroupContext or CGroupTasksMax or so. It simply operates on a unit. And it doesn't even modify a cgroup, but just modifies an nft set. Hence, to make the naming scheme systematic, change prefix from cgroup_ to unit_, matching the majority of the functions that operate on Unit* in the file. --- src/core/cgroup.c | 13 ++++++++----- src/core/cgroup.h | 2 +- src/core/unit.c | 5 +---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index e2046f2c86..b6c1a7265e 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1387,19 +1387,20 @@ static void cgroup_apply_firewall(Unit *u) { (void) bpf_firewall_install(u); } -void cgroup_modify_nft_set(Unit *u, bool add) { +void unit_modify_nft_set(Unit *u, bool add) { int r; - CGroupContext *c; assert(u); if (!MANAGER_IS_SYSTEM(u->manager)) return; + if (!UNIT_HAS_CGROUP_CONTEXT(u)) + return; + if (cg_all_unified() <= 0) return; - assert_se(c = unit_get_cgroup_context(u)); if (u->cgroup_id == 0) return; @@ -1411,6 +1412,8 @@ void cgroup_modify_nft_set(Unit *u, bool add) { assert(u->manager->fw_ctx); } + CGroupContext *c = ASSERT_PTR(unit_get_cgroup_context(u)); + FOREACH_ARRAY(nft_set, c->nft_set_context.sets, c->nft_set_context.n_sets) { uint64_t element = u->cgroup_id; @@ -1864,7 +1867,7 @@ static void cgroup_context_apply( if (apply_mask & CGROUP_MASK_BPF_RESTRICT_NETWORK_INTERFACES) cgroup_apply_restrict_network_interfaces(u); - cgroup_modify_nft_set(u, /* add = */ true); + unit_modify_nft_set(u, /* add = */ true); } static bool unit_get_needs_bpf_firewall(Unit *u) { @@ -3042,7 +3045,7 @@ void unit_prune_cgroup(Unit *u) { (void) lsm_bpf_cleanup(u); /* Remove cgroup from the global LSM BPF map */ #endif - cgroup_modify_nft_set(u, /* add = */ false); + unit_modify_nft_set(u, /* add = */ false); is_root_slice = unit_has_name(u, SPECIAL_ROOT_SLICE); diff --git a/src/core/cgroup.h b/src/core/cgroup.h index bd6345820c..b189ed7485 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -278,7 +278,7 @@ static inline bool cgroup_context_want_memory_pressure(const CGroupContext *c) { int cgroup_context_add_device_allow(CGroupContext *c, const char *dev, const char *mode); int cgroup_context_add_bpf_foreign_program(CGroupContext *c, uint32_t attach_type, const char *path); -void cgroup_modify_nft_set(Unit *u, bool add); +void unit_modify_nft_set(Unit *u, bool add); CGroupMask unit_get_own_mask(Unit *u); CGroupMask unit_get_delegate_mask(Unit *u); diff --git a/src/core/unit.c b/src/core/unit.c index eed092b4dc..109148b02a 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3843,10 +3843,7 @@ int unit_coldplug(Unit *u) { if (u->nop_job) RET_GATHER(r, job_coldplug(u->nop_job)); - CGroupContext *c = unit_get_cgroup_context(u); - if (c) - cgroup_modify_nft_set(u, /* add = */ true); - + unit_modify_nft_set(u, /* add = */ true); return r; }