On Mon, Feb 22, 2010 at 04:51:46PM +0200, Avi Kivity wrote: > 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<amit.shah@xxxxxxxxxx> > >>>Signed-off-by: Marcelo Tosatti<mtosatti@xxxxxxxxxx> > >>> > >>>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). Yes. Sure, perhaps calling read() itself is appropriate, and i see your point that >handle_output contains more context for a smarter decision. But one can argue thats an improvement on top of a dumb wakeup. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html