[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 05/36] ehci: Improve latency of interrupt delivery a
From: |
Gerd Hoffmann |
Subject: |
[Qemu-devel] [PATCH 05/36] ehci: Improve latency of interrupt delivery and async schedule scanning |
Date: |
Thu, 25 Oct 2012 14:51:38 +0200 |
From: Hans de Goede <address@hidden>
While doing various performance tests of reading from USB mass storage devices
I noticed the following::
1) When an async handled packet completes, we don't immediately report an
interrupt to the guest, instead we wait for the frame-timer to run and
report it from there
2) If 1) has been fixed and an async handled packet takes a while to complete,
then async_stepdown will become a high value, which means that there
will be a large latency before any new packets queued by the guest in
response to the interrupt get seen
1) was done deliberately as part of commit f0ad01f92:
http://www.kraxel.org/cgit/qemu/commit/?h=usb.57&id=f0ad01f92ca02eee7cadbfd225c5de753ebd5fce
Since setting the interrupt immediately on async packet completion was causing
issues with Linux guests, I believe this recently fixed Linux bug explains
why this is happening:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=361aabf395e4a23cf554cf4ec0c0c6963b8beb01
Note that we can *not* count on this fix being present in all Linux guests!
I was hoping that the recently added support for Interrupt Threshold Control
would fix the issues with Linux guests, but adding a simple ehci_commit_irq()
call to ehci_async_bh() still caused problems with Linux guests.
The problem is, that when doing ehci_commit_irq() from ehci_async_bh(),
the "old" frindex value is used to calculate usbsts_frindex, and when
the frame-timer then runs possibly very shortly after ehci_async_bh(),
it increases the frame-timer, and thus any interrupts raised from that
frame-timer run, will also get reported to the guest immediately, rather
then being delayed to the next frame-timer run.
Luckily the solution for this is simple, this means that we need to
increase frindex before calling ehci_commit_irq() from ehci_async_bh(),
which in the end boils down to simple calling ehci_frame_timer() instead
of ehci_async_bh() from the bh.
This may seem like it causes a lot of extra work to be done, but this
is not true. Any work done from the frame-timer processing the periodic
schedule is work which then does not need to be done the next time the
frame timer runs, also the frame-timer will re-arm itself at (possibly)
a later time then it was armed for saving a vmexit at that time.
As an additional advantage moving to simply calling the frame-timer also
fixes 2) as the packet completion will set async_stepdown to 0, and the
re-arming of the timer with an async_stepdown of 0 ensures that any
newly queued up packets get seen in a reasonable amount of time.
This improves the speed (MB/s) of a Linux guest reading from a USB mass
storage device by a factor of 1.5 - 1.7 with input pipelining disabled,
and by a factor of 1.8 with input pipelining enabled.
Signed-off-by: Hans de Goede <address@hidden>
Signed-off-by: Gerd Hoffmann <address@hidden>
---
hw/usb/hcd-ehci.c | 10 ++--------
1 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 8b4e3c8..f9ae05e 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1244,7 +1244,7 @@ static void ehci_opreg_write(void *ptr, hwaddr addr,
s->usbcmd = val; /* Set usbcmd for ehci_update_halt() */
ehci_update_halt(s);
s->async_stepdown = 0;
- qemu_mod_timer(s->frame_timer, qemu_get_clock_ns(vm_clock));
+ qemu_bh_schedule(s->async_bh);
}
break;
@@ -2510,12 +2510,6 @@ static void ehci_frame_timer(void *opaque)
}
}
-static void ehci_async_bh(void *opaque)
-{
- EHCIState *ehci = opaque;
- ehci_advance_async_state(ehci);
-}
-
static const MemoryRegionOps ehci_mmio_caps_ops = {
.read = ehci_caps_read,
.valid.min_access_size = 1,
@@ -2744,7 +2738,7 @@ static int usb_ehci_initfn(PCIDevice *dev)
}
s->frame_timer = qemu_new_timer_ns(vm_clock, ehci_frame_timer, s);
- s->async_bh = qemu_bh_new(ehci_async_bh, s);
+ s->async_bh = qemu_bh_new(ehci_frame_timer, s);
QTAILQ_INIT(&s->aqueues);
QTAILQ_INIT(&s->pqueues);
usb_packet_init(&s->ipacket);
--
1.7.1
- [Qemu-devel] [PATCH 34/36] xhci: allow disabling interrupters, (continued)
- [Qemu-devel] [PATCH 34/36] xhci: allow disabling interrupters, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 21/36] uhci: Rename UHCIAsync->td to UHCIAsync->td_addr, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 28/36] uhci: When the guest marks a pending td non-active, cancel the queue, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 27/36] uhci: Detect guest td re-use, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 13/36] usb: Move short-not-ok handling to the core, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 17/36] uhci: cleanup: Add an unlink call to uhci_async_cancel(), Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 25/36] uhci: Immediately free queues on device disconnect, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 14/36] usb: Add an int_req flag to USBPacket, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 36/36] xhci: fix usb name in caps, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 23/36] uhci: Make uhci_fill_queue() actually operate on an UHCIQueue, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 05/36] ehci: Improve latency of interrupt delivery and async schedule scanning,
Gerd Hoffmann <=
- [Qemu-devel] [PATCH 24/36] uhci: Store ep in UHCIQueue, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 19/36] uhci: Drop unnecessary forward declaration of some static functions, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 26/36] uhci: Verify queue has not been changed by guest, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 09/36] xhci: Add a xhci_ep_nuke_one_xfer helper function, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 07/36] ehci: Detect going in circles when filling the queue, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 04/36] ehci: Set int flag on a short input packet, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 22/36] uhci: Add uhci_read_td() helper function, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 31/36] uhci: Use only one queue for ctrl endpoints, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 30/36] uhci: Retry to fill the queue while waiting for td completion, Gerd Hoffmann, 2012/10/25
- [Qemu-devel] [PATCH 35/36] xhci: make number of interrupters and slots configurable, Gerd Hoffmann, 2012/10/25