[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH for-6.2 v3 01/12] job: Context changes in job_completed_txn_abort
From: |
Max Reitz |
Subject: |
[PATCH for-6.2 v3 01/12] job: Context changes in job_completed_txn_abort() |
Date: |
Fri, 6 Aug 2021 11:38:48 +0200 |
Finalizing the job may cause its AioContext to change. This is noted by
job_exit(), which points at job_txn_apply() to take this fact into
account.
However, job_completed() does not necessarily invoke job_txn_apply()
(through job_completed_txn_success()), but potentially also
job_completed_txn_abort(). The latter stores the context in a local
variable, and so always acquires the same context at its end that it has
released in the beginning -- which may be a different context from the
one that job_exit() releases at its end. If it is different, qemu
aborts ("qemu_mutex_unlock_impl: Operation not permitted").
Drop the local @outer_ctx variable from job_completed_txn_abort(), and
instead re-acquire the actual job's context at the end of the function,
so job_exit() will release the same.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
job.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/job.c b/job.c
index e7a5d28854..3fe23bb77e 100644
--- a/job.c
+++ b/job.c
@@ -737,7 +737,6 @@ static void job_cancel_async(Job *job, bool force)
static void job_completed_txn_abort(Job *job)
{
- AioContext *outer_ctx = job->aio_context;
AioContext *ctx;
JobTxn *txn = job->txn;
Job *other_job;
@@ -751,10 +750,14 @@ static void job_completed_txn_abort(Job *job)
txn->aborting = true;
job_txn_ref(txn);
- /* We can only hold the single job's AioContext lock while calling
+ /*
+ * We can only hold the single job's AioContext lock while calling
* job_finalize_single() because the finalization callbacks can involve
- * calls of AIO_WAIT_WHILE(), which could deadlock otherwise. */
- aio_context_release(outer_ctx);
+ * calls of AIO_WAIT_WHILE(), which could deadlock otherwise.
+ * Note that the job's AioContext may change when it is finalized.
+ */
+ job_ref(job);
+ aio_context_release(job->aio_context);
/* Other jobs are effectively cancelled by us, set the status for
* them; this job, however, may or may not be cancelled, depending
@@ -769,6 +772,10 @@ static void job_completed_txn_abort(Job *job)
}
while (!QLIST_EMPTY(&txn->jobs)) {
other_job = QLIST_FIRST(&txn->jobs);
+ /*
+ * The job's AioContext may change, so store it in @ctx so we
+ * release the same context that we have acquired before.
+ */
ctx = other_job->aio_context;
aio_context_acquire(ctx);
if (!job_is_completed(other_job)) {
@@ -779,7 +786,13 @@ static void job_completed_txn_abort(Job *job)
aio_context_release(ctx);
}
- aio_context_acquire(outer_ctx);
+ /*
+ * Use job_ref()/job_unref() so we can read the AioContext here
+ * even if the job went away during job_finalize_single().
+ */
+ ctx = job->aio_context;
+ job_unref(job);
+ aio_context_acquire(ctx);
job_txn_unref(txn);
}
--
2.31.1
- [PATCH for-6.2 v3 00/12] mirror: Handle errors after READY cancel, Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 01/12] job: Context changes in job_completed_txn_abort(),
Max Reitz <=
- [PATCH for-6.2 v3 02/12] mirror: Keep s->synced on error, Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 03/12] mirror: Drop s->synced, Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 04/12] job: Force-cancel jobs in a failed transaction, Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 05/12] job: @force parameter for job_cancel_sync{, _all}(), Max Reitz, 2021/08/06
- [PATCH for-6.2 v3 06/12] jobs: Give Job.force_cancel more meaning, Max Reitz, 2021/08/06