2004-06-23 21:19:24

by Mike Miller (OS Dev)

[permalink] [raw]
Subject: problems with alloc_disk in genhd.c


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


2004-06-23 21:32:50

by Al Viro

[permalink] [raw]
Subject: Re: problems with alloc_disk in genhd.c

On Wed, Jun 23, 2004 at 04:18:29PM -0500, [email protected] 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.

2004-06-23 22:01:58

by Mike Miller (OS Dev)

[permalink] [raw]
Subject: Re: problems with alloc_disk in genhd.c

On Wed, Jun 23, 2004 at 10:24:59PM +0100, [email protected] wrote:
> On Wed, Jun 23, 2004 at 04:18:29PM -0500, [email protected] 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.

2004-06-23 22:05:17

by Al Viro

[permalink] [raw]
Subject: Re: problems with alloc_disk in genhd.c

On Wed, Jun 23, 2004 at 04:55:12PM -0500, [email protected] wrote:
> On Wed, Jun 23, 2004 at 10:24:59PM +0100, [email protected] wrote:
> > On Wed, Jun 23, 2004 at 04:18:29PM -0500, [email protected] 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.