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.

4 files changed, 87 insertions, 44 deletions

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index a54634469f..b0e7be082c 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 1907461f7e..2b1edbafc4 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 29c2edaf6b..eef1c47c14 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 9626d861c5..86b4fb071b 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;
 }