[Qemu-devel] [PATCH 1/1] ehci: faster frame index calculation for skipped frames

[Qemu-devel] [PATCH 1/1] ehci: faster frame index calculation for skipped frames

[Denis V. Lunev]

From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>

ehci_update_frindex takes time linearly proportional to a number
of uframes to calculate new frame index and raise FLR interrupts,
which is a problem for large amounts of uframes.

If we experience large delays between echi timer callbacks (i.e. because
other periodic handlers have taken a lot of time to complete) we
get a lot of skipped frames which then delay ehci timer callback more
and this leads to deadlocking the system when ehci schedules next
callback to be too soon.

Observable behaviour is qemu consuming 100% host CPU time while guest
is unresponsive. This misbehavior could happen for a while and QEMU does
not get out from this state automatically without the patch.

This change makes ehci_update_frindex execute in constant time.

Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 43a8f7a..b093db7 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2206,29 +2206,28 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
 
 static void ehci_update_frindex(EHCIState *ehci, int uframes)
 {
-    int i;
-
     if (!ehci_enabled(ehci) && ehci->pstate == EST_INACTIVE) {
         return;
     }
 
-    for (i = 0; i < uframes; i++) {
-        ehci->frindex++;
-
-        if (ehci->frindex == 0x00002000) {
-            ehci_raise_irq(ehci, USBSTS_FLR);
-        }
+    /* Generate FLR interrupt if frame index rolls over 0x2000 */
+    if ((ehci->frindex % 0x2000) + uframes >= 0x2000) {
+        ehci_raise_irq(ehci, USBSTS_FLR);
+    }
 
-        if (ehci->frindex == 0x00004000) {
-            ehci_raise_irq(ehci, USBSTS_FLR);
-            ehci->frindex = 0;
-            if (ehci->usbsts_frindex >= 0x00004000) {
-                ehci->usbsts_frindex -= 0x00004000;
-            } else {
-                ehci->usbsts_frindex = 0;
-            }
+    /* How many times will frindex roll over 0x4000 with this frame count?
+     * usbsts_frindex is decremented by 0x4000 on rollover until it reaches 0
+     */
+    int rollovers = (ehci->frindex + uframes) / 0x4000;
+    if (rollovers > 0) {
+        if (ehci->usbsts_frindex >= (rollovers * 0x4000)) {
+            ehci->usbsts_frindex -= 0x4000 * rollovers;
+        } else {
+            ehci->usbsts_frindex = 0;
         }
     }
+
+    ehci->frindex = (ehci->frindex + uframes) % 0x4000;
 }
 
 static void ehci_frame_timer(void *opaque)
-- 
2.5.0

Re: [Qemu-devel] [PATCH 1/1] ehci: faster frame index calculation for skipped frames

[Denis V. Lunev]

On 07/27/2016 06:55 PM, Denis V. Lunev wrote:
> From: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
>
> ehci_update_frindex takes time linearly proportional to a number
> of uframes to calculate new frame index and raise FLR interrupts,
> which is a problem for large amounts of uframes.
>
> If we experience large delays between echi timer callbacks (i.e. because
> other periodic handlers have taken a lot of time to complete) we
> get a lot of skipped frames which then delay ehci timer callback more
> and this leads to deadlocking the system when ehci schedules next
> callback to be too soon.
>
> Observable behaviour is qemu consuming 100% host CPU time while guest
> is unresponsive. This misbehavior could happen for a while and QEMU does
> not get out from this state automatically without the patch.
>
> This change makes ehci_update_frindex execute in constant time.
>
> Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/usb/hcd-ehci.c | 31 +++++++++++++++----------------
>   1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
> index 43a8f7a..b093db7 100644
> --- a/hw/usb/hcd-ehci.c
> +++ b/hw/usb/hcd-ehci.c
> @@ -2206,29 +2206,28 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
>   
>   static void ehci_update_frindex(EHCIState *ehci, int uframes)
>   {
> -    int i;
> -
>       if (!ehci_enabled(ehci) && ehci->pstate == EST_INACTIVE) {
>           return;
>       }
>   
> -    for (i = 0; i < uframes; i++) {
> -        ehci->frindex++;
> -
> -        if (ehci->frindex == 0x00002000) {
> -            ehci_raise_irq(ehci, USBSTS_FLR);
> -        }
> +    /* Generate FLR interrupt if frame index rolls over 0x2000 */
> +    if ((ehci->frindex % 0x2000) + uframes >= 0x2000) {
> +        ehci_raise_irq(ehci, USBSTS_FLR);
> +    }
>   
> -        if (ehci->frindex == 0x00004000) {
> -            ehci_raise_irq(ehci, USBSTS_FLR);
> -            ehci->frindex = 0;
> -            if (ehci->usbsts_frindex >= 0x00004000) {
> -                ehci->usbsts_frindex -= 0x00004000;
> -            } else {
> -                ehci->usbsts_frindex = 0;
> -            }
> +    /* How many times will frindex roll over 0x4000 with this frame count?
> +     * usbsts_frindex is decremented by 0x4000 on rollover until it reaches 0
> +     */
> +    int rollovers = (ehci->frindex + uframes) / 0x4000;
> +    if (rollovers > 0) {
> +        if (ehci->usbsts_frindex >= (rollovers * 0x4000)) {
> +            ehci->usbsts_frindex -= 0x4000 * rollovers;
> +        } else {
> +            ehci->usbsts_frindex = 0;
>           }
>       }
> +
> +    ehci->frindex = (ehci->frindex + uframes) % 0x4000;
>   }
>   
>   static void ehci_frame_timer(void *opaque)

ping

Re: [Qemu-devel] [PATCH 1/1] ehci: faster frame index calculation for skipped frames

[Gerd Hoffmann]

On Mi, 2016-07-27 at 19:55 +0300, Denis V. Lunev wrote:
> ehci_update_frindex takes time linearly proportional to a number
> of uframes to calculate new frame index and raise FLR interrupts,
> which is a problem for large amounts of uframes.
> 
> If we experience large delays between echi timer callbacks (i.e.
> because
> other periodic handlers have taken a lot of time to complete) we
> get a lot of skipped frames which then delay ehci timer callback more
> and this leads to deadlocking the system when ehci schedules next
> callback to be too soon.
> 
> Observable behaviour is qemu consuming 100% host CPU time while guest
> is unresponsive. This misbehavior could happen for a while and QEMU
> does
> not get out from this state automatically without the patch.
> 
> This change makes ehci_update_frindex execute in constant time.
> 

queued up for 2.7

thanks,
  Gerd