On 7/6/22 23:15, Emanuele Giuseppe Esposito wrote:
Now that the API offers also _locked() functions, take advantage
of it and give also the caller control to take the lock and call
_locked functions.
This makes sense especially when we have for loops, because it
makes no sense to have:
for(job = job_next(); ...)
where each job_next() takes the lock internally.
Instead we want
JOB_LOCK_GUARD();
for(job = job_next_locked(); ...)
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com>
---
[..]
diff --git a/blockjob.c b/blockjob.c
index 0d59aba439..bce05a9096 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -99,7 +99,9 @@ static char *child_job_get_parent_desc(BdrvChild *c)
static void child_job_drained_begin(BdrvChild *c)
{
BlockJob *job = c->opaque;
- job_pause(&job->job);
+ WITH_JOB_LOCK_GUARD() {
+ job_pause_locked(&job->job);
+ }
What's the reason for it? I'd keep job_pause().
(and it doesn't correspond to what commit subject presents.)
}
static bool child_job_drained_poll(BdrvChild *c)
@@ -111,8 +113,10 @@ static bool child_job_drained_poll(BdrvChild *c)
/* An inactive or completed job doesn't have any pending requests. Jobs
* with !job->busy are either already paused or have a pause point after
* being reentered, so no job driver code will run before they pause. */
- if (!job->busy || job_is_completed(job)) {
- return false;
+ WITH_JOB_LOCK_GUARD() {
+ if (!job->busy || job_is_completed_locked(job)) {
+ return false;
+ }
}
This doesn't correspond to commit subject. I'd put such things to separate commit
"correct use of job_mutex in blockjob.c".
/* Otherwise, assume that it isn't fully stopped yet, but allow the job to
@@ -127,7 +131,9 @@ static bool child_job_drained_poll(BdrvChild *c)
static void child_job_drained_end(BdrvChild *c, int *drained_end_counter)
{
BlockJob *job = c->opaque;
- job_resume(&job->job);
+ WITH_JOB_LOCK_GUARD() {
+ job_resume_locked(&job->job);
+ }
}
Again, don't see a reason for such change.
[my comments relate to more similar cases in the patch]
--
Best regards,
Vladimir