Messages in this thread | | | Date | Tue, 26 Jul 2011 10:44:27 -0400 | From | Vivek Goyal <> | Subject | Re: [PATCH v3] blk-cgroup:be able to remove the record of unplugged device |
| |
On Tue, Jul 26, 2011 at 09:56:40AM +0800, Wanlong Gao wrote: > > Write a record like: > echo 8:0 1000 > blkio.throttle.read_bps_device > to control the bps of the device. > And then unplug this device without doing > "8:0 0 > blkio.throttle.read_bps_device" to remove the record, > it will not be removed until reboot, because if the device is removed, > write some record will return "-ENODEV". > > With this patch, when the device is removed, then if we want to remove > the record for this removed device, just write "0" then it'll not check > if it is a present device, just remove the present record.
Ok, not checking for device presence while deleting a rule makes sense. Some comments inline.
[..] > > Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> > --- > block/blk-cgroup.c | 18 +++++++++--------- > 1 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c > index bcaf16e..246645c 100644 > --- a/block/blk-cgroup.c > +++ b/block/blk-cgroup.c > @@ -785,7 +785,7 @@ 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, weight;
Do not replace temp with name weight. The place you are parsing it, it could be weight, bps or iops. So lets keep the generic name in place.
> int i = 0; > dev_t dev; > u64 bps, iops; > @@ -826,25 +826,25 @@ static int blkio_policy_parse_and_set(char *buf, > > dev = MKDEV(major, minor); > > - ret = blkio_check_dev_num(dev); > + ret = strict_strtoul(s[1], 10, &weight); > if (ret) > + return -EINVAL; > + > + ret = blkio_check_dev_num(dev); > + if (ret && (weight != 0)) > return ret;
Can you simplify above and just put a one comment also. Don't even call blkio_check_dev_num() if we are removing the rule.
/* For rule removal, do not check for device presence. */
if (temp != 0) ret = blkio_check_dev_num(dev);
Thanks Vivek
| |