From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
qemu-block@nongnu.org, qemu-devel@nongnu.org,
Wen Congyang <wencongyang2@huawei.com>,
Xie Changlong <xiechanglong.d@gmail.com>,
Markus Armbruster <armbru@redhat.com>,
Max Reitz <mreitz@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [RFC PATCH 0/6] job: replace AioContext lock with job_mutex
Date: Mon, 12 Jul 2021 10:42:47 +0200 [thread overview]
Message-ID: <1f85b7d3-41fe-e4e0-ec44-b3e1c22293ea@redhat.com> (raw)
In-Reply-To: <YOb5EZndlckpruhR@stefanha-x1.localdomain>
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
>
next prev parent reply other threads:[~2021-07-12 8:43 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-07 16:58 [RFC PATCH 0/6] job: replace AioContext lock with job_mutex Emanuele Giuseppe Esposito
2021-07-07 16:58 ` [RFC PATCH 1/6] job: use getter/setters instead of accessing the Job fields directly Emanuele Giuseppe Esposito
2021-07-07 16:58 ` [RFC PATCH 2/6] job: _locked functions and public job_lock/unlock for next patch Emanuele Giuseppe Esposito
2021-07-08 10:50 ` Stefan Hajnoczi
2021-07-12 8:43 ` Emanuele Giuseppe Esposito
2021-07-13 13:32 ` Stefan Hajnoczi
2021-07-07 16:58 ` [RFC PATCH 3/6] job: minor changes to simplify locking Emanuele Giuseppe Esposito
2021-07-08 10:55 ` Stefan Hajnoczi
2021-07-12 8:43 ` Emanuele Giuseppe Esposito
2021-07-13 17:56 ` Eric Blake
2021-07-07 16:58 ` [RFC PATCH 4/6] job.h: categorize job fields Emanuele Giuseppe Esposito
2021-07-08 11:02 ` Stefan Hajnoczi
2021-07-12 8:43 ` Emanuele Giuseppe Esposito
2021-07-07 16:58 ` [RFC PATCH 5/6] job: use global job_mutex to protect struct Job Emanuele Giuseppe Esposito
2021-07-08 12:56 ` Stefan Hajnoczi
2021-07-12 8:43 ` Emanuele Giuseppe Esposito
2021-07-07 16:58 ` [RFC PATCH 6/6] jobs: remove unnecessary AioContext aquire/release pairs Emanuele Giuseppe Esposito
2021-07-08 10:36 ` [RFC PATCH 0/6] job: replace AioContext lock with job_mutex Stefan Hajnoczi
2021-07-08 11:32 ` Paolo Bonzini
2021-07-08 12:14 ` Kevin Wolf
2021-07-08 13:04 ` Stefan Hajnoczi
2021-07-12 8:41 ` Emanuele Giuseppe Esposito
2021-07-13 13:10 ` Stefan Hajnoczi
2021-07-13 15:18 ` Vladimir Sementsov-Ogievskiy
2021-07-13 16:38 ` Stefan Hajnoczi
2021-07-15 12:35 ` Vladimir Sementsov-Ogievskiy
2021-07-15 13:29 ` Stefan Hajnoczi
2021-07-16 15:23 ` Kevin Wolf
2021-07-19 9:29 ` Stefan Hajnoczi
2021-07-19 14:54 ` Kevin Wolf
2021-07-08 13:09 ` Stefan Hajnoczi
2021-07-12 8:42 ` Emanuele Giuseppe Esposito [this message]
2021-07-13 13:27 ` Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1f85b7d3-41fe-e4e0-ec44-b3e1c22293ea@redhat.com \
--to=eesposit@redhat.com \
--cc=armbru@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=vsementsov@virtuozzo.com \
--cc=wencongyang2@huawei.com \
--cc=xiechanglong.d@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.