All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: virtio-fs@redhat.com, marcandre.lureau@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Virtio-fs] [RFC PATCH 0/6] vhost-user: Shutdown/Flush slave channel properly
Date: Wed, 10 Feb 2021 17:15:44 -0500	[thread overview]
Message-ID: <20210210221544.GC3200@redhat.com> (raw)
In-Reply-To: <20210210163830-mutt-send-email-mst@kernel.org>

On Wed, Feb 10, 2021 at 04:39:06PM -0500, Michael S. Tsirkin wrote:
> On Mon, Jan 25, 2021 at 01:01:09PM -0500, Vivek Goyal wrote:
> > Hi,
> > 
> > We are working on DAX support in virtiofs and have some patches out of
> > the tree hosted here.
> > 
> > https://gitlab.com/virtio-fs/qemu/-/commits/virtio-fs-dev
> > 
> > These patches have not been proposed for merge yet, becasue David
> > Gilbert noticed that we can run into a deadlock during an emergency
> > reboot of guest kernel. (echo b > /proc/sysrq-trigger).
> > 
> > I have provided details of deadlock in 4th path of the series with
> > subject "qemu, vhost-user: Extend protocol to start/stop/flush slave
> > channel".
> > 
> > Basic problem seems to be that we don't have a proper mechanism to
> > shutdown slave channel when vhost-user device is stopping. This means
> > there might be pending messages in slave channel and slave is blocked
> > and waiting for response.
> > 
> > This is an RFC patch series to enhance vhost-user protocol to 
> > properly shutdown/flush slave channel and avoid the deadlock. Though
> > we faced the issue in the context of virtiofs, any vhost-user
> > device using slave channel can potentially run into issues and
> > can benefit from these patches.
> > 
> > Any feedback is welcome. Currently patches are based on out of
> > tree code but after I get some feedback, I can only take pieces
> > which are relevant to upstream and post separately.
> > 
> > Thanks
> > Vivek
> 
> No comments so far - do you plan to post a non-RFC patchset?

Yes. Stefan wants me to poll both unix fd and slave fd in device
shutdown path and serve both of these in parallel, instead of
adding a new slave channel shutdown message. I am planning to give
it a try and post new patches.

Vivek
> 
> 
> > Vivek Goyal (6):
> >   virtiofsd: Drop ->vu_dispatch_rwlock while waiting for thread to exit
> >   libvhost-user: Use slave_mutex in all slave messages
> >   vhost-user: Return error code from slave_read()
> >   qemu, vhost-user: Extend protocol to start/stop/flush slave channel
> >   libvhost-user: Add support to start/stop/flush slave channel
> >   virtiofsd: Opt in for slave start/stop/shutdown functionality
> > 
> >  hw/virtio/vhost-user.c                    | 151 +++++++++++++++++++++-
> >  subprojects/libvhost-user/libvhost-user.c | 147 +++++++++++++++++----
> >  subprojects/libvhost-user/libvhost-user.h |   8 +-
> >  tools/virtiofsd/fuse_virtio.c             |  20 +++
> >  4 files changed, 294 insertions(+), 32 deletions(-)
> > 
> > -- 
> > 2.25.4
> 


WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: virtio-fs@redhat.com, marcandre.lureau@redhat.com,
	qemu-devel@nongnu.org, stefanha@redhat.com, dgilbert@redhat.com
Subject: Re: [RFC PATCH 0/6] vhost-user: Shutdown/Flush slave channel properly
Date: Wed, 10 Feb 2021 17:15:44 -0500	[thread overview]
Message-ID: <20210210221544.GC3200@redhat.com> (raw)
In-Reply-To: <20210210163830-mutt-send-email-mst@kernel.org>

On Wed, Feb 10, 2021 at 04:39:06PM -0500, Michael S. Tsirkin wrote:
> On Mon, Jan 25, 2021 at 01:01:09PM -0500, Vivek Goyal wrote:
> > Hi,
> > 
> > We are working on DAX support in virtiofs and have some patches out of
> > the tree hosted here.
> > 
> > https://gitlab.com/virtio-fs/qemu/-/commits/virtio-fs-dev
> > 
> > These patches have not been proposed for merge yet, becasue David
> > Gilbert noticed that we can run into a deadlock during an emergency
> > reboot of guest kernel. (echo b > /proc/sysrq-trigger).
> > 
> > I have provided details of deadlock in 4th path of the series with
> > subject "qemu, vhost-user: Extend protocol to start/stop/flush slave
> > channel".
> > 
> > Basic problem seems to be that we don't have a proper mechanism to
> > shutdown slave channel when vhost-user device is stopping. This means
> > there might be pending messages in slave channel and slave is blocked
> > and waiting for response.
> > 
> > This is an RFC patch series to enhance vhost-user protocol to 
> > properly shutdown/flush slave channel and avoid the deadlock. Though
> > we faced the issue in the context of virtiofs, any vhost-user
> > device using slave channel can potentially run into issues and
> > can benefit from these patches.
> > 
> > Any feedback is welcome. Currently patches are based on out of
> > tree code but after I get some feedback, I can only take pieces
> > which are relevant to upstream and post separately.
> > 
> > Thanks
> > Vivek
> 
> No comments so far - do you plan to post a non-RFC patchset?

