Re: [PATCH v1 5/6] scsi: ufs: Let host_sem cover the entire system suspend/resume

[Bart Van Assche <bvanassche@acm.org>]

On 5/16/21 8:22 PM, Can Guo wrote:
> Hi Bart,
> 
> On 2021-05-14 11:55, Bart Van Assche wrote:
>> On 5/12/21 10:55 PM, Can Guo wrote:
>>> UFS error handling now is doing more than just re-probing, but also
>>> sending
>>> scsi cmds, e.g., for clearing UACs, and recovering runtime PM error,
>>> which
>>> may change runtime status of scsi devices. To protect system
>>> suspend/resume
>>> from being disturbed by error handling, move the host_sem from wl pm ops
>>> to ufshcd_suspend_prepare() and ufshcd_resume_complete().
>>
>> In ufshcd.h I found the following:
>>
>>  * @host_sem: semaphore used to serialize concurrent contexts
>>
>> That's the wrong way to use a synchronization object. A synchronization
>> object must protect data instead of code. Does host_sem perhaps need to
>> be split into multiple synchronization objects?
> 
> Thanks for the comments. These contexts are changing critical data and
> registers, so the sem is used to protect data actually, just like the
> scaling_lock protecting scaling and cmd transations.

But where is the documentation that explains which data members are
protected by hba->host_sem and which data members are protected by
hba->host->host_lock? Was the host_lock protection perhaps introduced
before scsi-mq was introduced? Before scsi-mq acquiring the host_lock
was sufficient to serialize against ufshcd_queuecommand() but that is
not sufficient when using scsi-mq.

I want to verify whether locking is used correctly in the UFS driver but
without documentation of which synchronization object protects which
data members that is not possible.

Thanks,

Bart.