Re: [PATCH] iSeries virtual disk

[Stephen Rothwell <sfr@canb.auug.org.au>]

[-- Attachment #1: Type: text/plain, Size: 3143 bytes --]

Hi Christoph,

On Fri, 27 Feb 2004 11:32:02 +0000 Christoph Hellwig <hch@infradead.org> wrote:
>
> > > it to the maximum value and then reset it in a magic even handler?
> > > I think that logic needs some clarification.
> > 
> > The "magic event handler" is synchronous with the probe_disk routine.  I
> > agree it is a bit confusing, but, at least I have the comment there about
> > the side effects of the probe_disk routine.  Changed slightly.
> 
> The code that is in Linus' tree is still b0rked:
> 
>  - you set viodasd_max_disk in viodasd_open which looks completely bogus:
>     o the value is never used after module_init, and as long as module_init
>       and blkdev ->open under BKL they are serialized.
>     o even if they weren't you wouldn't ever get an open call for a device
>       > viodasd_max_disk
>     o that means if you actually got there it would either be the same or
>       decreased
>     o if it was decreased in parallel to module_init your loop in
>       module_init would be totally screwed.
>   - now to that loop in module_init:
>     o they only thing that it actually archives is that it breaks out of
>       the loop if a probe_disk fails - but you could archive that much
>       more easier by just returning an error from the probe_disk and
>       use a break out of the loop.  The >= MAX_DISKNO check could then
>       easily happen on the i used as loop counter.

In theory you can hot add virtual disks to an iSeries partition (up to 64
disks per partition).  We used to support that by registering all 32 disks
with Linux and then probing the hypervisor when any disk was opened at
which point the hypervisor lets us know that the value of viod_max_disk
should be increased.  I took that code out in the name of simplicity and
in the hope of getting the driver accepted.  (In fact it was your comments
that made me do it.)  I will investigate your suggestion of using
blk_register_region for the disks that don't exist yet over the next while
at which point (assuming I can figure out how to do it) the value of
viodasd_max_disk may again rise over time.

viodasd_max_disk will never decrease, because you cannot remove a virtual
disk from a partition while it is running.

It doesn't break out of the loop when probe_disk fails because it is
possible to have gaps in the list if disks (e.g. I have have disks 1 2 4 7
and viodasd_max_disk will be 7 but probe_disk will "fail" for the other
disks).

In fact, the admin could add a disk while that loop is running so that
viodasd_max_disk could change even then. They would have to be quick
though :-)

Any clearer?

> 
> > > for lowend configurations (remember we have a 32bit dev_t now)
> > 
> > Can I leave this for now?
> 
> It's really awkwards.  And IBM will most likely want lots of disks soon
> anyway :)

The viodasd driver (as used in distros for a couple of years) has never
supported more than 32 disks ...  If you want lots of disks on an iSeries
partition you would used real direct attached SCSI controllers with real
disks.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]