allocate an official device major number for virtio device?

allocate an official device major number for virtio device?

[Jin Qian]

Hi -

Can we allocate an official device major number for virtio devices?
Currently it's using 240-254 (LOCAL/EXPERIMENTAL USE). The reason we
ask for this is because userspace will need to treat virtio block
devices differently and need a way to detect such device. For example,
it checks major number to detect scsi and mmc device.

https://android-review.googlesource.com/#/c/195240

With dynamic major numbers 240-254, we might treat other devices as
virtio block device incorrectly.

Also open for suggestions on what's the correct way to handle this.

Thanks in advance!
jin

Re: allocate an official device major number for virtio device?

[Greg Kroah-Hartman]

On Wed, Mar 02, 2016 at 05:08:00PM -0800, Jin Qian wrote:
> Hi -
> 
> Can we allocate an official device major number for virtio devices?
> Currently it's using 240-254 (LOCAL/EXPERIMENTAL USE). The reason we
> ask for this is because userspace will need to treat virtio block
> devices differently and need a way to detect such device. For example,
> it checks major number to detect scsi and mmc device.
> 
> https://android-review.googlesource.com/#/c/195240
> 
> With dynamic major numbers 240-254, we might treat other devices as
> virtio block device incorrectly.

You shouldn't treat them incorrectly, devtmpfs handles this for you
automatically, so I'd recommend using the dynamic majors please.

And what in-kernel code is using the "experimental" range today?  Do you
have a pointer to that?  We should fix that now...

thanks,

greg k-h

Re: allocate an official device major number for virtio device?

[Jin Qian]

Do you mean detecting device name string as in /dev/...?

Just checked latest virtio_blk code, it's dynamic but not using
anything specific to experimental range. I guess we're fine here but
Yu can confirm.

Thanks,
jin

On Wed, Mar 2, 2016 at 5:25 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Mar 02, 2016 at 05:08:00PM -0800, Jin Qian wrote:
>> Hi -
>>
>> Can we allocate an official device major number for virtio devices?
>> Currently it's using 240-254 (LOCAL/EXPERIMENTAL USE). The reason we
>> ask for this is because userspace will need to treat virtio block
>> devices differently and need a way to detect such device. For example,
>> it checks major number to detect scsi and mmc device.
>>
>> https://android-review.googlesource.com/#/c/195240
>>
>> With dynamic major numbers 240-254, we might treat other devices as
>> virtio block device incorrectly.
>
> You shouldn't treat them incorrectly, devtmpfs handles this for you
> automatically, so I'd recommend using the dynamic majors please.
>
> And what in-kernel code is using the "experimental" range today?  Do you
> have a pointer to that?  We should fix that now...
>
> thanks,
>
> greg k-h

RE: allocate an official device major number for virtio device?

[Ning, Yu]

Well, virtio_blk does use dynamic major number allocation, but the allocated block major just happens to fall in the "experimental" range (240-254)...

In more detail:

virtio_blk calls register_blkdev() with major = 0 in init() (drivers/block/virtio_blk.c:872):

	major = register_blkdev(0, "virtblk");

