|
 |
357786 |
From c933e15569c9d9b3db7031d8c54712f819816f39 Mon Sep 17 00:00:00 2001
|
|
|
357786 |
From: Kevin Wolf <kwolf@redhat.com>
|
|
|
357786 |
Date: Fri, 14 Sep 2018 10:55:39 +0200
|
|
|
357786 |
Subject: [PATCH 48/49] job: Avoid deadlocks in job_completed_txn_abort()
|
|
|
357786 |
|
|
|
357786 |
RH-Author: Kevin Wolf <kwolf@redhat.com>
|
|
|
357786 |
Message-id: <20180914105540.18077-42-kwolf@redhat.com>
|
|
|
357786 |
Patchwork-id: 82194
|
|
|
357786 |
O-Subject: [RHV-7.6 qemu-kvm-rhev PATCH 41/42] job: Avoid deadlocks in job_completed_txn_abort()
|
|
|
357786 |
Bugzilla: 1601212
|
|
|
357786 |
RH-Acked-by: John Snow <jsnow@redhat.com>
|
|
|
357786 |
RH-Acked-by: Max Reitz <mreitz@redhat.com>
|
|
|
357786 |
RH-Acked-by: Fam Zheng <famz@redhat.com>
|
|
|
357786 |
|
|
|
357786 |
Amongst others, job_finalize_single() calls the .prepare/.commit/.abort
|
|
|
357786 |
callbacks of the individual job driver. Recently, their use was adapted
|
|
|
357786 |
for all block jobs so that they involve code calling AIO_WAIT_WHILE()
|
|
|
357786 |
now. Such code must be called under the AioContext lock for the
|
|
|
357786 |
respective job, but without holding any other AioContext lock.
|
|
|
357786 |
|
|
|
357786 |
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
|
357786 |
Reviewed-by: Max Reitz <mreitz@redhat.com>
|
|
|
357786 |
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
|
357786 |
---
|
|
|
357786 |
job.c | 16 +++++++++++-----
|
|
|
357786 |
1 file changed, 11 insertions(+), 5 deletions(-)
|
|
|
357786 |
|
|
|
357786 |
diff --git a/job.c b/job.c
|
|
|
357786 |
index 42af9e2..5b53e43 100644
|
|
|
357786 |
--- a/job.c
|
|
|
357786 |
+++ b/job.c
|
|
|
357786 |
@@ -713,6 +713,7 @@ static void job_cancel_async(Job *job, bool force)
|
|
|
357786 |
|
|
|
357786 |
static void job_completed_txn_abort(Job *job)
|
|
|
357786 |
{
|
|
|
357786 |
+ AioContext *outer_ctx = job->aio_context;
|
|
|
357786 |
AioContext *ctx;
|
|
|
357786 |
JobTxn *txn = job->txn;
|
|
|
357786 |
Job *other_job;
|
|
|
357786 |
@@ -726,23 +727,26 @@ static void job_completed_txn_abort(Job *job)
|
|
|
357786 |
txn->aborting = true;
|
|
|
357786 |
job_txn_ref(txn);
|
|
|
357786 |
|
|
|
357786 |
- /* We are the first failed job. Cancel other jobs. */
|
|
|
357786 |
- QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
|
|
|
357786 |
- ctx = other_job->aio_context;
|
|
|
357786 |
- aio_context_acquire(ctx);
|
|
|
357786 |
- }
|
|
|
357786 |
+ /* We can only hold the single job's AioContext lock while calling
|
|
|
357786 |
+ * job_finalize_single() because the finalization callbacks can involve
|
|
|
357786 |
+ * calls of AIO_WAIT_WHILE(), which could deadlock otherwise. */
|
|
|
357786 |
+ aio_context_release(outer_ctx);
|
|
|
357786 |
|
|
|
357786 |
/* Other jobs are effectively cancelled by us, set the status for
|
|
|
357786 |
* them; this job, however, may or may not be cancelled, depending
|
|
|
357786 |
* on the caller, so leave it. */
|
|
|
357786 |
QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
|
|
|
357786 |
if (other_job != job) {
|
|
|
357786 |
+ ctx = other_job->aio_context;
|
|
|
357786 |
+ aio_context_acquire(ctx);
|
|
|
357786 |
job_cancel_async(other_job, false);
|
|
|
357786 |
+ aio_context_release(ctx);
|
|
|
357786 |
}
|
|
|
357786 |
}
|
|
|
357786 |
while (!QLIST_EMPTY(&txn->jobs)) {
|
|
|
357786 |
other_job = QLIST_FIRST(&txn->jobs);
|
|
|
357786 |
ctx = other_job->aio_context;
|
|
|
357786 |
+ aio_context_acquire(ctx);
|
|
|
357786 |
if (!job_is_completed(other_job)) {
|
|
|
357786 |
assert(job_is_cancelled(other_job));
|
|
|
357786 |
job_finish_sync(other_job, NULL, NULL);
|
|
|
357786 |
@@ -751,6 +755,8 @@ static void job_completed_txn_abort(Job *job)
|
|
|
357786 |
aio_context_release(ctx);
|
|
|
357786 |
}
|
|
|
357786 |
|
|
|
357786 |
+ aio_context_acquire(outer_ctx);
|
|
|
357786 |
+
|
|
|
357786 |
job_txn_unref(txn);
|
|
|
357786 |
}
|
|
|
357786 |
|
|
|
357786 |
--
|
|
|
357786 |
1.8.3.1
|
|
|
357786 |
|