[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 17/42] job: Move defer_to_main_loop to Job
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 17/42] job: Move defer_to_main_loop to Job |
Date: |
Tue, 15 May 2018 14:22:35 +0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
Am 15.05.2018 um 00:33 hat John Snow geschrieben:
>
>
> On 05/09/2018 12:26 PM, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <address@hidden>
>
> Hmm, this one is a bit more than just code motion due to the way the
> aio_context acquisition has changed. I think at a minimum a good commit
> message is warranted.
I know it took a while to convince myself that it's right. I wonder
where the commit message has gone...
> > @@ -170,3 +171,35 @@ void job_unref(Job *job)
> > g_free(job);
> > }
> > }
> > +
> > +typedef struct {
> > + Job *job;
> > + JobDeferToMainLoopFn *fn;
> > + void *opaque;
> > +} JobDeferToMainLoopData;
> > +
> > +static void job_defer_to_main_loop_bh(void *opaque)
> > +{
> > + JobDeferToMainLoopData *data = opaque;
> > + Job *job = data->job;
> > + AioContext *aio_context = job->aio_context;
> > +
> > + /* Prevent race with job_defer_to_main_loop() */
> > + aio_context_acquire(aio_context);
> > + data->fn(data->job, data->opaque);
> > + aio_context_release(aio_context);
> > +
> > + g_free(data);
> > +}
> > +
>
> This function showed up in '14, with dec7d42 from Stefan. His first
> draft looked like Kevin's, until:
>
> https://lists.gnu.org/archive/html/qemu-devel/2014-10/msg00111.html
>
> Max, from 2014:
>
> "I'm not so sure whether it'd be possible to change the BDS's AIO
> context (in another thread) after the call to bdrv_get_aio_context() in
> block_job_defer_to_main_loop() and before
> block_job_defer_to_main_loop_bh() is run. If so, this might create race
> conditions."
>
> Err, I dunno either.
This one at least is not a problem in the new code any more because we
have job->aio_context now, which is updated when the BDS's context
changes. We read job->aio_context only in the BH, so we always get the
current one.
In contrast, the old code put it into BlockJobDeferToMainLoopData at
the time when the BH was scheduled, so it could be outdated when the BH
actually ran.
Kevin
[Qemu-devel] [PATCH 21/42] job: Replace BlockJob.completed with job_is_completed(), Kevin Wolf, 2018/05/09
[Qemu-devel] [PATCH 23/42] blockjob: Split block_job_event_pending(), Kevin Wolf, 2018/05/09
[Qemu-devel] [PATCH 19/42] job: Add job_sleep_ns(), Kevin Wolf, 2018/05/09