All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel@nongnu.org, alex.williamson@redhat.com,
	felipe@nutanix.com
Subject: Re: [Qemu-devel] [PATCH 1/3] virtio: introduce grab/release_ioeventfd to fix vhost
Date: Fri, 18 Nov 2016 16:23:06 +0200	[thread overview]
Message-ID: <20161118162253-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <32da704e-4472-2afa-109f-185575d9f39f@de.ibm.com>

On Fri, Nov 18, 2016 at 09:15:32AM +0100, Christian Borntraeger wrote:
> On 11/16/2016 07:05 PM, Paolo Bonzini wrote:
> > Following the recent refactoring of virtio notifiers [1], more specifically
> > the patch ed08a2a0b ("virtio: use virtio_bus_set_host_notifier to
> > start/stop ioeventfd") that uses virtio_bus_set_host_notifier [2]
> > by default, core virtio code requires 'ioeventfd_started' to be set
> > to true/false when the host notifiers are configured.
> > 
> > When vhost is stopped and started, however, there is a stop followed by
> > another start. Since ioeventfd_started was never set to true, the 'stop'
> > operation triggered by virtio_bus_set_host_notifier() will not result
> > in a call to virtio_pci_ioeventfd_assign(assign=false). This leaves
> > the memory regions with stale notifiers and results on the next start
> > triggering the following assertion:
> > 
> >   kvm_mem_ioeventfd_add: error adding ioeventfd: File exists
> >   Aborted
> > 
> > This patch reintroduces (hopefully in a cleaner way) the concept
> > that was present with ioeventfd_disabled before the refactoring.
> > When ioeventfd_grabbed>0, ioeventfd_started tracks whether ioeventfd
> > should be enabled or not, but ioeventfd is actually not started at
> > all until vhost releases the host notifiers.
> > 
> > [1] http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg07748.html
> > [2] http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg07760.html
> > 
> > Reported-by: Felipe Franciosi <felipe@nutanix.com>
> > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > Reported-by: Alex Williamson <alex.williamson@redhat.com>
> > Fixes: ed08a2a0b ("virtio: use virtio_bus_set_host_notifier to start/stop ioeventfd")
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > Message-Id: <20161111192855.26350-1-pbonzini@redhat.com>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >         v1->v2: more comments [Cornelia]
> 
> 
> As this seems to fix a functional issues, is there any chance to apply this
> patch now and not wait for the discussion about patch 2 and 3 to calm down?

It's in my tree, will be in the next pull.

  reply	other threads:[~2016-11-18 14:23 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-16 18:05 [Qemu-devel] [PATCH v2 for-2.8 0/3] virtio fixes Paolo Bonzini
2016-11-16 18:05 ` [Qemu-devel] [PATCH 1/3] virtio: introduce grab/release_ioeventfd to fix vhost Paolo Bonzini
2016-11-17  5:27   ` Alexey Kardashevskiy
2016-11-17  8:36   ` Cornelia Huck
2016-11-18  8:15   ` Christian Borntraeger
2016-11-18 14:23     ` Michael S. Tsirkin [this message]
2016-11-16 18:05 ` [Qemu-devel] [PATCH 2/3] virtio: access ISR atomically Paolo Bonzini
2016-11-17 17:55   ` Michael S. Tsirkin
2016-11-17 19:49     ` Paolo Bonzini
2016-11-17 22:33       ` Michael S. Tsirkin
2016-11-18  8:09         ` Paolo Bonzini
2016-11-16 18:05 ` [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications Paolo Bonzini
2016-11-16 20:15   ` Michael S. Tsirkin
2016-11-16 20:38     ` Paolo Bonzini
2016-11-16 20:39       ` Michael S. Tsirkin
2016-11-16 21:05         ` Paolo Bonzini
2016-11-16 21:20           ` Michael S. Tsirkin
2016-11-17  9:04             ` Paolo Bonzini
2016-11-17 16:58               ` Michael S. Tsirkin
2016-11-17 10:44     ` Stefan Hajnoczi
2016-11-16 18:50 ` [Qemu-devel] [PATCH v2 for-2.8 0/3] virtio fixes no-reply
  -- strict thread matches above, loose matches on Subject: below --
2016-11-15 13:46 [Qemu-devel] [PATCH " Paolo Bonzini
2016-11-15 13:46 ` [Qemu-devel] [PATCH 1/3] virtio: introduce grab/release_ioeventfd to fix vhost Paolo Bonzini
2016-11-15 15:32   ` Cornelia Huck
2016-11-15 16:21     ` Paolo Bonzini

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=20161118162253-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=felipe@nutanix.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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.