qemu-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]