Re: [PATCH v1 2/3] scsi: ufs: Optimize host lock on transfer requests send/compl paths

From: Can Guo
Date: Mon May 24 2021 - 21:47:15 EST



On 2021-05-25 04:10, Bart Van Assche wrote:
On 5/24/21 1:36 AM, Can Guo wrote:
Current UFS IRQ handler is completely wrapped by host lock, and because
ufshcd_send_command() is also protected by host lock, when IRQ handler
fires, not only the CPU running the IRQ handler cannot send new requests,
the rest CPUs can neither. Move the host lock wrapping the IRQ handler into
specific branches, i.e., ufshcd_uic_cmd_compl(), ufshcd_check_errors(),
ufshcd_tmc_handler() and ufshcd_transfer_req_compl(). Meanwhile, to further
reduce occpuation of host lock in ufshcd_transfer_req_compl(), host lock is
no longer required to call __ufshcd_transfer_req_compl(). As per test, the
optimization can bring considerable gain to random read/write performance.

Hi Can,

Using the host lock to serialize the completion path against the
submission path was a common practice 11 years ago, before the host lock
push-down (see also
https://linux-scsi.vger.kernel.narkive.com/UEmGgwAc/rfc-patch-scsi-host-lock-push-down).
Modern SCSI LLDs should not use the SCSI host lock. Please consider
introducing one or more new synchronization objects in struct ufs_hba
and to use these instead of the SCSI host lock. That will save multiple
pointer dereferences in the hot path since hba->host->host_lock will
become hba->new_spin_lock.

An additional question is whether it is necessary for v3.0 UFS devices
to serialize the submission path against the completion path? Multiple
high-performance SCSI LLDs support hardware with separate submission and
completion queues and hence do not need any serialization between the
submission and the completion path. I'm asking this because it is likely
that sooner or later multiqueue support will be added in the UFS
specification. Benefiting from multiqueue support will require to rework
locking in the UFS driver anyway.


Hi Bart,

Agree with all above, and what you ask is right what we are doing in the
3rd change - get rid of host lock on dispatch and completion paths.

I agree with using dedicated spin locks for dedicated purposes in UFS driver,
e.g., clk gating has its own gating_lock and clk scaling has its own scaling_lock.
But this specific series is only for improving performance. We will take your
comments into consideration and address it in future.

Thanks,

Can Guo.

Thanks,

Bart.