Re: [PATCH 3/4] tcm_vhost: Fix vs->vs_endpoint checking in vhost_scsi_handle_vq()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 12, 2013 at 01:11:19PM +0200, Michael S. Tsirkin wrote:
> On Tue, Mar 12, 2013 at 10:42:50AM +0800, Asias He wrote:
> > vs->vs_endpoint is protected by the vs->dev.mutex. Use
> > tcm_vhost_check_endpoint() to do check. The helper does the needed
> > locking for us.
> > 
> > Signed-off-by: Asias He <asias@xxxxxxxxxx>
> > Reviewed-by: Stefan Hajnoczi <stefanha@xxxxxxxxxx>
> 
> This takes dev mutex on data path which will introduce
> contention esp for multiqueue.

Yes, for now it is okay, but for concurrent execution of multiqueue it is
really bad.

By the way, what is the overhead of taking and releasing the
vs->dev.mutex even if no one contents for it? Is this overhead gnorable.

> How about storing the endpoint as part of vq
> private data and protecting with vq mutex?

Hmm, this makes sense, let's see how well it works.

> > ---
> >  drivers/vhost/tcm_vhost.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
> > index 29612bc..61093d1 100644
> > --- a/drivers/vhost/tcm_vhost.c
> > +++ b/drivers/vhost/tcm_vhost.c
> > @@ -594,7 +594,7 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs,
> >  	u8 target;
> >  
> >  	/* Must use ioctl VHOST_SCSI_SET_ENDPOINT */
> > -	if (unlikely(!vs->vs_endpoint))
> > +	if (!tcm_vhost_check_endpoint(vs))
> >  		return;
> >  
> >  	mutex_lock(&vq->mutex);
> > -- 
> > 1.8.1.4

-- 
Asias
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux