Messages in this thread | | | Date | Wed, 10 Mar 2010 08:33:16 +0800 | From | Gui Jianfeng <> | Subject | Re: [PATCH 1/2 V3] io-controller: Add a new interface "weight_device" for IO-Controller |
| |
Vivek Goyal wrote: > On Tue, Mar 09, 2010 at 09:52:06AM +0800, Gui Jianfeng wrote: > > [..] >>>>>> +static int blkio_policy_parse_and_set(char *buf, >>>>>> + struct blkio_policy_node *newpn) >>>>>> +{ >>>>>> + char *s[4], *p, *major_s = NULL, *minor_s = NULL; >>>>>> + int ret; >>>>>> + unsigned long major, minor, temp; >>>>>> + int i = 0; >>>>>> + dev_t dev; >>>>>> + >>>>>> + memset(s, 0, sizeof(s)); >>>>>> + >>>>>> + while ((p = strsep(&buf, " ")) != NULL) { >>>>>> + if (!*p) >>>>>> + continue; >>>>>> + >>>>>> + s[i++] = p; >>>>>> + >>>>>> + /* Prevent from inputing too many things */ >>>>>> + if (i == 3) >>>>>> + break; >>>>>> + } >>>>>> + >>>>>> + if (i != 2) >>>>>> + 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); >>>> I am not quite sure if exposing a mojor,minor number is the best >>>> interface that can be exposed to user space. How about actual disk >>>> names like sda, sdb, .. etc? The only problem I see there is that it >>>> seems tricky to get to these disk names from within the block layer. >>>> "struct request_queue" has a pointer to backing_dev which has a device >>>> from which we can get major,minor. But in order to get to disk name, >>>> we would have to call get_gendisk which can hold a semaphore. Is this >>>> the reason for us going with major,minor as a user interface to >>>> specify a disk? I bet there are good reasons for us not keeping a >>>> pointer to "struct gendisk" from "struct request_queue". If we could >>>> keep that pointer, our user interface could be very easily modified to >>>> be the disk name like sda, sdb, etc. >>> That's a good question. Why not use device names instead of device >>> numbers? From user's perspective, device names will be more intutive >>> to use. >>> >>> At the same time, will it look odd to handle devices with their names as WWID. >>> >>> /dev/mapper/3600508b400105df70000e000026f0000 >>> >>> Though I see that there is an alternate way to address the same device >>> like /dev/dm-2 etc. >>> >>> So from user's perspective I think it will be more intutive to handle >>> disk names instead of numbers. >>> >>> Gui, did you forsee issues in implementing disk names? >> Hi Vivek, >> >> >From the implementation of view, we need a device number as a key in blkio_policy_node, >> if using device name as user interface, i can't figure out a way to retirve the >> corresponding device number by means of device name (like sda, not "/dev/sda"). > > Hi Gui, > > How about using full device path names (/dev/sda)? "blockdev" utility also > expects full device pathnames. Same seems to be the case with device mapper > targets. > > "device" cgroup controller probably is using major and minor numbers because > it needs to control creation of device file (mknod). > > May be we can use lookup_bdev() to get block_device pointer and then > get_gendisk() to check if it is a partition. > > I am not very sure but device name/path interface might turn out to be > more intutive.
Hi Vivek,
I don't think using an inode path name as interface is a good idea. Because, one can create new file to point to the same device. Also, pathname could be removed or renamed by user. So, i think device number is a better choice.
Thanks Gui
> > Jens, do you have any thoughts on this? > > Thanks > Vivek > > >
| |