Re: [PATCH] iSeries virtual disk

From: Stephen Rothwell
Date: Thu Feb 26 2004 - 21:47:57 EST


Hi Jeff,

Thanks for the feedback, comments follow:

On Thu, 26 Feb 2004 20:50:17 -0500 Jeff Garzik <jgarzik@xxxxxxxxx> wrote:
>
> Stephen Rothwell wrote:
> > On Thu, 26 Feb 2004 02:29:26 -0500 Jeff Garzik <jgarzik@xxxxxxxxx> wrote:
> >>2) num_req_outstanding accessed without lock in do_viodasd_request
> >>(driver's request_fn). all other accesses are inside spinlock.
> >
> >
> > This is actually OK because:
> > 1) if we see a value too large, when it get decremented by
> > handle_read_write, all the queue requst functions will get rerun.
> > 2) in send_request, if we get an error and decrement the count
> > to zero, then the count could have been at most 1 (sonce sends
> > are serialised) so in the request funtion, we would not have
> > stopped processing requests.
>
> That doesn't solve the race though... IMO protect it with the spinlock
> and be done with it...

As you said elsewhere, we can quibble after it is merged.

> >>5) is it really OK to call viodasd_open() and viodasd_release() multiple
> >>times? These functions do not look guarded against multiple openers.
> >
> > It is OK.
>
> I need more explanation than that.
>
> Multiple openers _are_ supported at the block layer, and this driver
> does not guard against multiple openers.

Sorry - the underlying Hypervisor calls supprto multiple open.

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

Attachment: pgp00000.pgp
Description: PGP signature