|
From: | Emanuele Giuseppe Esposito |
Subject: | Re: [RFC PATCH 0/6] job: replace AioContext lock with job_mutex |
Date: | Mon, 12 Jul 2021 10:42:47 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 |
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.
Please let me know if something is not clear and/or if you have additional comments on this!
Emanuele
Stefan
[Prev in Thread] | Current Thread | [Next in Thread] |