From 9f0c0c4e158e835c7227dd677d6148607eece89d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Date: Fri, 10 Dec 2021 18:41:09 +0100 Subject: [PATCH 1/4] core/cgroup: Apply IODeviceWeight= through BFQ attributes as well There are two possible IO control mechanisms (io.cost and BFQ) that expose separate weights APIs. In systemd units we configure only a single device weight value so we need to write it to both attribute files. This patch extends to devices what is already done for default cgroup IO weight in commit 2dbc45aea747 ("cgroup: Also set io.bfq.weight"). This is possible with kernels >= v5.4 with commit 795fe54c2a82 ("bfq: Add per-device weight"). A warning is logged if the write fails on older kernels. Since per-device weight of a particular device cannot use both io.cost policy and BFQ scheduler, at least one warning is always expected with this approach (the method that is unconfigured). --- src/core/cgroup.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 42055e4e41..35edeac2ad 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1063,8 +1063,8 @@ static uint64_t cgroup_weight_io_to_blkio(uint64_t io_weight) { CGROUP_BLKIO_WEIGHT_MIN, CGROUP_BLKIO_WEIGHT_MAX); } -static void set_bfq_weight(Unit *u, const char *controller, uint64_t io_weight) { - char buf[DECIMAL_STR_MAX(uint64_t)+STRLEN("\n")]; +static void set_bfq_weight(Unit *u, const char *controller, dev_t dev, uint64_t io_weight) { + char buf[DECIMAL_STR_MAX(dev_t)*2+2+DECIMAL_STR_MAX(uint64_t)+STRLEN("\n")]; const char *p; uint64_t bfq_weight; @@ -1075,11 +1075,15 @@ static void set_bfq_weight(Unit *u, const char *controller, uint64_t io_weight) /* Adjust to kernel range is 1..1000, the default is 100. */ bfq_weight = BFQ_WEIGHT(io_weight); - xsprintf(buf, "%" PRIu64 "\n", bfq_weight); + if (major(dev) > 0) + xsprintf(buf, "%u:%u %" PRIu64 "\n", major(dev), minor(dev), bfq_weight); + else + xsprintf(buf, "%" PRIu64 "\n", bfq_weight); if (set_attribute_and_warn(u, controller, p, buf) >= 0 && io_weight != bfq_weight) - log_unit_debug(u, "%sIOWeight=%" PRIu64 " scaled to %s=%" PRIu64, + log_unit_debug(u, "%sIO%sWeight=%" PRIu64 " scaled to %s=%" PRIu64, streq(controller, "blkio") ? "Block" : "", + major(dev) > 0 ? "Device" : "", io_weight, p, bfq_weight); } @@ -1092,6 +1096,9 @@ static void cgroup_apply_io_device_weight(Unit *u, const char *dev_path, uint64_ if (r < 0) return; + /* BFQ per-device weights work since Linux kernel v5.4. */ + set_bfq_weight(u, "io", dev, io_weight); + xsprintf(buf, "%u:%u %" PRIu64 "\n", major(dev), minor(dev), io_weight); (void) set_attribute_and_warn(u, "io", "io.weight", buf); } @@ -1298,7 +1305,7 @@ static void set_io_weight(Unit *u, uint64_t weight) { assert(u); - set_bfq_weight(u, "io", weight); + set_bfq_weight(u, "io", makedev(0, 0), weight); xsprintf(buf, "default %" PRIu64 "\n", weight); (void) set_attribute_and_warn(u, "io", "io.weight", buf); @@ -1309,7 +1316,7 @@ static void set_blkio_weight(Unit *u, uint64_t weight) { assert(u); - set_bfq_weight(u, "blkio", weight); + set_bfq_weight(u, "blkio", makedev(0, 0), weight); xsprintf(buf, "%" PRIu64 "\n", weight); (void) set_attribute_and_warn(u, "blkio", "blkio.weight", buf); From 1cf4a685ad334d85dc5c0e61928577d76ca76311 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Date: Tue, 29 Mar 2022 17:42:19 +0200 Subject: [PATCH 2/4] core/cgroup: Refactor device weight property name Use arithmetic over explicit list of existing property names. --- src/core/cgroup.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 35edeac2ad..10081a0f9a 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1064,6 +1064,12 @@ static uint64_t cgroup_weight_io_to_blkio(uint64_t io_weight) { } static void set_bfq_weight(Unit *u, const char *controller, dev_t dev, uint64_t io_weight) { + static const char * const prop_names[] = { + "IOWeight", + "BlockIOWeight", + "IODeviceWeight", + "BlockIODeviceWeight", + }; char buf[DECIMAL_STR_MAX(dev_t)*2+2+DECIMAL_STR_MAX(uint64_t)+STRLEN("\n")]; const char *p; uint64_t bfq_weight; @@ -1081,9 +1087,8 @@ static void set_bfq_weight(Unit *u, const char *controller, dev_t dev, uint64_t xsprintf(buf, "%" PRIu64 "\n", bfq_weight); if (set_attribute_and_warn(u, controller, p, buf) >= 0 && io_weight != bfq_weight) - log_unit_debug(u, "%sIO%sWeight=%" PRIu64 " scaled to %s=%" PRIu64, - streq(controller, "blkio") ? "Block" : "", - major(dev) > 0 ? "Device" : "", + log_unit_debug(u, "%s=%" PRIu64 " scaled to %s=%" PRIu64, + prop_names[2*(major(dev) > 0) + streq(controller, "blkio")], io_weight, p, bfq_weight); } From 8d75f60e3ab861fb2e527003bd2da26605a26de5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Date: Wed, 16 Mar 2022 19:25:34 +0100 Subject: [PATCH 3/4] core/cgroup: Silence warnings IODeviceWeigth= on old kernels Old kernels (795fe54c2a82 ("bfq: Add per-device weight") v5.4) won't allow io.bfq.weight per-device configuration, don't get too excited about it but give the user one warning message to be aware this settings don't apply. --- src/core/cgroup.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 10081a0f9a..f1d5fda69b 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1070,9 +1070,11 @@ static void set_bfq_weight(Unit *u, const char *controller, dev_t dev, uint64_t "IODeviceWeight", "BlockIODeviceWeight", }; + static bool warned = false; char buf[DECIMAL_STR_MAX(dev_t)*2+2+DECIMAL_STR_MAX(uint64_t)+STRLEN("\n")]; const char *p; uint64_t bfq_weight; + int r; /* FIXME: drop this function when distro kernels properly support BFQ through "io.weight" * See also: https://github.com/systemd/systemd/pull/13335 and @@ -1086,7 +1088,21 @@ static void set_bfq_weight(Unit *u, const char *controller, dev_t dev, uint64_t else xsprintf(buf, "%" PRIu64 "\n", bfq_weight); - if (set_attribute_and_warn(u, controller, p, buf) >= 0 && io_weight != bfq_weight) + r = cg_set_attribute(controller, u->cgroup_path, p, buf); + + /* FIXME: drop this when kernels prior + * 795fe54c2a82 ("bfq: Add per-device weight") v5.4 + * are not interesting anymore. Old kernels will fail with EINVAL, while new kernels won't return + * EINVAL on properly formatted input by us. Treat EINVAL accordingly. */ + if (r == -EINVAL && major(dev) > 0 && !warned) { + log_unit_warning(u, "Kernel version does not accept per-device setting in %s.", p); + warned = true; + } + if (r < 0) + log_unit_full_errno(u, r == -EINVAL ? LOG_DEBUG : LOG_LEVEL_CGROUP_WRITE(r), + r, "Failed to set '%s' attribute on '%s' to '%.*s': %m", + p, empty_to_root(u->cgroup_path), (int) strcspn(buf, NEWLINE), buf); + else if (io_weight != bfq_weight) log_unit_debug(u, "%s=%" PRIu64 " scaled to %s=%" PRIu64, prop_names[2*(major(dev) > 0) + streq(controller, "blkio")], io_weight, p, bfq_weight); From 3e6eafddbfa3ed8a33f38dd33f872add042dae1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Date: Wed, 23 Mar 2022 19:02:06 +0100 Subject: [PATCH 4/4] core/cgroup: Silence IODeviceWeight= exclusive policy warnings A single device cannot have more than a single proportional control policy at the same time. So either io.bfq.weight or io.weight will always fail. Process the resulting values and emit a warning message only when none was really configured. This patch also drops warnings when setting [blk]io.bfq.weight fails, the code would be too convoluted otherwise. --- src/core/cgroup.c | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index f1d5fda69b..c4e078d78e 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1063,7 +1063,7 @@ static uint64_t cgroup_weight_io_to_blkio(uint64_t io_weight) { CGROUP_BLKIO_WEIGHT_MIN, CGROUP_BLKIO_WEIGHT_MAX); } -static void set_bfq_weight(Unit *u, const char *controller, dev_t dev, uint64_t io_weight) { +static int set_bfq_weight(Unit *u, const char *controller, dev_t dev, uint64_t io_weight) { static const char * const prop_names[] = { "IOWeight", "BlockIOWeight", @@ -1094,34 +1094,39 @@ static void set_bfq_weight(Unit *u, const char *controller, dev_t dev, uint64_t * 795fe54c2a82 ("bfq: Add per-device weight") v5.4 * are not interesting anymore. Old kernels will fail with EINVAL, while new kernels won't return * EINVAL on properly formatted input by us. Treat EINVAL accordingly. */ - if (r == -EINVAL && major(dev) > 0 && !warned) { - log_unit_warning(u, "Kernel version does not accept per-device setting in %s.", p); - warned = true; - } - if (r < 0) - log_unit_full_errno(u, r == -EINVAL ? LOG_DEBUG : LOG_LEVEL_CGROUP_WRITE(r), - r, "Failed to set '%s' attribute on '%s' to '%.*s': %m", - p, empty_to_root(u->cgroup_path), (int) strcspn(buf, NEWLINE), buf); - else if (io_weight != bfq_weight) + if (r == -EINVAL && major(dev) > 0) { + if (!warned) { + log_unit_warning(u, "Kernel version does not accept per-device setting in %s.", p); + warned = true; + } + r = -EOPNOTSUPP; /* mask as unconfigured device */ + } else if (r >= 0 && io_weight != bfq_weight) log_unit_debug(u, "%s=%" PRIu64 " scaled to %s=%" PRIu64, prop_names[2*(major(dev) > 0) + streq(controller, "blkio")], io_weight, p, bfq_weight); + return r; } static void cgroup_apply_io_device_weight(Unit *u, const char *dev_path, uint64_t io_weight) { char buf[DECIMAL_STR_MAX(dev_t)*2+2+DECIMAL_STR_MAX(uint64_t)+1]; dev_t dev; - int r; + int r, r1, r2; - r = lookup_block_device(dev_path, &dev); - if (r < 0) + if (lookup_block_device(dev_path, &dev) < 0) return; - /* BFQ per-device weights work since Linux kernel v5.4. */ - set_bfq_weight(u, "io", dev, io_weight); + r1 = set_bfq_weight(u, "io", dev, io_weight); xsprintf(buf, "%u:%u %" PRIu64 "\n", major(dev), minor(dev), io_weight); - (void) set_attribute_and_warn(u, "io", "io.weight", buf); + r2 = cg_set_attribute("io", u->cgroup_path, "io.weight", buf); + + /* Look at the configured device, when both fail, prefer io.weight errno. */ + r = r2 == -EOPNOTSUPP ? r1 : r2; + + if (r < 0) + log_unit_full_errno(u, LOG_LEVEL_CGROUP_WRITE(r), + r, "Failed to set 'io[.bfq].weight' attribute on '%s' to '%.*s': %m", + empty_to_root(u->cgroup_path), (int) strcspn(buf, NEWLINE), buf); } static void cgroup_apply_blkio_device_weight(Unit *u, const char *dev_path, uint64_t blkio_weight) { @@ -1326,7 +1331,7 @@ static void set_io_weight(Unit *u, uint64_t weight) { assert(u); - set_bfq_weight(u, "io", makedev(0, 0), weight); + (void) set_bfq_weight(u, "io", makedev(0, 0), weight); xsprintf(buf, "default %" PRIu64 "\n", weight); (void) set_attribute_and_warn(u, "io", "io.weight", buf); @@ -1337,7 +1342,7 @@ static void set_blkio_weight(Unit *u, uint64_t weight) { assert(u); - set_bfq_weight(u, "blkio", makedev(0, 0), weight); + (void) set_bfq_weight(u, "blkio", makedev(0, 0), weight); xsprintf(buf, "%" PRIu64 "\n", weight); (void) set_attribute_and_warn(u, "blkio", "blkio.weight", buf);