Re: [PATCH 2/1] io-controller: Add a new interface "policy" for IO Controller

From: Chad Talbott
Date: Wed Mar 03 2010 - 21:26:01 EST

On Wed, Mar 3, 2010 at 3:19 PM, Nauman Rafique <nauman@xxxxxxxxxx> wrote:
> On Wed, Mar 03, 2010 at 09:33:11AM -0500, Vivek Goyal wrote:
>> On Wed, Mar 03, 2010 at 02:21:11PM +0800, Gui Jianfeng wrote:
>> > You can use the following format to assigned specific weight for a given
>> > device:
>> > #echo "major:minor weight" > blkio.policy

>> Can we use a different name for per device weight file name than "blkio.policy".
>> May be "blkio.weight_device" or "blkio.weight_per_device" etc.

I agree with Vivek here, and his reasoning below. This becomes more
important as more attributes are added.

>> The reason being that "blkio.policy" name might be more suitable to switch
>> between differnt kind of BW control policies (proportional, max bandwidth etc),
>> once we implement some kind of max BW controlling policies also. So it
>> might be a good idea to not use that file name for specifying per device
>> weights.
> Well, thinking more about it, what kind of policy you implement on a block
> device will not be a per cgroup property. It will not be the case that on
> a device you are implementing max-bw for one cgroup and proportional
> weight for other cgroup. It probably will be a per device attribute and
> if need be should be controlled by /sys/block/<dev> interface.
> Still, being very clear what a particular cgroup file does makes sense. So
> that in future for max-bw control, we can bring in more cgroup files like
> blkio.max_bw or blkio.max_iops etc which can co-exist with blkio.weight_device
> etc.

Agreed. I'd like to add that since we are already thinking about
expanding the policy with more attributes, perhaps
blkio_update_group_weight_fn in blkio_policy_ops should be renamed to
blkio_policy_updated_fn. Then it could be called if the user changed
any part of the policy. Further, instead of storing "weight" in
blkio_cgroup, we could store a blkio_policy_node there instead. Then
the handling of whole-cgroup and per-block-device policy items could
be more regular.

Some quick code comments:

policy_parse_and_set() could be simplified with scanf, like:

if (sscanf(buf, "%d:%d %d", &major, &minor, &temp) != 3)
return -EINVAL;

blkcg_get_weight() might better be blkcg_get_policy() and it could
return a per-disk policy node, or fall back to the cgroup policy if
none existed for this dev. This would be across policy attributes,
rather than just weight.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at