qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/4] Tweaks around virtio-blk start/stop


From: Cornelia Huck
Subject: Re: [Qemu-devel] [PATCH 0/4] Tweaks around virtio-blk start/stop
Date: Mon, 21 Mar 2016 13:17:46 +0100

On Mon, 21 Mar 2016 17:42:06 +0800
Fam Zheng <address@hidden> wrote:

> On Fri, 03/18 16:03, Paolo Bonzini wrote:
> >
> >
> > On 17/03/2016 17:08, Christian Borntraeger wrote:
> > > Good (or bad?) news is the assert also triggers on F23, it just seems
> > > to take longer.
> >
> > I guess good news, because we can rule out the kernel (not that I
> > believed it was a kernel problem, but the thought is always there in
> > the background...).
> >
> > The interaction between ioeventfd and dataplane is too complicated.  I
> > think if we get rid of the start/stop ioeventfd calls (just set up the
> > ioeventfd as soon as possible and then only set/clear the handlers)
> > things would be much simpler.
> >
> > I'll see if I can produce something based on Conny's patches, which are
> > already a start.  Today I had a short day so I couldn't play with the
> > bug; out of curiosity, does the bug reproduce with her work + patch 4
> > from this series + the reentrancy assertion?
> 
> The other half of the race condition is from ioport write in the vcpu thread. 
> I
> hit this by adding an extra assert(is_in_iothread()) in
> virtio_blk_handle_request(), at the same place with Paolo's atomic read of
> variable "test".
> 
> I haven't tried to find where this ioport write is from, but that is indeed an
> issue in virtio-pci.

Might this be a slow-path notification from before the ioeventfd got
registered on the io bus (IOW before qemu got control)? We have the
first notification that triggers dataplane setup (including registering
the ioeventfd). The second notification was already making its way to
qemu, and gets only handled when ->started had already been set.

Now I'm totally disregarding any possible locking between threads etc.
(perhaps somebody on cc: knows better?), but would the following logic
in handle_output make sense?

if (dataplane) {
   if (!started) {
      dataplane_start();
   }
   if (!disabled) {
      return;
   }
}
/* slow path processing */




reply via email to

[Prev in Thread] Current Thread [Next in Thread]