problems with alloc_disk in genhd.c

problems with alloc_disk in genhd.c

[mikem]

We've encountered a problem using one of our internal test tools. It calls
our CCISS_GETLUNINFO ioctl for partition info. In the *alloc_disk(int minors)
function it only tests for the max_number_of_parts - 1.

        if (minors > 1) {
                int size = (minors - 1) * sizeof(struct hd_struct *);

When we allocate space we pass in

        for (n = 0; n < NWD; n++) {
                disk[n] = alloc_disk(1 << NWD_SHIFT);

In the  ioctl we are doing

        /* count partitions 1 to 15 with sizes > 0 */
        for(i=0; i <MAX_PART; i++) {


Depending on what lies beyond the array we have seen either Oops's or
a hard lock with a reboot about 30 seconds later. If we pass in MAX_PART - 1
we have no problems.
Is the entire disk no longer counted as partition zero?
Other drivers also pass in their max part value. Have any other problems
been reported?

Thanks,
mikem

Re: problems with alloc_disk in genhd.c

[viro]

On Wed, Jun 23, 2004 at 04:18:29PM -0500, mikem@beardog.cca.cpqcorp.net wrote:
> In the  ioctl we are doing
> 
>         /* count partitions 1 to 15 with sizes > 0 */
>         for(i=0; i <MAX_PART; i++) {

... followed by what?

Array of per-partition structures contains the data for partitions,
obviously.  And drivers have no damn business to ever touching it
directly, while we are at it.

BTW, take a look at the comment and loop following it.  And note
that you are doing 16 iterations in the loop, contrary to what
the comment above it says.

Re: problems with alloc_disk in genhd.c

[mikem]

On Wed, Jun 23, 2004 at 10:24:59PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> On Wed, Jun 23, 2004 at 04:18:29PM -0500, mikem@beardog.cca.cpqcorp.net wrote:
> > In the  ioctl we are doing
> > 
> >         /* count partitions 1 to 15 with sizes > 0 */
> >         for(i=0; i <MAX_PART; i++) {
> 
> ... followed by what?
Here's the actual code, I typoed this first time.
	/* count partitions 1 to 15 with sizes > 0 */
	for(i=1; i <MAX_PART; i++) {
		if (!disk->part[i])
			continue;
		if (disk->part[i]->nr_sects != 0)
			luninfo.num_parts++;
We're trying to figure how many partitions are physically on the disk. We do
this for one of our utilities. Is there a kernel API that will return this
data for us?

mikem
> 
> Array of per-partition structures contains the data for partitions,
> obviously.  And drivers have no damn business to ever touching it
> directly, while we are at it.
> 
> BTW, take a look at the comment and loop following it.  And note
> that you are doing 16 iterations in the loop, contrary to what
> the comment above it says.

Re: problems with alloc_disk in genhd.c

[viro]

On Wed, Jun 23, 2004 at 04:55:12PM -0500, mikem@beardog.cca.cpqcorp.net wrote:
> On Wed, Jun 23, 2004 at 10:24:59PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> > On Wed, Jun 23, 2004 at 04:18:29PM -0500, mikem@beardog.cca.cpqcorp.net wrote:
> > > In the  ioctl we are doing
> > > 
> > >         /* count partitions 1 to 15 with sizes > 0 */
> > >         for(i=0; i <MAX_PART; i++) {
> > 
> > ... followed by what?
> Here's the actual code, I typoed this first time.
> 	/* count partitions 1 to 15 with sizes > 0 */
> 	for(i=1; i <MAX_PART; i++) {
> 		if (!disk->part[i])
> 			continue;
> 		if (disk->part[i]->nr_sects != 0)
> 			luninfo.num_parts++;
> We're trying to figure how many partitions are physically on the disk. We do
> this for one of our utilities. Is there a kernel API that will return this
> data for us?

A bunch of those, starting with readdir on /sys/block/<whatever>.  Why do
you want that as a driver-specific ioctl?

That stuff has no business being in the driver.  At all.