Re: [PATCH] iSeries virtual disk

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

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

Hi Jeff,

Thanks for the feedback, comments follow:

On Thu, 26 Feb 2004 20:50:17 -0500 Jeff Garzik <jgarzik@pobox.com> wrote:
>
> Stephen Rothwell wrote:
> > On Thu, 26 Feb 2004 02:29:26 -0500 Jeff Garzik <jgarzik@pobox.com> 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@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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