| Message ID | 1272371652-23087-19-git-send-email-amit.shah@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04/27/2010 07:34 AM, Amit Shah wrote: > From: Marcelo Tosatti<mtosatti@redhat.com> > > Wake up iothread when buffers are consumed. > > Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com> > Signed-off-by: Amit Shah<amit.shah@redhat.com> > What's the race here? This looks very odd to me. Regards, Anthony Liguori > --- > hw/virtio-serial-bus.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > index 97694d5..ee868e9 100644 > --- a/hw/virtio-serial-bus.c > +++ b/hw/virtio-serial-bus.c > @@ -417,6 +417,7 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq) > > static void handle_input(VirtIODevice *vdev, VirtQueue *vq) > { > + qemu_notify_event(); > } > > static uint32_t get_features(VirtIODevice *vdev, uint32_t features) >
On Tue, Apr 27, 2010 at 12:41:27PM -0500, Anthony Liguori wrote: > On 04/27/2010 07:34 AM, Amit Shah wrote: > >From: Marcelo Tosatti<mtosatti@redhat.com> > > > >Wake up iothread when buffers are consumed. > > > >Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com> > >Signed-off-by: Amit Shah<amit.shah@redhat.com> > > What's the race here? This looks very odd to me. We discussed this on the following thread: http://www.mail-archive.com/kvm@vger.kernel.org/msg29249.html
On 04/27/2010 12:58 PM, Marcelo Tosatti wrote: > On Tue, Apr 27, 2010 at 12:41:27PM -0500, Anthony Liguori wrote: > >> On 04/27/2010 07:34 AM, Amit Shah wrote: >> >>> From: Marcelo Tosatti<mtosatti@redhat.com> >>> >>> Wake up iothread when buffers are consumed. >>> >>> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com> >>> Signed-off-by: Amit Shah<amit.shah@redhat.com> >>> >> What's the race here? This looks very odd to me. >> > We discussed this on the following thread: > > http://www.mail-archive.com/kvm@vger.kernel.org/msg29249.html > I don't think there was ever an adequate explanation of exactly what was happening. My suspicion is that the real problem is due to the can_read() handlers whereas it's necessary to invoke the main loop for can_read() to be executed again to cause the event to be polled. The better solution though is to not use the can_read() handler and instead explicitly register and deregister the read callbacks. qemu_set_fd_handler() would require a notify call but that makes sense. I think this is a good justification for removing uses of qemu_set_fd_handler2(). Regards, Anthony Liguori
On (Tue) Apr 27 2010 [12:41:27], Anthony Liguori wrote: > On 04/27/2010 07:34 AM, Amit Shah wrote: >> From: Marcelo Tosatti<mtosatti@redhat.com> >> >> Wake up iothread when buffers are consumed. >> >> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com> >> Signed-off-by: Amit Shah<amit.shah@redhat.com> >> > > What's the race here? This looks very odd to me. When the guest indicates it has added buffers to the vq, the iothread can then start consuming them. Without this notification, the iothread only polls for free buffers when it times out or gets woken up otherwise. Other virtio devices do it similarly as well. Amit
On 04/28/2010 02:29 AM, Amit Shah wrote: > On (Tue) Apr 27 2010 [12:41:27], Anthony Liguori wrote: > >> On 04/27/2010 07:34 AM, Amit Shah wrote: >> >>> From: Marcelo Tosatti<mtosatti@redhat.com> >>> >>> Wake up iothread when buffers are consumed. >>> >>> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com> >>> Signed-off-by: Amit Shah<amit.shah@redhat.com> >>> >>> >> What's the race here? This looks very odd to me. >> > When the guest indicates it has added buffers to the vq, the iothread > can then start consuming them. Without this notification, the iothread > only polls for free buffers when it times out or gets woken up > otherwise. > When you say, polls for free buffers, what do you mean by that? You mean, there's a can_read() somewhere that checks for free buffers? I think switching to qemu_set_fd_handler() would be a better solution. Regards, Anthony Liguori > Other virtio devices do it similarly as well. > > Amit >
On (Wed) Apr 28 2010 [08:25:59], Anthony Liguori wrote: > On 04/28/2010 02:29 AM, Amit Shah wrote: >> On (Tue) Apr 27 2010 [12:41:27], Anthony Liguori wrote: >> >>> On 04/27/2010 07:34 AM, Amit Shah wrote: >>> >>>> From: Marcelo Tosatti<mtosatti@redhat.com> >>>> >>>> Wake up iothread when buffers are consumed. >>>> >>>> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com> >>>> Signed-off-by: Amit Shah<amit.shah@redhat.com> >>>> >>>> >>> What's the race here? This looks very odd to me. >>> >> When the guest indicates it has added buffers to the vq, the iothread >> can then start consuming them. Without this notification, the iothread >> only polls for free buffers when it times out or gets woken up >> otherwise. >> > > When you say, polls for free buffers, what do you mean by that? > > You mean, there's a can_read() somewhere that checks for free buffers? Not just can_read(), in-qemu applications too can query when writes to guests will go through. > I think switching to qemu_set_fd_handler() would be a better solution. Hm, there's no fd here. How to signal to apps (ports) when a guest becomes writable? Amit
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 97694d5..ee868e9 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -417,6 +417,7 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq) static void handle_input(VirtIODevice *vdev, VirtQueue *vq) { + qemu_notify_event(); } static uint32_t get_features(VirtIODevice *vdev, uint32_t features)