blk-cgroup: be able to remove the record of unplugged device
authorWanlong Gao <gaowanlong@cn.fujitsu.com>
Wed, 21 Sep 2011 08:22:10 +0000 (10:22 +0200)
committerJens Axboe <axboe@kernel.dk>
Wed, 21 Sep 2011 08:22:10 +0000 (10:22 +0200)
The bug is we're not able to remove the device from blkio cgroup's
per-device control files if it gets unplugged.

To reproduce the bug:

  # mount -t cgroup -o blkio xxx /cgroup
  # cd /cgroup
  # echo "8:0 1000" > blkio.throttle.read_bps_device
  # unplug the device
  # cat blkio.throttle.read_bps_device
  8:0 1000
  # echo "8:0 0" > blkio.throttle.read_bps_device
  -bash: echo: write error: No such device

After patching, the device removal will succeed.

Thanks for the comments of Paul, Zefan, and Vivek.

Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Paul Menage <paul@paulmenage.org>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
block/blk-cgroup.c

index bcaf16ee6ad1ba321836413ee8841ab0bbde8f5e..b596e54ddd717d427d95ee609bc4a276c36c93a1 100644 (file)
@@ -785,10 +785,10 @@ static int blkio_policy_parse_and_set(char *buf,
 {
        char *s[4], *p, *major_s = NULL, *minor_s = NULL;
        int ret;
-       unsigned long major, minor, temp;
+       unsigned long major, minor;
        int i = 0;
        dev_t dev;
-       u64 bps, iops;
+       u64 temp;
 
        memset(s, 0, sizeof(s));
 
@@ -826,20 +826,23 @@ static int blkio_policy_parse_and_set(char *buf,
 
        dev = MKDEV(major, minor);
 
-       ret = blkio_check_dev_num(dev);
+       ret = strict_strtoull(s[1], 10, &temp);
        if (ret)
-               return ret;
+               return -EINVAL;
 
-       newpn->dev = dev;
+       /* For rule removal, do not check for device presence. */
+       if (temp) {
+               ret = blkio_check_dev_num(dev);
+               if (ret)
+                       return ret;
+       }
 
-       if (s[1] == NULL)
-               return -EINVAL;
+       newpn->dev = dev;
 
        switch (plid) {
        case BLKIO_POLICY_PROP:
-               ret = strict_strtoul(s[1], 10, &temp);
-               if (ret || (temp < BLKIO_WEIGHT_MIN && temp > 0) ||
-                       temp > BLKIO_WEIGHT_MAX)
+               if ((temp < BLKIO_WEIGHT_MIN && temp > 0) ||
+                    temp > BLKIO_WEIGHT_MAX)
                        return -EINVAL;
 
                newpn->plid = plid;
@@ -850,26 +853,18 @@ static int blkio_policy_parse_and_set(char *buf,
                switch(fileid) {
                case BLKIO_THROTL_read_bps_device:
                case BLKIO_THROTL_write_bps_device:
-                       ret = strict_strtoull(s[1], 10, &bps);
-                       if (ret)
-                               return -EINVAL;
-
                        newpn->plid = plid;
                        newpn->fileid = fileid;
-                       newpn->val.bps = bps;
+                       newpn->val.bps = temp;
                        break;
                case BLKIO_THROTL_read_iops_device:
                case BLKIO_THROTL_write_iops_device:
-                       ret = strict_strtoull(s[1], 10, &iops);
-                       if (ret)
-                               return -EINVAL;
-
-                       if (iops > THROTL_IOPS_MAX)
+                       if (temp > THROTL_IOPS_MAX)
                                return -EINVAL;
 
                        newpn->plid = plid;
                        newpn->fileid = fileid;
-                       newpn->val.iops = (unsigned int)iops;
+                       newpn->val.iops = (unsigned int)temp;
                        break;
                }
                break;