From 979d03117ffeccd2cc18c8ff843932b03ea065b9 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 18 May 2016 13:51:46 -0700 Subject: [PATCH] core: update CGroupBlockIODeviceBandwidth to record both rbps and wbps CGroupBlockIODeviceBandwith is used to keep track of IO bandwidth limits for legacy cgroup hierarchies. Unlike the unified hierarchy counterpart CGroupIODeviceLimit, a CGroupBlockIODeviceBandwiddth records either a read or write limit and has a couple issues. * There's no way to clear specific config entry. * When configs are cleared for an IO direction of a unit, the kernel settings aren't cleared accordingly creating discrepancies. This patch updates CGroupBlockIODeviceBandwidth so that it behaves similarly to CGroupIODeviceLimit - each entry records both rbps and wbps limits and is cleared if both are at default values after kernel settings are updated. --- src/core/cgroup.c | 46 ++++++++++++++++++++++++++++++++-------------- src/core/cgroup.h | 4 ++-- src/core/dbus-cgroup.c | 39 ++++++++++++++++++++++++++------------- src/core/load-fragment.c | 42 +++++++++++++++++++++++++++--------------- 4 files changed, 87 insertions(+), 44 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index a546344..b0e7be0 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -206,12 +206,18 @@ void cgroup_context_dump(CGroupContext *c, FILE* f, const char *prefix) { LIST_FOREACH(device_bandwidths, b, c->blockio_device_bandwidths) { char buf[FORMAT_BYTES_MAX]; - fprintf(f, - "%s%s=%s %s\n", - prefix, - b->read ? "BlockIOReadBandwidth" : "BlockIOWriteBandwidth", - b->path, - format_bytes(buf, sizeof(buf), b->bandwidth)); + if (b->rbps != CGROUP_LIMIT_MAX) + fprintf(f, + "%sBlockIOReadBandwidth=%s %s\n", + prefix, + b->path, + format_bytes(buf, sizeof(buf), b->rbps)); + if (b->wbps != CGROUP_LIMIT_MAX) + fprintf(f, + "%sBlockIOWriteBandwidth=%s %s\n", + prefix, + b->path, + format_bytes(buf, sizeof(buf), b->wbps)); } } @@ -477,7 +483,7 @@ void cgroup_context_apply(CGroupContext *c, CGroupMask mask, const char *path, M char buf[MAX(DECIMAL_STR_MAX(uint64_t)+1, DECIMAL_STR_MAX(dev_t)*2+2+DECIMAL_STR_MAX(uint64_t)+1)]; CGroupBlockIODeviceWeight *w; - CGroupBlockIODeviceBandwidth *b; + CGroupBlockIODeviceBandwidth *b, *next; if (!is_root) { sprintf(buf, "%" PRIu64 "\n", @@ -504,22 +510,34 @@ void cgroup_context_apply(CGroupContext *c, CGroupMask mask, const char *path, M } } - /* FIXME: no way to reset this list */ - LIST_FOREACH(device_bandwidths, b, c->blockio_device_bandwidths) { - const char *a; + LIST_FOREACH_SAFE(device_bandwidths, b, next, c->blockio_device_bandwidths) { dev_t dev; + unsigned n = 0; r = lookup_block_device(b->path, &dev); if (r < 0) continue; - a = b->read ? "blkio.throttle.read_bps_device" : "blkio.throttle.write_bps_device"; + if (b->rbps != CGROUP_LIMIT_MAX) + n++; + sprintf(buf, "%u:%u %" PRIu64 "\n", major(dev), minor(dev), b->rbps); + r = cg_set_attribute("blkio", path, "blkio.throttle.read_bps_device", buf); + if (r < 0) + log_full_errno(IN_SET(r, -ENOENT, -EROFS, -EACCES) ? LOG_DEBUG : LOG_WARNING, r, + "Failed to set blkio.throttle.read_bps_device on %s: %m", path); - sprintf(buf, "%u:%u %" PRIu64 "\n", major(dev), minor(dev), b->bandwidth); - r = cg_set_attribute("blkio", path, a, buf); + if (b->wbps != CGROUP_LIMIT_MAX) + n++; + sprintf(buf, "%u:%u %" PRIu64 "\n", major(dev), minor(dev), b->wbps); + r = cg_set_attribute("blkio", path, "blkio.throttle.write_bps_device", buf); if (r < 0) log_full_errno(IN_SET(r, -ENOENT, -EROFS, -EACCES) ? LOG_DEBUG : LOG_WARNING, r, - "Failed to set %s on %s: %m", a, path); + "Failed to set blkio.throttle.write_bps_device on %s: %m", path); + + /* If @b contained no config, we just cleared the kernel + * counterpart too. No reason to keep @l around. */ + if (!n) + cgroup_context_free_blockio_device_bandwidth(c, b); } } diff --git a/src/core/cgroup.h b/src/core/cgroup.h index 1907461..2b1edba 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -77,8 +77,8 @@ struct CGroupBlockIODeviceWeight { struct CGroupBlockIODeviceBandwidth { LIST_FIELDS(CGroupBlockIODeviceBandwidth, device_bandwidths); char *path; - uint64_t bandwidth; - bool read; + uint64_t rbps; + uint64_t wbps; }; struct CGroupContext { diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 29c2eda..eef1c47 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -146,11 +146,17 @@ static int property_get_blockio_device_bandwidths( return r; LIST_FOREACH(device_bandwidths, b, c->blockio_device_bandwidths) { + uint64_t v; - if (streq(property, "BlockIOReadBandwidth") != b->read) + if (streq(property, "BlockIOReadBandwidth")) + v = b->rbps; + else + v = b->wbps; + + if (v == CGROUP_LIMIT_MAX) continue; - r = sd_bus_message_append(reply, "(st)", b->path, b->bandwidth); + r = sd_bus_message_append(reply, "(st)", b->path, v); if (r < 0) return r; } @@ -651,7 +657,7 @@ int bus_cgroup_set_property( CGroupBlockIODeviceBandwidth *a = NULL, *b; LIST_FOREACH(device_bandwidths, b, c->blockio_device_bandwidths) { - if (path_equal(path, b->path) && read == b->read) { + if (path_equal(path, b->path)) { a = b; break; } @@ -662,7 +668,8 @@ int bus_cgroup_set_property( if (!a) return -ENOMEM; - a->read = read; + a->rbps = CGROUP_LIMIT_MAX; + a->wbps = CGROUP_LIMIT_MAX; a->path = strdup(path); if (!a->path) { free(a); @@ -672,7 +679,10 @@ int bus_cgroup_set_property( LIST_PREPEND(device_bandwidths, c->blockio_device_bandwidths, a); } - a->bandwidth = u64; + if (read) + a->rbps = u64; + else + a->wbps = u64; } n++; @@ -685,15 +695,18 @@ int bus_cgroup_set_property( return r; if (mode != UNIT_CHECK) { - CGroupBlockIODeviceBandwidth *a, *next; + CGroupBlockIODeviceBandwidth *a; _cleanup_free_ char *buf = NULL; _cleanup_fclose_ FILE *f = NULL; size_t size = 0; if (n == 0) { - LIST_FOREACH_SAFE(device_bandwidths, a, next, c->blockio_device_bandwidths) - if (a->read == read) - cgroup_context_free_blockio_device_bandwidth(c, a); + LIST_FOREACH(device_bandwidths, a, c->blockio_device_bandwidths) { + if (read) + a->rbps = CGROUP_LIMIT_MAX; + else + a->wbps = CGROUP_LIMIT_MAX; + } } unit_invalidate_cgroup(u, CGROUP_MASK_BLKIO); @@ -705,13 +718,13 @@ int bus_cgroup_set_property( if (read) { fputs("BlockIOReadBandwidth=\n", f); LIST_FOREACH(device_bandwidths, a, c->blockio_device_bandwidths) - if (a->read) - fprintf(f, "BlockIOReadBandwidth=%s %" PRIu64 "\n", a->path, a->bandwidth); + if (a->rbps != CGROUP_LIMIT_MAX) + fprintf(f, "BlockIOReadBandwidth=%s %" PRIu64 "\n", a->path, a->rbps); } else { fputs("BlockIOWriteBandwidth=\n", f); LIST_FOREACH(device_bandwidths, a, c->blockio_device_bandwidths) - if (!a->read) - fprintf(f, "BlockIOWriteBandwidth=%s %" PRIu64 "\n", a->path, a->bandwidth); + if (a->wbps != CGROUP_LIMIT_MAX) + fprintf(f, "BlockIOWriteBandwidth=%s %" PRIu64 "\n", a->path, a->wbps); } r = fflush_and_check(f); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 9626d86..86b4fb0 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3208,7 +3208,7 @@ int config_parse_blockio_bandwidth( void *userdata) { _cleanup_free_ char *path = NULL; - CGroupBlockIODeviceBandwidth *b; + CGroupBlockIODeviceBandwidth *b = NULL, *t; CGroupContext *c = data; const char *bandwidth; uint64_t bytes; @@ -3223,12 +3223,10 @@ int config_parse_blockio_bandwidth( read = streq("BlockIOReadBandwidth", lvalue); if (isempty(rvalue)) { - CGroupBlockIODeviceBandwidth *next; - - LIST_FOREACH_SAFE (device_bandwidths, b, next, c->blockio_device_bandwidths) - if (b->read == read) - cgroup_context_free_blockio_device_bandwidth(c, b); - + LIST_FOREACH(device_bandwidths, b, c->blockio_device_bandwidths) { + b->rbps = CGROUP_LIMIT_MAX; + b->wbps = CGROUP_LIMIT_MAX; + } return 0; } @@ -3256,16 +3254,30 @@ int config_parse_blockio_bandwidth( return 0; } - b = new0(CGroupBlockIODeviceBandwidth, 1); - if (!b) - return log_oom(); + LIST_FOREACH(device_bandwidths, t, c->blockio_device_bandwidths) { + if (path_equal(path, t->path)) { + b = t; + break; + } + } - b->path = path; - path = NULL; - b->bandwidth = bytes; - b->read = read; + if (!t) { + b = new0(CGroupBlockIODeviceBandwidth, 1); + if (!b) + return log_oom(); + + b->path = path; + path = NULL; + b->rbps = CGROUP_LIMIT_MAX; + b->wbps = CGROUP_LIMIT_MAX; + + LIST_PREPEND(device_bandwidths, c->blockio_device_bandwidths, b); + } - LIST_PREPEND(device_bandwidths, c->blockio_device_bandwidths, b); + if (read) + b->rbps = bytes; + else + b->wbps = bytes; return 0; } -- 2.7.4