All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: Greg Kurz <groug@kaod.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	qemu-devel@nongnu.org, Vivek Goyal <vgoyal@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH 4/4] virtiofsd: Release vu_dispatch_lock when stopping queue
Date: Tue, 9 Mar 2021 15:20:00 +0000	[thread overview]
Message-ID: <YEeSIMtS6mzIar3A@stefanha-x1.localdomain> (raw)
In-Reply-To: <20210308123141.26444-5-groug@kaod.org>

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

On Mon, Mar 08, 2021 at 01:31:41PM +0100, Greg Kurz wrote:
> QEMU can stop a virtqueue by sending a VHOST_USER_GET_VRING_BASE request
> to virtiofsd. As with all other vhost-user protocol messages, the thread
> that runs the main event loop in virtiofsd takes the vu_dispatch lock in
> write mode. This ensures that no other thread can access virtqueues or
> memory tables at the same time.
> 
> In the case of VHOST_USER_GET_VRING_BASE, the main thread basically
> notifies the queue thread that it should terminate and waits for its
> termination:
> 
> main()
>  virtio_loop()
>   vu_dispatch_wrlock()
>   vu_dispatch()
>    vu_process_message()
>     vu_get_vring_base_exec()
>      fv_queue_cleanup_thread()
>       pthread_join()
> 
> Unfortunately, the queue thread ends up calling virtio_send_msg()
> at some point, which itself needs to grab the lock:
> 
> fv_queue_thread()
>  g_list_foreach()
>   fv_queue_worker()
>    fuse_session_process_buf_int()
>     do_release()
>      lo_release()
>       fuse_reply_err()
>        send_reply()
>         send_reply_iov()
>          fuse_send_reply_iov_nofree()
>           fuse_send_msg()
>            virtio_send_msg()
>             vu_dispatch_rdlock() <-- Deadlock !
> 
> Simply have the main thread to release the lock before going to
> sleep and take it back afterwards. A very similar patch was already
> sent by Vivek Goyal sometime back:
> 
> https://listman.redhat.com/archives/virtio-fs/2021-January/msg00073.html
> 
> The only difference here is that this done in fv_queue_set_started()
> because fv_queue_cleanup_thread() can also be called from virtio_loop()
> without the lock being held.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  tools/virtiofsd/fuse_virtio.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

      parent reply	other threads:[~2021-03-09 15:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-08 12:31 [PATCH 0/4] virtiofsd: Avoid potential deadlocks Greg Kurz
2021-03-08 12:31 ` [PATCH 1/4] vhost-user: Introduce nested event loop in vhost_user_read() Greg Kurz
2021-03-09 15:02   ` Stefan Hajnoczi
2021-03-09 18:35     ` Greg Kurz
2021-03-08 12:31 ` [PATCH 2/4] vhost-user: Convert slave channel to QIOChannelSocket Greg Kurz
2021-03-09 15:17   ` Stefan Hajnoczi
2021-03-09 20:23     ` Greg Kurz
2021-03-10 11:27       ` Stefan Hajnoczi
2021-03-10 13:08         ` Greg Kurz
2021-03-10 11:43       ` Daniel P. Berrangé
2021-03-10 13:45         ` Greg Kurz
2021-03-10 13:48           ` Daniel P. Berrangé
2021-03-08 12:31 ` [PATCH 3/4] vhost-user: Monitor slave channel in vhost_user_read() Greg Kurz
2021-03-09 15:18   ` Stefan Hajnoczi
2021-03-09 22:56     ` Greg Kurz
2021-03-08 12:31 ` [PATCH 4/4] virtiofsd: Release vu_dispatch_lock when stopping queue Greg Kurz
2021-03-09 14:00   ` Vivek Goyal
2021-03-09 15:20   ` Stefan Hajnoczi [this message]

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=YEeSIMtS6mzIar3A@stefanha-x1.localdomain \
    --to=stefanha@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=groug@kaod.org \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=vgoyal@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 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.