qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: virtio-fs-list <virtio-fs@redhat.com>, qemu-devel@nongnu.org
Cc: gkurz@redhat.com, johannes.berg@intel.com, slp@redhat.com,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	marcandre.lureau@redhat.com
Subject: What are libvhost-user locking requirements
Date: Tue, 19 Jan 2021 17:18:49 -0500	[thread overview]
Message-ID: <20210119221849.GC77840@redhat.com> (raw)

Hi,

Current virtiofsd code uses libvhost-user and I am assuming virtiofsd-rs
uses it too. I am wondering what are the locking requirements for
this library.

Looking at it it does not look like thread safe. Well parts of of kind
of look thread safe. For example, David Gilbert introduced a slave_mutex
to control reading/writeing on slave_fd. But dev->slave_fd can be modified
vu_set_slave_req_fd() without any locks. Similiarly _vu_queue_notify()
uses dev->slave_fd but  does not take any lock. May be these are just
bugs and we can take slave_mutex in those paths so not a big deal.

But this library does not talk about locking at all. Of course there
are many shared data structures like "struct VuDev" and helpers which
access this structure. Is client supposed to provide locking and
make sure not more than one thread is calling into the library
at one point of time.

But in virtiofsd I see that we seem to be in mixed mode. In some cases
we are holding ->vu_dispatch_rwlock in read-only mode. So that will
allow multipler threads to call into library for one queue.

In other places like lo_setupmapping() and lo_removemapping(), we are
not holding ->vu_dispatch_rwlock() at all and simply call into
library vu_fs_cache_request(VHOST_USER_SLAVE_FS_MAP/...). So multiple
threads can call in. I think precisely for this use case dev->slave_mutex
has been introduced in library.

So few queries.

- what's the locking model needed to use libvhost-user. Is there one? 

- Is it ok to selectively add locking for some data structures in
  libvhost-user. As slave_mutex has been added. So user will have to
  go through the code to figure out which paths can be called without
  locks and which paths can't be.

/me is confused and trying to wrap my head around the locking requirements
while using libvhost-user.

Vivek



             reply	other threads:[~2021-01-19 22:20 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-19 22:18 Vivek Goyal [this message]
2021-01-20 10:45 ` What are libvhost-user locking requirements Dr. David Alan Gilbert
2021-01-21 17:03   ` Stefan Hajnoczi

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=20210119221849.GC77840@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=gkurz@redhat.com \
    --cc=johannes.berg@intel.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=slp@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=virtio-fs@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).