On 6/26/2024 12:13 AM, Bart Van Assche wrote:
On 6/24/24 8:38 PM, Peter Wang (王信友) wrote:
But ufshcd_scsi_block_requests usage is correct in SDR mode.
ufshcd_scsi_block_requests() uses scsi_block_requests(). It is almost
never correct to use scsi_block_requests() in a blk-mq driver because
scsi_block_requests() does not wait for ongoing request submission
calls to complete. scsi_block_requests() is a legacy from the time when
all request dispatching and queueing was protected by the SCSI host
lock, a change that was made in 2010 or about 14 years ago. See also
https://lore.kernel.org/linux-scsi/20101105002409.GA21714@xxxxxxxxxxxxx/
So, I think ufshcd_wait_for_doorbell_clr should be revise.
Check tr_doorbell in SDR mode. (before 8d077ede48c1 do)
Check each HWQ's are all empty in MCQ mode. (need think how to do)
Make sure all requests is complete, and finish this function' job
correctly.
Or there still have a gap in ufshcd_wait_for_doorbell_clr.
ufshcd_wait_for_doorbell_clr() should be removed and
ufshcd_clock_scaling_prepare() should use blk_mq_freeze_*().
See also my patch "ufs: Simplify the clock scaling mechanism
implementation" from 5 years ago
(https://lore.kernel.org/linux-scsi/20191112173743.141503-5-bvanassche@xxxxxxx/).
The defect of blk_mq_freeze_*() is that it would bring in significant
latency and performance regression. I don't think it is what many people
want to see.
BRs,
Ziqi
Best regards,
Bart.
