Re: [PATCH v7 01/18] job.c: make job_mutex and job_lock/unlock() public

[Vladimir Sementsov-Ogievskiy]

I've already acked this (honestly, because Stefan do), but still, want to 
clarify:

On 6/16/22 16:18, Emanuele Giuseppe Esposito wrote:
> job mutex will be used to protect the job struct elements and list,
> replacing AioContext locks.
>
> Right now use a shared lock for all jobs, in order to keep things
> simple. Once the AioContext lock is gone, we can introduce per-job
> locks.
>
> To simplify the switch from aiocontext to job lock, introduce
> *nop*  lock/unlock functions and macros.
> We want to always call job_lock/unlock outside the AioContext locks,
> and not vice-versa, otherwise we might get a deadlock.

Could you describe here, why we get a deadlock?

As I understand, we'll deadlock if two code paths exist simultaneously:

1. we take job mutex under aiocontext lock
2. we take aiocontex lock under job mutex

If these paths exists, it's possible that one thread goes through [1] and 
another through [2]. If thread [1] holds job-mutex and want to take 
aiocontext-lock, and in the same time thread [2] holds aiocontext-lock and want 
to take job-mutext, that's a dead-lock.

If you say, that we must avoid [1], do you have in mind that we have [2] 
somewhere? If so, this should be mentioned here.

If not, could we just make a normal mutex, not a noop?

> This is not
> straightforward to do, and that's why we start with nop functions.
> Once everything is protected by job_lock/unlock, we can change the nop into
> an actual mutex and remove the aiocontext lock.
>
> Since job_mutex is already being used, add static
> real_job_{lock/unlock} for the existing usage.
>
> Signed-off-by: Emanuele Giuseppe Esposito<eesposit@redhat.com>
> Reviewed-by: Stefan Hajnoczi<stefanha@redhat.com>


--
Best regards,
Vladimir