Re: [PATCH 1/2] tcm_vhost: Wait for pending requests in vhost_scsi_flush()

[Asias He <asias@redhat.com>]

On Wed, Mar 13, 2013 at 01:16:59PM +0800, Asias He wrote:
> On Tue, Mar 12, 2013 at 09:21:51AM +0100, Paolo Bonzini wrote:
> > Il 12/03/2013 02:31, Asias He ha scritto:
> > > On Mon, Mar 11, 2013 at 12:36:37PM +0100, Paolo Bonzini wrote:
> > >> Il 11/03/2013 06:09, Asias He ha scritto:
> > >>> This patch makes vhost_scsi_flush() wait for all the pending requests
> > >>> issued before the flush operation to be finished.
> > >>
> > >> There is no protection against issuing concurrent flush operations.  If
> > >> we later would like to make the flush a ioctl (for example for migration
> > >> purposes), it would be confusing, and I'm not sure how you could extend
> > >> the during_flush machinery.
> > > 
> > > vhost_scsi_flush() is called under the vs->dev.mutex lock.
> > 
> > Ah, ok.
> > 
> > >> What about making vhost_scsi_flush() wait for all pending requests,
> > >> including those issues during the flush operation?
> > > 
> > > This will take unbonded time if guest keep sending requests.
> > 
> > Yes, that's correct, but flush doesn't really mean much if new requests
> > can come in (unlike _cache_ flushes like SYNCHRONIZE CACHE).  In the end
> > you'll have to stop the VM first and then issue the flush.  At this
> > point, it does not change much if you wait for previous requests or all
> > requests, and I suspect that waiting for all requests simplifies the
> > code noticeably.
> 
> Michael, any comments? You suggested flushing of previous requests other
> than all the requests when I was doing vhost-blk.

Ping.

> > >> Then you can easily
> > >> support concurrent flushes; just add a waitqueue and wake_up_all at the
> > >> end of the flush operation.
> > > 
> > > I am not sure why we want concurrent flushes. The flush thing is
> > > already getting complex.
> > 
> > Yeah, it is too complex...
> > 
> > Paolo
> > 
> > >> BTW, adding such a ioctl as part of this patch would probably be a good
> > >> thing to do anyway.
> > >>
> > >> Paolo
> > > 
> > 
> 
> -- 
> Asias

-- 
Asias