ehci: Improve latency of interrupt delivery and async schedule scanning
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 <hdegoede@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This commit is contained in:
parent
cf08a8a1f6
commit
0262f65aaa
|
@ -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);
|
||||
|
|
Loading…
Reference in a new issue