[Qemu-devel] Re: [patch uq/master 1/2] virtio-pci: wake up iothread on VIRTIO_PCI_QUEUE_NOTIFY

[Avi Kivity]

On 02/22/2010 04:29 PM, Marcelo Tosatti wrote:
> On Mon, Feb 22, 2010 at 04:20:52PM +0200, Avi Kivity wrote:
>    
> > On 02/22/2010 03:59 PM, Marcelo Tosatti wrote:
> >      
> > > VIRTIO_PCI_QUEUE_NOTIFY is used to inform availability of new buffers,
> > > so wakeup the iothread to process that information immediately.
> > >
> > > Reported-by: Amit Shah<address@hidden>
> > > Signed-off-by: Marcelo Tosatti<address@hidden>
> > >
> > > Index: qemu/hw/virtio-pci.c
> > > ===================================================================
> > > --- qemu.orig/hw/virtio-pci.c
> > > +++ qemu/hw/virtio-pci.c
> > > @@ -204,6 +204,7 @@ static void virtio_ioport_write(void *op
> > >           break;
> > >       case VIRTIO_PCI_QUEUE_NOTIFY:
> > >           virtio_queue_notify(vdev, val);
> > > +        qemu_notify_event();
> > >           break;
> > >        
> > virtio_queue_notify() will call ->handle_output(), which should
> > either do what's needed to be done, or wake up some iothread itself.
> >      
> kick is used to inform either output processing, in which case
> ->handle_output() does what its supposed to.
>
> But its also used to inform availability of new buffers, which is common
> to all virtio devices. So what is the point pushing this to
> ->handle_output?
>    

I don't understand what this means.  ->handle_output() _is_ informing 
the device model of new buffers.  What more is needed?

> Are you concerned about spurious wakeups?
>    

Yes.  Also, qemu_notify_event() is an undirected notification (wakes up 
all iothreads, and all devices), whereas ->handle_output() is directed 
(wakes up exactly what is needed).

What's the underlying problem?  A new input buffer has become available, 
and we need to re-poll the incoming file descriptor?  If so, that's best 
done from ->handle_output() (either by waking the iothread or calling 
read() itself and perhaps receiving -EAGAIN).

--
error compiling committee.c: too many arguments to function