Re: [RFC PATCH 0/6] job: replace AioContext lock with job_mutex

[Stefan Hajnoczi]

On Mon, Jul 12, 2021 at 10:42:47AM +0200, Emanuele Giuseppe Esposito wrote:
> On 08/07/2021 15:09, Stefan Hajnoczi wrote:
> > On Wed, Jul 07, 2021 at 06:58:07PM +0200, Emanuele Giuseppe Esposito wrote:
> > > This is a continuation on the work to reduce (and possibly get rid of) 
> > > the usage of AioContext lock, by introducing smaller granularity locks to 
> > > keep the thread safety.
> > > 
> > > This series aims to:
> > > 1) remove the aiocontext lock and substitute it with the already existing
> > >     global job_mutex
> > > 2) fix what it looks like to be an oversight when moving the blockjob.c 
> > > logic
> > >     into the more generic job.c: job_mutex was introduced especially to
> > >     protect job->busy flag, but it seems that it was not used in 
> > > successive
> > >     patches, because there are multiple code sections that directly
> > >     access the field without any locking.
> > > 3) use job_mutex instead of the aiocontext_lock
> > > 4) extend the reach of the job_mutex to protect all shared fields
> > >     that the job structure has.
> > 
> > Can you explain the big picture:
> > 
> > 1. What are the rules for JobDrivers? Imagine you are implementing a new
> >     JobDriver. What do you need to know in order to write correct code?
> 
> I think that in general, the rules for JobDrivers remain the same. The
> job_mutex lock should be invisible (or almost) from the point of view of a
> JobDriver, because the job API available for it should take care of the
> necessary locking/unlocking.
> 
> > 
> > 2. What are the rules for monitor? The main pattern is looking up a job,
> >     invoking a job API on it, and then calling job_unlock().
> 
> The monitor instead is aware of this lock: the reason for that is exactly
> what you have described here.
> Looking up + invoking a job API operation (for example calling find_job()
> and then job_pause() ) must be performed with the same lock hold all the
> time, otherwise other threads could modify the job while the monitor runs
> its command.

That helps, thanks!

Stefan

signature.asc
Description: PGP signature