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.