This line has been there since day one.  And register_blkdev() implements dynamic major allocation pretty straightforwardly:

	/* temporary */
	if (major == 0) {
		for (index = ARRAY_SIZE(major_names)-1; index > 0; index--) {
			if (major_names[index] == NULL)
				break;
		}

So it goes from index = 254 to 1 and picks the first unused.  Apparently, there's a good chance that the allocated major is between 240-254 (although lower numbers are also possible, theoretically).  Indeed, we always get 253 for virtio_blk with the x86_64 Android emulator kernel.

But "dynamic" means we can't rely on checking major == 253 to detect virtio_blk.  That's why we are doing a fnmatch() using pattern /sys/devices/*/block/vd* instead.  Is that the recommended approach?

Thanks,
Yu

> -----Original Message-----
> From: Jin Qian [mailto:jinqian@android.com]
> Sent: Thursday, March 3, 2016 9:48
> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-kernel@vger.kernel.org; Jeff Sharkey <jsharkey@google.com>; David
> Turner <digit@google.com>; pprabhu@google.com; Ning, Yu
> <yu.ning@intel.com>
> Subject: Re: allocate an official device major number for virtio device?
> 
> Do you mean detecting device name string as in /dev/...?
> 
> Just checked latest virtio_blk code, it's dynamic but not using anything specific
> to experimental range. I guess we're fine here but Yu can confirm.
> 
> Thanks,
> jin
> 
> On Wed, Mar 2, 2016 at 5:25 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Wed, Mar 02, 2016 at 05:08:00PM -0800, Jin Qian wrote:
> >> Hi -
> >>
> >> Can we allocate an official device major number for virtio devices?
> >> Currently it's using 240-254 (LOCAL/EXPERIMENTAL USE). The reason we
> >> ask for this is because userspace will need to treat virtio block
> >> devices differently and need a way to detect such device. For
> >> example, it checks major number to detect scsi and mmc device.
> >>
> >> https://android-review.googlesource.com/#/c/195240
> >>
> >> With dynamic major numbers 240-254, we might treat other devices as
> >> virtio block device incorrectly.
> >
> > You shouldn't treat them incorrectly, devtmpfs handles this for you
> > automatically, so I'd recommend using the dynamic majors please.
> >
> > And what in-kernel code is using the "experimental" range today?  Do
> > you have a pointer to that?  We should fix that now...
> >
> > thanks,
> >
> > greg k-h

Re: allocate an official device major number for virtio device?

[One Thousand Gnomes]

On Thu, 3 Mar 2016 03:52:20 +0000
"Ning, Yu" <yu.ning@intel.com> wrote:

> Well, virtio_blk does use dynamic major number allocation, but the allocated block major just happens to fall in the "experimental" range (240-254)...
> 
> In more detail:
> 
> virtio_blk calls register_blkdev() with major = 0 in init() (drivers/block/virtio_blk.c:872):
> 
> 	major = register_blkdev(0, "virtblk");
> 
> This line has been there since day one.  And register_blkdev() implements dynamic major allocation pretty straightforwardly:
> 
> 	/* temporary */
> 	if (major == 0) {
> 		for (index = ARRAY_SIZE(major_names)-1; index > 0; index--) {
> 			if (major_names[index] == NULL)
> 				break;
> 		}
> 
> So it goes from index = 254 to 1 and picks the first unused.  Apparently, there's a good chance that the allocated major is between 240-254 (although lower numbers are also possible, theoretically).  Indeed, we always get 253 for virtio_blk with the x86_64 Android emulator kernel.
> 
> But "dynamic" means we can't rely on checking major == 253 to detect virtio_blk.  That's why we are doing a fnmatch() using pattern /sys/devices/*/block/vd* instead.  Is that the recommended approach?

That sounds fine to me - if you desperately need to know the assigned
major you can look in /proc/devices

Alan

Re: allocate an official device major number for virtio device?

[Greg Kroah-Hartman]

On Wed, Mar 02, 2016 at 05:48:24PM -0800, Jin Qian wrote:
> Do you mean detecting device name string as in /dev/...?

No, I mean reacting to the uevents the kernel gives you when your device
shows up.

thanks,

greg k-h

Re: allocate an official device major number for virtio device?

[Greg Kroah-Hartman]

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Thu, Mar 03, 2016 at 03:52:20AM +0000, Ning, Yu wrote:
> Well, virtio_blk does use dynamic major number allocation, but the
> allocated block major just happens to fall in the "experimental" range
> (240-254)...

That all depends on what else is registered in the system at the moment.

> In more detail:
> 
> virtio_blk calls register_blkdev() with major = 0 in init() (drivers/block/virtio_blk.c:872):
> 
> 	major = register_blkdev(0, "virtblk");
> 
> This line has been there since day one.  And register_blkdev() implements dynamic major allocation pretty straightforwardly:
> 
> 	/* temporary */
> 	if (major == 0) {
> 		for (index = ARRAY_SIZE(major_names)-1; index > 0; index--) {
> 			if (major_names[index] == NULL)
> 				break;
> 		}
> 
> So it goes from index = 254 to 1 and picks the first unused.
> Apparently, there's a good chance that the allocated major is between
> 240-254 (although lower numbers are also possible, theoretically).
> Indeed, we always get 253 for virtio_blk with the x86_64 Android
> emulator kernel.
> 
> But "dynamic" means we can't rely on checking major == 253 to detect
> virtio_blk.

Nor should you, why would you care?

> That's why we are doing a fnmatch() using pattern
> /sys/devices/*/block/vd* instead.  Is that the recommended approach?

Yes, or just look at the device node that is already created in /dev/
for you automatially by devtmpfs.  Doesn't that work as expected today?

I still don't understand the issue you are having here at all, sorry.

greg k-h

Re: allocate an official device major number for virtio device?

[Jin Qian]

On Thu, Mar 3, 2016 at 9:18 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> A: No.
> Q: Should I include quotations after my reply?
>
> http://daringfireball.net/2007/07/on_top
>
> On Thu, Mar 03, 2016 at 03:52:20AM +0000, Ning, Yu wrote:
>> Well, virtio_blk does use dynamic major number allocation, but the
>> allocated block major just happens to fall in the "experimental" range
>> (240-254)...
>
> That all depends on what else is registered in the system at the moment.
>
>> In more detail:
>>
>> virtio_blk calls register_blkdev() with major = 0 in init() (drivers/block/virtio_blk.c:872):
>>
>>       major = register_blkdev(0, "virtblk");
>>
>> This line has been there since day one.  And register_blkdev() implements dynamic major allocation pretty straightforwardly:
>>
>>       /* temporary */
>>       if (major == 0) {
>>               for (index = ARRAY_SIZE(major_names)-1; index > 0; index--) {
>>                       if (major_names[index] == NULL)
>>                               break;
>>               }
>>
>> So it goes from index = 254 to 1 and picks the first unused.
>> Apparently, there's a good chance that the allocated major is between
>> 240-254 (although lower numbers are also possible, theoretically).
>> Indeed, we always get 253 for virtio_blk with the x86_64 Android
>> emulator kernel.
>>
>> But "dynamic" means we can't rely on checking major == 253 to detect
>> virtio_blk.
>
> Nor should you, why would you care?
>
>> That's why we are doing a fnmatch() using pattern
>> /sys/devices/*/block/vd* instead.  Is that the recommended approach?
>
> Yes, or just look at the device node that is already created in /dev/
> for you automatially by devtmpfs.  Doesn't that work as expected today?
>
> I still don't understand the issue you are having here at all, sorry.

Maybe Jeff can comment in more detail about the issue and which method
is preferred?

Just to summarize, we have a few ways to detect the device

1. match /sys/devices/...
2. match /dev/...
3. use uevent rule to detect virtio device creation
4. check a fixed major#

Thanks,
jin

>
> greg k-h

Re: allocate an official device major number for virtio device?

[Greg Kroah-Hartman]

On Thu, Mar 03, 2016 at 11:32:00AM -0800, Jin Qian wrote:
> On Thu, Mar 3, 2016 at 9:18 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > A: No.
> > Q: Should I include quotations after my reply?
> >
> > http://daringfireball.net/2007/07/on_top
> >
> > On Thu, Mar 03, 2016 at 03:52:20AM +0000, Ning, Yu wrote:
> >> Well, virtio_blk does use dynamic major number allocation, but the
> >> allocated block major just happens to fall in the "experimental" range
> >> (240-254)...
> >
> > That all depends on what else is registered in the system at the moment.
> >
> >> In more detail:
> >>
> >> virtio_blk calls register_blkdev() with major = 0 in init() (drivers/block/virtio_blk.c:872):
> >>
> >>       major = register_blkdev(0, "virtblk");
> >>
> >> This line has been there since day one.  And register_blkdev() implements dynamic major allocation pretty straightforwardly:
> >>
> >>       /* temporary */
> >>       if (major == 0) {
> >>               for (index = ARRAY_SIZE(major_names)-1; index > 0; index--) {
> >>                       if (major_names[index] == NULL)
> >>                               break;
> >>               }
> >>
> >> So it goes from index = 254 to 1 and picks the first unused.
> >> Apparently, there's a good chance that the allocated major is between
> >> 240-254 (although lower numbers are also possible, theoretically).
> >> Indeed, we always get 253 for virtio_blk with the x86_64 Android
> >> emulator kernel.
> >>
> >> But "dynamic" means we can't rely on checking major == 253 to detect
> >> virtio_blk.
> >
> > Nor should you, why would you care?
> >
> >> That's why we are doing a fnmatch() using pattern
> >> /sys/devices/*/block/vd* instead.  Is that the recommended approach?
> >
> > Yes, or just look at the device node that is already created in /dev/
> > for you automatially by devtmpfs.  Doesn't that work as expected today?
> >
> > I still don't understand the issue you are having here at all, sorry.
> 
> Maybe Jeff can comment in more detail about the issue and which method
> is preferred?
> 
> Just to summarize, we have a few ways to detect the device
> 
> 1. match /sys/devices/...

/sys/dev/block/ you mean.

> 2. match /dev/...

Yes.

> 3. use uevent rule to detect virtio device creation

Yes.

> 4. check a fixed major#

Don't do that, it's not going to work as you don't have an assigned
major number.

But again, what do you mean by "detect the device"?  What do you want to
do here, just open/mount it?  Why don't the existing tools just handle
this automatically for you?

What do you want to do when you "detect the device"?  What needs this?

thanks,

greg k-h

Re: allocate an official device major number for virtio device?

[Jeff Sharkey]

Adding a bit more context, in Android when we receive a uevent about a
new disk we use a separate userspace utility (sgdisk) to inspect the
partitions it contains, and we construct our own mknod() for the
partitions that we're interested in.  (We're doing this manually
because Android doesn't have devtmpfs.)

Disks with GPT can have up to 128 partitions, but when mknod()'ing the
partition devices I need to carefully ignore any partitions beyond the
maximum number of partitions supported by the underlying device
system, otherwise I risk jumping into the next disk.  The maximum
number of partitions appears to be well-known for certain major device
numbers (Documentation/devices.txt says 15 for SCSI, or reading
perdev_minors for MMC).

Is there a good way to determine this upper limit for disks surfaced
through virtio_blk?  Currently I've been using the major device number
as a heuristic, which is what kicked off this entire discussion.

j

On Thu, Mar 3, 2016 at 1:00 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Mar 03, 2016 at 11:32:00AM -0800, Jin Qian wrote:
>> On Thu, Mar 3, 2016 at 9:18 AM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>> > A: No.
>> > Q: Should I include quotations after my reply?
>> >
>> > http://daringfireball.net/2007/07/on_top
>> >
>> > On Thu, Mar 03, 2016 at 03:52:20AM +0000, Ning, Yu wrote:
>> >> Well, virtio_blk does use dynamic major number allocation, but the
>> >> allocated block major just happens to fall in the "experimental" range
>> >> (240-254)...
>> >
>> > That all depends on what else is registered in the system at the moment.
>> >
>> >> In more detail:
>> >>
>> >> virtio_blk calls register_blkdev() with major = 0 in init() (drivers/block/virtio_blk.c:872):
>> >>
>> >>       major = register_blkdev(0, "virtblk");
>> >>
>> >> This line has been there since day one.  And register_blkdev() implements dynamic major allocation pretty straightforwardly:
>> >>
>> >>       /* temporary */
>> >>       if (major == 0) {
>> >>               for (index = ARRAY_SIZE(major_names)-1; index > 0; index--) {
>> >>                       if (major_names[index] == NULL)
>> >>                               break;
>> >>               }
>> >>
>> >> So it goes from index = 254 to 1 and picks the first unused.
>> >> Apparently, there's a good chance that the allocated major is between
>> >> 240-254 (although lower numbers are also possible, theoretically).
>> >> Indeed, we always get 253 for virtio_blk with the x86_64 Android
>> >> emulator kernel.
>> >>
>> >> But "dynamic" means we can't rely on checking major == 253 to detect
>> >> virtio_blk.
>> >
>> > Nor should you, why would you care?
>> >
>> >> That's why we are doing a fnmatch() using pattern
>> >> /sys/devices/*/block/vd* instead.  Is that the recommended approach?
>> >
>> > Yes, or just look at the device node that is already created in /dev/
>> > for you automatially by devtmpfs.  Doesn't that work as expected today?
>> >
>> > I still don't understand the issue you are having here at all, sorry.
>>
>> Maybe Jeff can comment in more detail about the issue and which method
>> is preferred?
>>
>> Just to summarize, we have a few ways to detect the device
>>
>> 1. match /sys/devices/...
>
> /sys/dev/block/ you mean.
>
>> 2. match /dev/...
>
> Yes.
>
>> 3. use uevent rule to detect virtio device creation
>
> Yes.
>
>> 4. check a fixed major#
>
> Don't do that, it's not going to work as you don't have an assigned
> major number.
>
> But again, what do you mean by "detect the device"?  What do you want to
> do here, just open/mount it?  Why don't the existing tools just handle
> this automatically for you?
>
> What do you want to do when you "detect the device"?  What needs this?
>
> thanks,
>
> greg k-h



-- 
Jeff Sharkey
jsharkey@android.com

Re: allocate an official device major number for virtio device?

[Richard Weinberger]

On Thu, Mar 3, 2016 at 11:12 PM, Jeff Sharkey <jsharkey@android.com> wrote:
> Adding a bit more context, in Android when we receive a uevent about a
> new disk we use a separate userspace utility (sgdisk) to inspect the
> partitions it contains, and we construct our own mknod() for the
> partitions that we're interested in.  (We're doing this manually
> because Android doesn't have devtmpfs.)

Why don't you just enable devtmpfs? :-)

-- 
Thanks,
//richard

Re: allocate an official device major number for virtio device?

[Greg Kroah-Hartman]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

On Thu, Mar 03, 2016 at 03:12:04PM -0700, Jeff Sharkey wrote:
> Adding a bit more context, in Android when we receive a uevent about a
> new disk we use a separate userspace utility (sgdisk) to inspect the
> partitions it contains, and we construct our own mknod() for the
> partitions that we're interested in.  (We're doing this manually
> because Android doesn't have devtmpfs.)

Note, you can easily fix the 'no devtmpfs' issue :)

Well, not "easily" but note that because Android diverged from the
traditional (i.e. standard) device naming scheme, you all really are on
your own here, this is your own hole you have dug yourself into, I have
very little sympathy at all, sorry.

> Disks with GPT can have up to 128 partitions, but when mknod()'ing the
> partition devices I need to carefully ignore any partitions beyond the
> maximum number of partitions supported by the underlying device
> system, otherwise I risk jumping into the next disk.  The maximum
> number of partitions appears to be well-known for certain major device
> numbers (Documentation/devices.txt says 15 for SCSI, or reading
> perdev_minors for MMC).
> 
> Is there a good way to determine this upper limit for disks surfaced
> through virtio_blk?  Currently I've been using the major device number
> as a heuristic, which is what kicked off this entire discussion.

The kernel should know about this already, doesn't /sys/dev/block/
already contain pointers to the known partitions?  If not, how are you
creating a partition that the kernel doesn't know about?

And why not just use libudev and the disk probing tools that it provides
already for all of this instead of using something "non-standard" (i.e.
sgdisk)?  It already has worked all of this out for you, why reimplement
the wheel again?

thanks,

greg k-h