Yes. Stefan wants me to poll both unix fd and slave fd in device
shutdown path and serve both of these in parallel, instead of
adding a new slave channel shutdown message. I am planning to give
it a try and post new patches.

Vivek
> 
> 
> > Vivek Goyal (6):
> >   virtiofsd: Drop ->vu_dispatch_rwlock while waiting for thread to exit
> >   libvhost-user: Use slave_mutex in all slave messages
> >   vhost-user: Return error code from slave_read()
> >   qemu, vhost-user: Extend protocol to start/stop/flush slave channel
> >   libvhost-user: Add support to start/stop/flush slave channel
> >   virtiofsd: Opt in for slave start/stop/shutdown functionality
> > 
> >  hw/virtio/vhost-user.c                    | 151 +++++++++++++++++++++-
> >  subprojects/libvhost-user/libvhost-user.c | 147 +++++++++++++++++----
> >  subprojects/libvhost-user/libvhost-user.h |   8 +-
> >  tools/virtiofsd/fuse_virtio.c             |  20 +++
> >  4 files changed, 294 insertions(+), 32 deletions(-)
> > 
> > -- 
> > 2.25.4
> 



  reply	other threads:[~2021-02-10 22:15 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-25 18:01 [Virtio-fs] [RFC PATCH 0/6] vhost-user: Shutdown/Flush slave channel properly Vivek Goyal
2021-01-25 18:01 ` Vivek Goyal
2021-01-25 18:01 ` [Virtio-fs] [PATCH 1/6] virtiofsd: Drop ->vu_dispatch_rwlock while waiting for thread to exit Vivek Goyal
2021-01-25 18:01   ` Vivek Goyal
2021-01-26 15:56   ` [Virtio-fs] " Greg Kurz
2021-01-26 15:56     ` Greg Kurz
2021-01-26 18:33     ` [Virtio-fs] " Vivek Goyal
2021-01-26 18:33       ` Vivek Goyal
2021-01-29 12:03       ` [Virtio-fs] " Greg Kurz
2021-01-29 12:03         ` Greg Kurz
2021-01-29 15:04         ` [Virtio-fs] " Vivek Goyal
2021-01-29 15:04           ` Vivek Goyal
2021-01-25 18:01 ` [Virtio-fs] [PATCH 2/6] libvhost-user: Use slave_mutex in all slave messages Vivek Goyal
2021-01-25 18:01   ` Vivek Goyal
2021-01-28 14:31   ` [Virtio-fs] " Greg Kurz
2021-01-28 14:31     ` Greg Kurz
2021-01-28 14:48     ` [Virtio-fs] " Vivek Goyal
2021-01-28 14:48       ` Vivek Goyal
2021-01-28 15:06       ` [Virtio-fs] " Greg Kurz
2021-01-28 15:06         ` Greg Kurz
2021-01-25 18:01 ` [Virtio-fs] [PATCH 3/6] vhost-user: Return error code from slave_read() Vivek Goyal
2021-01-25 18:01   ` Vivek Goyal
2021-01-29  9:45   ` [Virtio-fs] " Greg Kurz
2021-01-29  9:45     ` Greg Kurz
2021-01-29 15:02     ` [Virtio-fs] " Vivek Goyal
2021-01-29 15:02       ` Vivek Goyal
2021-01-25 18:01 ` [Virtio-fs] [PATCH 4/6] qemu, vhost-user: Extend protocol to start/stop/flush slave channel Vivek Goyal
2021-01-25 18:01   ` Vivek Goyal
2021-01-28 16:52   ` [Virtio-fs] " Stefan Hajnoczi
2021-01-28 16:52     ` Stefan Hajnoczi
2021-01-29 14:16     ` [Virtio-fs] " Vivek Goyal
2021-01-29 14:16       ` Vivek Goyal
2021-01-29 15:11       ` [Virtio-fs] " Vivek Goyal
2021-01-29 15:11         ` Vivek Goyal
2021-02-08 17:41         ` [Virtio-fs] " Stefan Hajnoczi
2021-02-08 17:41           ` Stefan Hajnoczi
2021-01-25 18:01 ` [Virtio-fs] [PATCH 5/6] libvhost-user: Add support " Vivek Goyal
2021-01-25 18:01   ` Vivek Goyal
2021-01-25 18:01 ` [Virtio-fs] [PATCH 6/6] virtiofsd: Opt in for slave start/stop/shutdown functionality Vivek Goyal
2021-01-25 18:01   ` Vivek Goyal
2021-02-10 21:39 ` [Virtio-fs] [RFC PATCH 0/6] vhost-user: Shutdown/Flush slave channel properly Michael S. Tsirkin
2021-02-10 21:39   ` Michael S. Tsirkin
2021-02-10 22:15   ` Vivek Goyal [this message]
2021-02-10 22:15     ` Vivek Goyal
2021-02-23 14:14 ` [Virtio-fs] " Michael S. Tsirkin
2021-02-23 14:14   ` Michael S. Tsirkin
2021-02-23 14:23   ` [Virtio-fs] " Vivek Goyal
2021-02-23 14:23     ` Vivek Goyal
2021-03-14 22:21 ` [Virtio-fs] " Michael S. Tsirkin
2021-03-14 22:21   ` Michael S. Tsirkin
2021-03-14 22:26   ` [Virtio-fs] " Vivek Goyal
2021-03-14 22:26     ` Vivek Goyal

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=20210210221544.GC3200@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --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 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.