Re: [RFC PATCH v2 11/14] block_job_query: remove atomic read

[Vladimir Sementsov-Ogievskiy]

04.11.2021 17:53, Emanuele Giuseppe Esposito wrote:
> Not sure what the atomic here was supposed to do, since job.busy
> is protected by the job lock.

In block_job_query() it is protected only since previous commit. So, before 
previous commit, atomic read make sense.

Hmm. but job_lock() is still a no-op at this point. So, actually, it would be 
more correct to drop this qatomic_read after patch 14.

> Since the whole function will
> be called under job_mutex, just remove the atomic.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   blockjob.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/blockjob.c b/blockjob.c
> index dcc13dc336..426dcddcc1 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -314,6 +314,7 @@ int64_t block_job_ratelimit_get_delay(BlockJob *job, 
> uint64_t n)
>       return ratelimit_calculate_delay(&job->limit, n);
>   }
>   
> +/* Called with job_mutex held */
>   BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
>   {
>       BlockJobInfo *info;
> @@ -332,13 +333,13 @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
>       info = g_new0(BlockJobInfo, 1);
>       info->type      = g_strdup(job_type_str(&job->job));
>       info->device    = g_strdup(job->job.id);
> -    info->busy      = qatomic_read(&job->job.busy);
> +    info->busy      = job->job.busy;
>       info->paused    = job->job.pause_count > 0;
>       info->offset    = progress_current;
>       info->len       = progress_total;
>       info->speed     = job->speed;
>       info->io_status = job->iostatus;
> -    info->ready     = job_is_ready(&job->job),
> +    info->ready     = job_is_ready_locked(&job->job),
>       info->status    = job->job.status;
>       info->auto_finalize = job->job.auto_finalize;
>       info->auto_dismiss  = job->job.auto_dismiss;


--
Best regards,
Vladimir