Re: [PATCH v8 08/20] blockjob.h: introduce block_job _locked() APIs

[Emanuele Giuseppe Esposito]

Am 06/07/2022 um 14:36 schrieb Emanuele Giuseppe Esposito:
> 
> 
> Am 06/07/2022 um 14:23 schrieb Vladimir Sementsov-Ogievskiy:
>> On 7/6/22 15:05, Emanuele Giuseppe Esposito wrote:
>>>
>>>
>>> Am 05/07/2022 um 17:01 schrieb Vladimir Sementsov-Ogievskiy:
>>>> On 6/29/22 17:15, Emanuele Giuseppe Esposito wrote:
>>>>> Just as done with job.h, create _locked() functions in blockjob.h
>>>>
>>>> We modify not only blockjob.h, I'd s/blockjob.h/blockjob/ in subject.
>>>>
>>>> Also, we start to introduce _locked block_job_* APIs.
>>>>
>>>> Does it mean that BlockJob and Job share the global mutex to protect
>>>> themselves? Than I think we should document in BlockJob struct what is
>>>> protected by job_mutex.
>>>
>>> There is nothing in the struct (apart from Job) that is protected by the
>>> job lock. I can add a comment "Protected by job mutex" on top of Job job
>>> field?
>>
>> Yes, I think that's worth doing.
>>
>> Other fields doesn't need the lock?
>>
> Well I didn't plan to actually look at it but now that you ask:
> 
> /** needs protection, so it can go under job lock */
> BlockDeviceIoStatus iostatus;
> 
> /** mostly under lock, not sure when it is called as notifier callback
> though. I think they are GLOBAL_STATE, what do you think?  */
> int64_t speed;
> 
> /** thread safe API */
> RateLimit limit;
> 
> /** I think it's also thread safe */
> Error *blocker;
> 
> /* always under job lock */
Actually that's wrong, they are just set once and never modified.

And GSList *nodes; is also always called under GS.

So there's only iostatus to protect and maybe speed.

Emanuele

> Notifier finalize_cancelled_notifier;
> Notifier finalize_completed_notifier;
> Notifier pending_notifier;
> Notifier ready_notifier;
> Notifier idle_notifier;
> 
> Not sure about blockjob->speed though.
> 
> Emanuele
>