All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Vlad Yasevich <vyasevich@gmail.com>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
Cc: netdev@vger.kernel.org, David Miller <davem@davemloft.net>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	virtualization@lists.linux-foundation.org
Subject: Re: [PULL 2/2] vhost: replace rcu with mutex
Date: Tue, 03 Jun 2014 15:55:45 +0200	[thread overview]
Message-ID: <538DD3E1.8000805@redhat.com> (raw)
In-Reply-To: <538DCF15.4080904@gmail.com>

Il 03/06/2014 15:35, Vlad Yasevich ha scritto:
> > Yes, vhost_get_vq_desc must be called with the vq mutex held.
> >
> > The rcu_read_lock/unlock in translate_desc is unnecessary.
>
> If that's true, then does dev->memory really needs to be rcu protected?
> It appears to always be read under mutex.

It's always read under one of many mutexes, yes.

However, it's still RCU-like in the sense that you separate the removal 
and reclamation phases so you still need rcu_dereference/rcu_assign_pointer.

With this mechanism, readers do not contend the mutexes with the 
VHOST_SET_MEMORY ioctl, except for the very short lock-and-unlock 
sequence at the end of it.  They also never contend the mutexes between 
themselves (which would be the case if VHOST_SET_MEMORY locked all the 
mutexes).

You could also wrap all virtqueue processing with a rwsem and take the 
rwsem for write in VHOST_SET_MEMORY.  That simplifies some things however:

- unnecessarily complicates the code for all users of vhost_get_vq_desc

- suppose the reader-writer lock is fair, and VHOST_SET_MEMORY places a 
writer in the queue.  Then a long-running reader R1 could still block 
another reader R2, because the writer would be served before R2.


The RCU-like approach avoids all this, which is important because of the 
generally simpler code and because VHOST_SET_MEMORY is the only vhost 
ioctl that can happen in the hot path.

Paolo

WARNING: multiple messages have this Message-ID (diff)
From: Paolo Bonzini <pbonzini@redhat.com>
To: Vlad Yasevich <vyasevich@gmail.com>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
Cc: netdev@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	David Miller <davem@davemloft.net>
Subject: Re: [PULL 2/2] vhost: replace rcu with mutex
Date: Tue, 03 Jun 2014 15:55:45 +0200	[thread overview]
Message-ID: <538DD3E1.8000805@redhat.com> (raw)
In-Reply-To: <538DCF15.4080904@gmail.com>

Il 03/06/2014 15:35, Vlad Yasevich ha scritto:
> > Yes, vhost_get_vq_desc must be called with the vq mutex held.
> >
> > The rcu_read_lock/unlock in translate_desc is unnecessary.
>
> If that's true, then does dev->memory really needs to be rcu protected?
> It appears to always be read under mutex.

It's always read under one of many mutexes, yes.

However, it's still RCU-like in the sense that you separate the removal 
and reclamation phases so you still need rcu_dereference/rcu_assign_pointer.

With this mechanism, readers do not contend the mutexes with the 
VHOST_SET_MEMORY ioctl, except for the very short lock-and-unlock 
sequence at the end of it.  They also never contend the mutexes between 
themselves (which would be the case if VHOST_SET_MEMORY locked all the 
mutexes).

You could also wrap all virtqueue processing with a rwsem and take the 
rwsem for write in VHOST_SET_MEMORY.  That simplifies some things however:

- unnecessarily complicates the code for all users of vhost_get_vq_desc

- suppose the reader-writer lock is fair, and VHOST_SET_MEMORY places a 
writer in the queue.  Then a long-running reader R1 could still block 
another reader R2, because the writer would be served before R2.


The RCU-like approach avoids all this, which is important because of the 
generally simpler code and because VHOST_SET_MEMORY is the only vhost 
ioctl that can happen in the hot path.

Paolo

  reply	other threads:[~2014-06-03 13:55 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-02 21:30 [PULL 0/2] vhost enhancements for 3.16 Michael S. Tsirkin
2014-06-02 21:30 ` Michael S. Tsirkin
2014-06-02 21:30 ` [PULL 1/2] vhost-net: extend device allocation to vmalloc Michael S. Tsirkin
2014-06-02 21:30   ` Michael S. Tsirkin
2014-06-02 21:30 ` [PULL 2/2] vhost: replace rcu with mutex Michael S. Tsirkin
2014-06-02 21:30   ` Michael S. Tsirkin
2014-06-02 21:58   ` Eric Dumazet
2014-06-02 21:58     ` Eric Dumazet
2014-06-03 12:48     ` Paolo Bonzini
2014-06-03 12:48       ` Paolo Bonzini
2014-06-03 13:35       ` Vlad Yasevich
2014-06-03 13:35         ` Vlad Yasevich
2014-06-03 13:55         ` Paolo Bonzini [this message]
2014-06-03 13:55           ` Paolo Bonzini
2014-06-03 13:57       ` Eric Dumazet
2014-06-03 13:57         ` Eric Dumazet
2014-06-03 14:20         ` Paolo Bonzini
2014-06-03 14:20           ` Paolo Bonzini
2014-06-04 19:51         ` Michael S. Tsirkin
2014-06-04 19:51           ` Michael S. Tsirkin
2014-06-05 10:45           ` Michael S. Tsirkin
2014-06-05 10:45             ` Michael S. Tsirkin
2014-06-04 18:12     ` Michael S. Tsirkin
2014-06-04 18:12       ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=538DD3E1.8000805@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=vyasevich@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.