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

[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 */
Notifier finalize_cancelled_notifier;
Notifier finalize_completed_notifier;
Notifier pending_notifier;
Notifier ready_notifier;
Notifier idle_notifier;

Not sure about blockjob->speed though.

Emanuele