[Qemu-devel] [PATCH for-2.11] throttle-groups: forget timer and schedule next TGM on detach

Stefan Hajnoczi posted 1 patch 6 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171116112150.27607-1-stefanha@redhat.com
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
block/throttle-groups.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
[Qemu-devel] [PATCH for-2.11] throttle-groups: forget timer and schedule next TGM on detach
Posted by Stefan Hajnoczi 6 years, 5 months ago
tg->any_timer_armed[] must be cleared when detaching pending timers from
the AioContext.  Failure to do so leads to hung I/O because it looks
like there are still timers pending when in fact they have been removed.

Other ThrottleGroupMembers might have requests pending too so it's
necessary to schedule the next TGM so it can set a timer.

This patch fixes hung I/O when QEMU is launched with drives that are in
the same throttling group:

  (guest)$ dd if=/dev/zero of=/dev/vdb oflag=direct bs=512 &
  (guest)$ dd if=/dev/zero of=/dev/vdc oflag=direct bs=512 &
  (qemu) stop
  (qemu) cont
  ...I/O is stuck...

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/throttle-groups.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 2587f19ca3..f26bcb5eee 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -593,13 +593,25 @@ void throttle_group_attach_aio_context(ThrottleGroupMember *tgm,
 
 void throttle_group_detach_aio_context(ThrottleGroupMember *tgm)
 {
+    ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts);
     ThrottleTimers *tt = &tgm->throttle_timers;
+    int i;
 
     /* Requests must have been drained */
     assert(tgm->pending_reqs[0] == 0 && tgm->pending_reqs[1] == 0);
     assert(qemu_co_queue_empty(&tgm->throttled_reqs[0]));
     assert(qemu_co_queue_empty(&tgm->throttled_reqs[1]));
 
+    /* Kick off next ThrottleGroupMember, if necessary */
+    qemu_mutex_lock(&tg->lock);
+    for (i = 0; i < 2; i++) {
+        if (timer_pending(tt->timers[i])) {
+            tg->any_timer_armed[i] = false;
+            schedule_next_request(tgm, i);
+        }
+    }
+    qemu_mutex_unlock(&tg->lock);
+
     throttle_timers_detach_aio_context(tt);
     tgm->aio_context = NULL;
 }
-- 
2.13.6


Re: [Qemu-devel] [PATCH for-2.11] throttle-groups: forget timer and schedule next TGM on detach
Posted by Alberto Garcia 6 years, 5 months ago
On Thu 16 Nov 2017 12:21:50 PM CET, Stefan Hajnoczi wrote:
> tg->any_timer_armed[] must be cleared when detaching pending timers from
> the AioContext.  Failure to do so leads to hung I/O because it looks
> like there are still timers pending when in fact they have been removed.
>
> Other ThrottleGroupMembers might have requests pending too so it's
> necessary to schedule the next TGM so it can set a timer.
>
> This patch fixes hung I/O when QEMU is launched with drives that are in
> the same throttling group:
>
>   (guest)$ dd if=/dev/zero of=/dev/vdb oflag=direct bs=512 &
>   (guest)$ dd if=/dev/zero of=/dev/vdc oflag=direct bs=512 &
>   (qemu) stop
>   (qemu) cont
>   ...I/O is stuck...
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [Qemu-block] [PATCH for-2.11] throttle-groups: forget timer and schedule next TGM on detach
Posted by Stefan Hajnoczi 6 years, 5 months ago
On Thu, Nov 16, 2017 at 11:21:50AM +0000, Stefan Hajnoczi wrote:
> tg->any_timer_armed[] must be cleared when detaching pending timers from
> the AioContext.  Failure to do so leads to hung I/O because it looks
> like there are still timers pending when in fact they have been removed.
> 
> Other ThrottleGroupMembers might have requests pending too so it's
> necessary to schedule the next TGM so it can set a timer.
> 
> This patch fixes hung I/O when QEMU is launched with drives that are in
> the same throttling group:
> 
>   (guest)$ dd if=/dev/zero of=/dev/vdb oflag=direct bs=512 &
>   (guest)$ dd if=/dev/zero of=/dev/vdc oflag=direct bs=512 &
>   (qemu) stop
>   (qemu) cont
>   ...I/O is stuck...
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/throttle-groups.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan