Blame SOURCES/kvm-blockjob-Lie-better-in-child_job_drained_poll.patch

26ba25
From 7c16384f8ce4d46d6baa11db376c488ed8478744 Mon Sep 17 00:00:00 2001
26ba25
From: Kevin Wolf <kwolf@redhat.com>
26ba25
Date: Wed, 10 Oct 2018 20:22:06 +0100
26ba25
Subject: [PATCH 40/49] blockjob: Lie better in child_job_drained_poll()
26ba25
26ba25
RH-Author: Kevin Wolf <kwolf@redhat.com>
26ba25
Message-id: <20181010202213.7372-28-kwolf@redhat.com>
26ba25
Patchwork-id: 82616
26ba25
O-Subject: [RHEL-8 qemu-kvm PATCH 37/44] blockjob: Lie better in child_job_drained_poll()
26ba25
Bugzilla: 1637976
26ba25
RH-Acked-by: Max Reitz <mreitz@redhat.com>
26ba25
RH-Acked-by: John Snow <jsnow@redhat.com>
26ba25
RH-Acked-by: Thomas Huth <thuth@redhat.com>
26ba25
26ba25
Block jobs claim in .drained_poll() that they are in a quiescent state
26ba25
as soon as job->deferred_to_main_loop is true. This is obviously wrong,
26ba25
they still have a completion BH to run. We only get away with this
26ba25
because commit 91af091f923 added an unconditional aio_poll(false) to the
26ba25
drain functions, but this is bypassing the regular drain mechanisms.
26ba25
26ba25
However, just removing this and telling that the job is still active
26ba25
doesn't work either: The completion callbacks themselves call drain
26ba25
functions (directly, or indirectly with bdrv_reopen), so they would
26ba25
deadlock then.
26ba25
26ba25
As a better lie, tell that the job is active as long as the BH is
26ba25
pending, but falsely call it quiescent from the point in the BH when the
26ba25
completion callback is called. At this point, nested drain calls won't
26ba25
deadlock because they ignore the job, and outer drains will wait for the
26ba25
job to really reach a quiescent state because the callback is already
26ba25
running.
26ba25
26ba25
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
26ba25
Reviewed-by: Max Reitz <mreitz@redhat.com>
26ba25
(cherry picked from commit b5a7a0573530698ee448b063ac01d485e30446bd)
26ba25
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
26ba25
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
26ba25
---
26ba25
 blockjob.c         |  2 +-
26ba25
 include/qemu/job.h |  3 +++
26ba25
 job.c              | 11 ++++++++++-
26ba25
 3 files changed, 14 insertions(+), 2 deletions(-)
26ba25
26ba25
diff --git a/blockjob.c b/blockjob.c
26ba25
index 8d27e8e..617d86f 100644
26ba25
--- a/blockjob.c
26ba25
+++ b/blockjob.c
26ba25
@@ -164,7 +164,7 @@ static bool child_job_drained_poll(BdrvChild *c)
26ba25
     /* An inactive or completed job doesn't have any pending requests. Jobs
26ba25
      * with !job->busy are either already paused or have a pause point after
26ba25
      * being reentered, so no job driver code will run before they pause. */
26ba25
-    if (!job->busy || job_is_completed(job) || job->deferred_to_main_loop) {
26ba25
+    if (!job->busy || job_is_completed(job)) {
26ba25
         return false;
26ba25
     }
26ba25
 
26ba25
diff --git a/include/qemu/job.h b/include/qemu/job.h
26ba25
index 35ac7a9..d1710f3 100644
26ba25
--- a/include/qemu/job.h
26ba25
+++ b/include/qemu/job.h
26ba25
@@ -76,6 +76,9 @@ typedef struct Job {
26ba25
      * Set to false by the job while the coroutine has yielded and may be
26ba25
      * re-entered by job_enter(). There may still be I/O or event loop activity
26ba25
      * pending. Accessed under block_job_mutex (in blockjob.c).
26ba25
+     *
26ba25
+     * When the job is deferred to the main loop, busy is true as long as the
26ba25
+     * bottom half is still pending.
26ba25
      */
26ba25
     bool busy;
26ba25
 
26ba25
diff --git a/job.c b/job.c
26ba25
index 47b5a11..42af9e2 100644
26ba25
--- a/job.c
26ba25
+++ b/job.c
26ba25
@@ -852,7 +852,16 @@ static void job_exit(void *opaque)
26ba25
     AioContext *ctx = job->aio_context;
26ba25
 
26ba25
     aio_context_acquire(ctx);
26ba25
+
26ba25
+    /* This is a lie, we're not quiescent, but still doing the completion
26ba25
+     * callbacks. However, completion callbacks tend to involve operations that
26ba25
+     * drain block nodes, and if .drained_poll still returned true, we would
26ba25
+     * deadlock. */
26ba25
+    job->busy = false;
26ba25
+    job_event_idle(job);
26ba25
+
26ba25
     job_completed(job);
26ba25
+
26ba25
     aio_context_release(ctx);
26ba25
 }
26ba25
 
26ba25
@@ -867,8 +876,8 @@ static void coroutine_fn job_co_entry(void *opaque)
26ba25
     assert(job && job->driver && job->driver->run);
26ba25
     job_pause_point(job);
26ba25
     job->ret = job->driver->run(job, &job->err);
26ba25
-    job_event_idle(job);
26ba25
     job->deferred_to_main_loop = true;
26ba25
+    job->busy = true;
26ba25
     aio_bh_schedule_oneshot(qemu_get_aio_context(), job_exit, job);
26ba25
 }
26ba25
 
26ba25
-- 
26ba25
1.8.3.1
26ba25