Paul Menage wrote:
> On Fri, Jun 19, 2009 at 1:37 PM, Vivek Goyal<vgoyal@xxxxxxxxxx> wrote:
>> You can use the following format to play with the new interface.
>> #echo DEV:weight:ioprio_class > /patch/to/cgroup/policy
>> weight=0 means removing the policy for DEV.
>>
>> Examples:
>> Configure weight=300 ioprio_class=2 on /dev/hdb in this cgroup
>> # echo /dev/hdb:300:2 > io.policy
>> # cat io.policy
>> dev weight class
>> /dev/hdb 300 2
>
> I think that the read and write should be consistent. Can you just use
> white-space separation for both, rather than colon-separation for
> writes and white-space separation for reads?
>
> Also, storing device inode paths statically as strings into the
> io_policy structure seems wrong, since it's quite possible for the
> device node that was used originally to be gone by the time that
> someone reads the io.policy file, or renamed, or even replaced with an
> inode that refers to to a different block device
>
> My preferred alternatives would be:
>
> - read/write the value as a device number rather than a name
> - read/write the block device's actual name (e.g. hda or sda) rather
> than a path to the inode
>
Hi Paul, Vivek
Here is a patch to fix the issue Paul raised.
This patch achives the following goals
1 According to Paul's comment, Modifing io.policy interface to
use device number for read/write directly.
2 Just use white-space separation for both, rather than colon-
separation for writes and white-space separation for reads.
3 Do more strict checking for inputting.
old interface:
Configure weight=300 ioprio_class=2 on /dev/hdb in this cgroup
# echo "/dev/hdb:300:2" > io.policy
# cat io.policy
dev weight class
/dev/hdb 300 2
new interface:
Configure weight=300 ioprio_class=2 on /dev/hdb in this cgroup
# echo "3:64 300 2" > io.policy
# cat io.policy
dev weight class
3:64 300 2
Signed-off-by: Gui Jianfeng <guijianfeng@xxxxxxxxxxxxxx>
---
block/elevator-fq.c | 59 ++++++++++++++++++++++++++++++++++----------------
block/elevator-fq.h | 1 -
2 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/block/elevator-fq.c b/block/elevator-fq.c
index d779282..83c831b 100644
--- a/block/elevator-fq.c
+++ b/block/elevator-fq.c
@@ -1895,12 +1895,12 @@ static int io_cgroup_policy_read(struct cgroup *cgrp, struct cftype *cft,
if (list_empty(&iocg->policy_list))
goto out;
- seq_printf(m, "dev weight class\n");
+ seq_printf(m, "dev\tweight\tclass\n");
spin_lock_irq(&iocg->lock);
list_for_each_entry(pn, &iocg->policy_list, node) {
- seq_printf(m, "%s %lu %lu\n", pn->dev_name,
- pn->weight, pn->ioprio_class);
+ seq_printf(m, "%u:%u\t%lu\t%lu\n", MAJOR(pn->dev),
+ MINOR(pn->dev), pn->weight, pn->ioprio_class);
}
spin_unlock_irq(&iocg->lock);
out:
@@ -1936,44 +1936,65 @@ static struct io_policy_node *policy_search_node(const struct io_cgroup *iocg,
return NULL;
}
-static int devname_to_devnum(const char *buf, dev_t *dev)
+static int check_dev_num(dev_t dev)
{
- struct block_device *bdev;
+ int part = 0;
struct gendisk *disk;
- int part;
- bdev = lookup_bdev(buf);
- if (IS_ERR(bdev))
+ disk = get_gendisk(dev, &part);
+ if (!disk || part)
return -ENODEV;
- disk = get_gendisk(bdev->bd_dev, &part);
- if (part)
- return -EINVAL;
-
- *dev = MKDEV(disk->major, disk->first_minor);
- bdput(bdev);
-
return 0;
}
static int policy_parse_and_set(char *buf, struct io_policy_node *newpn)
{
- char *s[3], *p;
+ char *s[4], *p, *major_s = NULL, *minor_s = NULL;
int ret;
+ unsigned long major, minor;
int i = 0;
+ dev_t dev;
memset(s, 0, sizeof(s));
- while ((p = strsep(&buf, ":")) != NULL) {
+ while ((p = strsep(&buf, " ")) != NULL) {
if (!*p)
continue;
s[i++] = p;
+
+ /* Prevent from inputing too many things */
+ if (i == 4)
+ break;
}
- ret = devname_to_devnum(s[0], &newpn->dev);
+ if (i != 3)
+ return -EINVAL;
+
+ p = strsep(&s[0], ":");
+ if (p != NULL)
+ major_s = p;
+ else
+ return -EINVAL;
+
+ minor_s = s[0];
+ if (!minor_s)
+ return -EINVAL;
+
+ ret = strict_strtoul(major_s, 10, &major);
+ if (ret)
+ return -EINVAL;
+
+ ret = strict_strtoul(minor_s, 10, &minor);
+ if (ret)
+ return -EINVAL;
+
+ dev = MKDEV(major, minor);
+
+ ret = check_dev_num(dev);
if (ret)
return ret;
- strcpy(newpn->dev_name, s[0]);
+ newpn->dev = dev;
if (s[1] == NULL)
return -EINVAL;
diff --git a/block/elevator-fq.h b/block/elevator-fq.h
index b3193f8..7722ebe 100644
--- a/block/elevator-fq.h
+++ b/block/elevator-fq.h
@@ -286,7 +286,6 @@ struct io_group {
struct io_policy_node {
struct list_head node;
- char dev_name[32];
dev_t dev;
unsigned long weight;
unsigned long ioprio_class;
--
1.5.4.rc3
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
