From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38933) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wk7zb-0005mm-Vj for qemu-devel@nongnu.org; Tue, 13 May 2014 04:23:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wk7zV-0000IN-Pl for qemu-devel@nongnu.org; Tue, 13 May 2014 04:23:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17420) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wk7zV-0000I8-I8 for qemu-devel@nongnu.org; Tue, 13 May 2014 04:23:13 -0400 Date: Tue, 13 May 2014 11:21:58 +0300 From: "Michael S. Tsirkin" Message-ID: <20140513082158.GC29442@redhat.com> References: <33183CC9F5247A488A2544077AF19020815E7324@SZXEMA503-MBS.china.huawei.com> <536C8E83.8030504@redhat.com> <33183CC9F5247A488A2544077AF19020815E744C@SZXEMA503-MBS.china.huawei.com> <536CA5A5.4080303@redhat.com> <33183CC9F5247A488A2544077AF19020815E7B70@SZXEMA503-MBS.china.huawei.com> <53709B0C.4030808@redhat.com> <20140512103052.GB15294@redhat.com> <33183CC9F5247A488A2544077AF19020815E801C@SZXEMA503-MBS.china.huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <33183CC9F5247A488A2544077AF19020815E801C@SZXEMA503-MBS.china.huawei.com> Subject: Re: [Qemu-devel] [RFC] vhost: Can we change synchronize_rcu to call_rcu in vhost_set_memory() in vhost kernel module? List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gonglei (Arei)" Cc: "Huangweidong (C)" , "gleb@redhat.com" , "qemu-devel@nongnu.org" , "avi.kivity@gmail.com" , Paolo Bonzini , "Herongguang (Stephen)" On Tue, May 13, 2014 at 07:03:20AM +0000, Gonglei (Arei) wrote: > Hi, > > > -----Original Message----- > > From: Michael S. Tsirkin [mailto:mst@redhat.com] > > Sent: Monday, May 12, 2014 6:31 PM > > > > vhost does everything under a VQ lock. > > I think RCU for VHOST_SET_MEM_TABLE can be replaced with > > taking and freeing the VQ lock. > > > > Does the below solve the problem for you > > (warning: untested, sorry, busy with other bugs right now)? > > > > > > Signed-off-by: Michael S. Tsirkin > > > > --- > > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > index 78987e4..df2e3eb 100644 > > --- a/drivers/vhost/vhost.c > > +++ b/drivers/vhost/vhost.c > > @@ -593,6 +593,7 @@ static long vhost_set_memory(struct vhost_dev *d, > > struct vhost_memory __user *m) > > { > > struct vhost_memory mem, *newmem, *oldmem; > > unsigned long size = offsetof(struct vhost_memory, regions); > > + int i; > > > > if (copy_from_user(&mem, m, size)) > > return -EFAULT; > > @@ -619,7 +620,14 @@ static long vhost_set_memory(struct vhost_dev *d, > > struct vhost_memory __user *m) > > oldmem = rcu_dereference_protected(d->memory, > > lockdep_is_held(&d->mutex)); > > rcu_assign_pointer(d->memory, newmem); > > - synchronize_rcu(); > > + > > + /* All memory accesses are done under some VQ mutex. > > + * So below is a faster equivalent of synchronize_rcu() > > + */ > > + for (i = 0; i < dev->nvqs; ++i) { > > + mutex_lock(d->vqs[idx]->mutex); > > + mutex_unlock(d->vqs[idx]->mutex); > > + } > > kfree(oldmem); > > return 0; > > } > > Thanks for your advice, I suppose getting mutexes should generally be faster than waiting for > CPU context switches. And I think d->mutex should also be synchronized since somewhere gets > only this mutex directly and not vq mutexes. Is this right? No because all memory table accesses are under some vq mutex. > I'll try this approach, thanks. > > Best regards, > -Gonglei