job: Context changes in job_completed_txn_abort()
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: Hanna Reitz <hreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20211006151940.214590-2-hreitz@redhat.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
This commit is contained in:
parent
a9515df4d6
commit
d431131439
22
job.c
22
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,12 @@ 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().
|
||||
*/
|
||||
aio_context_acquire(job->aio_context);
|
||||
job_unref(job);
|
||||
|
||||
job_txn_unref(txn);
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user