Message-ID: <1786ab031003261554y40c21bc8sb4ea2f347c2c8bf8@mail.gmail.com>
Date:	Fri, 26 Mar 2010 15:54:40 -0700
From:	Chad Talbott <ctalbott@...gle.com>
To:	Vivek Goyal <vgoyal@...hat.com>
Cc:	Gui Jianfeng <guijianfeng@...fujitsu.com>, jens.axboe@...cle.com,
	mrubin@...gle.com, Li Zefan <lizf@...fujitsu.com>,
	linux-kernel@...r.kernel.org, dpshah@...gle.com,
	Nauman Rafique <nauman@...gle.com>
Subject: Re: [PATCH 0/4] io-controller: Use names rather than major:minor

On Fri, Mar 26, 2010 at 8:20 AM, Vivek Goyal <vgoyal@...hat.com> wrote:
> On Fri, Mar 26, 2010 at 09:31:41AM +0800, Gui Jianfeng wrote:
>> +int blk_lookup_devname(dev_t devt, char *name)
>> +{

[ snip... loop through all block devices for devt ...snip ]

>> So we can keep dev_t in blkio layer, and export to user a device name by calling
>> this function. Also, we retrive device number by calling blk_lookup_devt().
>> This change might keep things much simple. Jens, do you have any thoughts?
>>
> I agree with Gui that lets keep the dev_t the core in blkio layer. Keeping
> a pointer to gendisk in request queue is becoming little messy.

Agreed on leaving gendisk pointer out of request_queue.  In doing
further investigation, I've found that it's up to the driver to
maintain the association between gendisk and request_queue, and some
drivers put multiple gendisk behind a single request_queue, so the
back pointer would be ill-specified.

> But if that does not work for you, then I would also like to keep things
> simple and translate dev_t to diskname during read routine. Similiarly,
> while somebody is putting policy, use blk_lookup_devt().

I like the simplicity of blk_lookup_devt(), but I don't like the idea
of iterating through all block devices on every lookup of the name.
Perhaps we could cache the name somewhere?

Actually, the name is the name of the *queue* (or the key in
blk-cgroup), because as I mentioned above there can be a many to one
relationship between disks and queues in general.

The more I think about it, the more it seems to make sense to extend
blkio_policy_ops to include a function to get the name of the key.
blk-cgroup makes no current use of the dev, except to invent a name
for the request_queue whose policy is being set or printed.  It could
be argued that the thing being scheduled has a better idea of the name
of that thing.

> But this will lead to issue of how do you now display both device number
> and disk name in the output. May be following.
>
> major:minor  diskname  data
>
> I am not sure if people are fond of multiple values in a single file. At
> the same time for setting the rules or deleting the rules, it will make
> syntax complicated/confusing. Also will require breaking ABI for existing
> blkio.time, blkio.sectors, blkio.dequeue files.

I don't like this, either.  It breaks ABI and is more confusing for users.

> So I would prefer to keep the major/minor number based interface for
> follwing reasons.
>
> - Chaning it now breaks ABI.
> - Other cgroup controller "device" is also using major/minor number based
>  interface for device access policy. So it is consistent with other
>  controller.

Which controllers are these?

> - Displaying both device major/minor and diskname is an option but that
>  makes the file format syntax little complicated and new rule setting
>  or removoal confusing.

A few messages back you mentioned that you preferred device names
because they would be better for users of the system.  If there was a
simple implementation, would you still be behind a new name-based
interface?  We could go that direction and maintain ABI by deprecating
current interface and making a new interface with names.

If you can't tell, I'm a big fan of using the name! :)  It's *much*
more consistent with the interfaces in /sys.

Chad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists