qemu-kvm/kvm-throttle-groups-fix-restart-coroutine-iothread-race.patch
Danilo C. L. de Paula d9dd3f8dd3 * Tue Jan 29 2019 Danilo Cesar Lemes de Paula <ddepaula@redhat.com> - 3.1.0-10.el8
- kvm-throttle-groups-fix-restart-coroutine-iothread-race.patch [bz#1655947]
- kvm-iotests-add-238-for-throttling-tgm-unregister-iothre.patch [bz#1655947]
- Resolves: bz#1655947
  (qemu-kvm core dumped after unplug the device which was set io throttling parameters)
2019-01-29 14:28:22 +00:00

121 lines
4.6 KiB
Diff

From 02287430957782ffb1db0d7d17693a73925ea02f Mon Sep 17 00:00:00 2001
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Tue, 29 Jan 2019 07:02:49 +0000
Subject: [PATCH 1/2] throttle-groups: fix restart coroutine iothread race
RH-Author: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: <20190129070250.22709-2-stefanha@redhat.com>
Patchwork-id: 84139
O-Subject: [RHEL-8.0/AV qemu-kvm PATCH 1/2] throttle-groups: fix restart coroutine iothread race
Bugzilla: 1655947
RH-Acked-by: Stefano Garzarella <sgarzare@redhat.com>
RH-Acked-by: Thomas Huth <thuth@redhat.com>
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
The following QMP command leads to a crash when iothreads are used:
{ 'execute': 'device_del', 'arguments': {'id': 'data'} }
The backtrace involves the queue restart coroutine where
tgm->throttle_state is a NULL pointer because
throttle_group_unregister_tgm() has already been called:
(gdb) bt full
#0 0x00005585a7a3b378 in qemu_mutex_lock_impl (mutex=0xffffffffffffffd0, file=0x5585a7bb3d54 "block/throttle-groups.c", line=412) at util/qemu-thread-posix.c:64
err = <optimized out>
__PRETTY_FUNCTION__ = "qemu_mutex_lock_impl"
__func__ = "qemu_mutex_lock_impl"
#1 0x00005585a79be074 in throttle_group_restart_queue_entry (opaque=0x5585a9de4eb0) at block/throttle-groups.c:412
_f = <optimized out>
data = 0x5585a9de4eb0
tgm = 0x5585a9079440
ts = 0x0
tg = 0xffffffffffffff98
is_write = false
empty_queue = 255
This coroutine should not execute in the iothread after the throttle
group member has been unregistered!
The root cause is that the device_del code path schedules the restart
coroutine in the iothread while holding the AioContext lock. Therefore
the iothread cannot execute the coroutine until after device_del
releases the lock - by this time it's too late.
This patch adds a reference count to ThrottleGroupMember so we can
synchronously wait for restart coroutines to complete. Once they are
done it is safe to unregister the ThrottleGroupMember.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-id: 20190114133257.30299-2-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
(cherry picked from commit bc19a0a6e4505390f99d3c593ebaf11b7962cc59)
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
---
block/throttle-groups.c | 9 +++++++++
include/block/throttle-groups.h | 5 +++++
2 files changed, 14 insertions(+)
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 5d8213a..a5a2037 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -415,6 +415,9 @@ static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
}
g_free(data);
+
+ atomic_dec(&tgm->restart_pending);
+ aio_wait_kick();
}
static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write)
@@ -430,6 +433,8 @@ static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write
* be no timer pending on this tgm at this point */
assert(!timer_pending(tgm->throttle_timers.timers[is_write]));
+ atomic_inc(&tgm->restart_pending);
+
co = qemu_coroutine_create(throttle_group_restart_queue_entry, rd);
aio_co_enter(tgm->aio_context, co);
}
@@ -538,6 +543,7 @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
tgm->throttle_state = ts;
tgm->aio_context = ctx;
+ atomic_set(&tgm->restart_pending, 0);
qemu_mutex_lock(&tg->lock);
/* If the ThrottleGroup is new set this ThrottleGroupMember as the token */
@@ -584,6 +590,9 @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
return;
}
+ /* Wait for throttle_group_restart_queue_entry() coroutines to finish */
+ AIO_WAIT_WHILE(tgm->aio_context, atomic_read(&tgm->restart_pending) > 0);
+
qemu_mutex_lock(&tg->lock);
for (i = 0; i < 2; i++) {
assert(tgm->pending_reqs[i] == 0);
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index e2fd051..712a8e6 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -43,6 +43,11 @@ typedef struct ThrottleGroupMember {
*/
unsigned int io_limits_disabled;
+ /* Number of pending throttle_group_restart_queue_entry() coroutines.
+ * Accessed with atomic operations.
+ */
+ unsigned int restart_pending;
+
/* The following fields are protected by the ThrottleGroup lock.
* See the ThrottleGroup documentation for details.
* throttle_state tells us if I/O limits are configured. */
--
1.8.3.1