From: Can Guo <cang@codeaurora.org> To: Bart Van Assche <bvanassche@acm.org> Cc: asutoshd@codeaurora.org, nguyenb@codeaurora.org, hongwus@codeaurora.org, linux-scsi@vger.kernel.org, kernel-team@android.com, Stanley Chu <stanley.chu@mediatek.com>, Alim Akhtar <alim.akhtar@samsung.com>, Avri Altman <avri.altman@wdc.com>, "James E.J. Bottomley" <jejb@linux.ibm.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, Matthias Brugger <matthias.bgg@gmail.com>, Bean Huo <beanhuo@micron.com>, Jaegeuk Kim <jaegeuk@kernel.org>, Adrian Hunter <adrian.hunter@intel.com>, Kiwoong Kim <kwmad.kim@samsung.com>, Satya Tangirala <satyat@google.com>, open list <linux-kernel@vger.kernel.org>, "moderated list:ARM/Mediatek SoC support" <linux-arm-kernel@lists.infradead.org>, "moderated list:ARM/Mediatek SoC support" <linux-mediatek@lists.infradead.org> Subject: Re: [PATCH v1 2/3] scsi: ufs: Optimize host lock on transfer requests send/compl paths Date: Tue, 25 May 2021 09:40:18 +0800 [thread overview] Message-ID: <0cfbf580e340073ff972be493a59dbe7@codeaurora.org> (raw) In-Reply-To: <41a08b3e-122d-4f1a-abbd-4b5730f880b2@acm.org> 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.
WARNING: multiple messages have this Message-ID (diff)
From: Can Guo <cang@codeaurora.org> To: Bart Van Assche <bvanassche@acm.org> Cc: asutoshd@codeaurora.org, nguyenb@codeaurora.org, hongwus@codeaurora.org, linux-scsi@vger.kernel.org, kernel-team@android.com, Stanley Chu <stanley.chu@mediatek.com>, Alim Akhtar <alim.akhtar@samsung.com>, Avri Altman <avri.altman@wdc.com>, "James E.J. Bottomley" <jejb@linux.ibm.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, Matthias Brugger <matthias.bgg@gmail.com>, Bean Huo <beanhuo@micron.com>, Jaegeuk Kim <jaegeuk@kernel.org>, Adrian Hunter <adrian.hunter@intel.com>, Kiwoong Kim <kwmad.kim@samsung.com>, Satya Tangirala <satyat@google.com>, open list <linux-kernel@vger.kernel.org>, "moderated list:ARM/Mediatek SoC support" <linux-arm-kernel@lists.infradead.org>, "moderated list:ARM/Mediatek SoC support" <linux-mediatek@lists.infradead.org> Subject: Re: [PATCH v1 2/3] scsi: ufs: Optimize host lock on transfer requests send/compl paths Date: Tue, 25 May 2021 09:40:18 +0800 [thread overview] Message-ID: <0cfbf580e340073ff972be493a59dbe7@codeaurora.org> (raw) In-Reply-To: <41a08b3e-122d-4f1a-abbd-4b5730f880b2@acm.org> 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. _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek
next prev parent reply other threads:[~2021-05-25 1:40 UTC|newest] Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-24 8:36 [PATCH v1 0/3] Optimize host lock on TR send/compl paths and utilize UTRLCNR Can Guo 2021-05-24 8:36 ` [PATCH v1 1/3] scsi: ufs: Remove a redundant command completion logic in error handler Can Guo 2021-05-24 16:43 ` Bart Van Assche 2021-05-25 4:15 ` Stanley Chu 2021-05-31 7:14 ` Bean Huo 2021-05-24 8:36 ` [PATCH v1 2/3] scsi: ufs: Optimize host lock on transfer requests send/compl paths Can Guo 2021-05-24 8:36 ` Can Guo 2021-05-24 8:36 ` Can Guo 2021-05-24 11:25 ` kernel test robot 2021-05-24 11:25 ` kernel test robot 2021-06-08 17:53 ` Nathan Chancellor 2021-06-08 17:53 ` Nathan Chancellor 2021-06-09 1:01 ` Can Guo 2021-06-09 1:01 ` Can Guo 2021-05-24 20:10 ` Bart Van Assche 2021-05-24 20:10 ` Bart Van Assche 2021-05-24 20:10 ` Bart Van Assche 2021-05-25 1:34 ` Asutosh Das (asd) 2021-05-25 1:34 ` Asutosh Das (asd) 2021-05-25 8:24 ` Avri Altman 2021-05-25 8:24 ` Avri Altman 2021-05-25 8:24 ` Avri Altman 2021-05-28 7:30 ` Avri Altman 2021-05-28 7:30 ` Avri Altman 2021-05-28 7:30 ` Avri Altman 2021-06-02 21:18 ` Asutosh Das (asd) 2021-06-02 21:18 ` Asutosh Das (asd) 2021-05-25 1:40 ` Can Guo [this message] 2021-05-25 1:40 ` Can Guo 2021-05-25 16:40 ` Bart Van Assche 2021-05-25 16:40 ` Bart Van Assche 2021-05-25 16:40 ` Bart Van Assche 2021-05-31 16:04 ` Bean Huo 2021-05-31 16:04 ` Bean Huo 2021-05-31 16:04 ` Bean Huo 2021-06-02 2:14 ` Can Guo 2021-06-02 2:14 ` Can Guo 2021-06-03 0:18 ` Bart Van Assche 2021-06-03 0:18 ` Bart Van Assche 2021-06-03 0:18 ` Bart Van Assche 2021-06-03 2:54 ` Stanley Chu 2021-06-03 2:54 ` Stanley Chu 2021-06-03 2:54 ` Stanley Chu 2021-06-04 1:49 ` Can Guo 2021-06-04 1:49 ` Can Guo 2021-06-17 2:49 ` Bart Van Assche 2021-06-17 2:49 ` Bart Van Assche 2021-06-17 2:49 ` Bart Van Assche 2021-06-23 2:04 ` Can Guo 2021-06-23 2:04 ` Can Guo 2021-06-28 22:58 ` Bart Van Assche 2021-06-28 22:58 ` Bart Van Assche 2021-06-28 22:58 ` Bart Van Assche 2021-06-29 5:41 ` Can Guo 2021-06-29 5:41 ` Can Guo 2021-07-01 15:57 ` Bart Van Assche 2021-07-01 15:57 ` Bart Van Assche 2021-07-01 15:57 ` Bart Van Assche 2021-05-24 8:36 ` [PATCH v1 3/3] scsi: ufs: Utilize Transfer Request List Completion Notification Register Can Guo 2021-05-24 8:36 ` Can Guo 2021-05-24 8:36 ` Can Guo 2021-05-31 16:05 ` Bean Huo 2021-05-31 16:05 ` Bean Huo 2021-05-31 16:05 ` Bean Huo 2021-06-03 2:54 ` Stanley Chu 2021-06-03 2:54 ` Stanley Chu 2021-06-03 2:54 ` Stanley Chu 2021-06-16 3:48 ` [PATCH v1 0/3] Optimize host lock on TR send/compl paths and utilize UTRLCNR Martin K. Petersen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=0cfbf580e340073ff972be493a59dbe7@codeaurora.org \ --to=cang@codeaurora.org \ --cc=adrian.hunter@intel.com \ --cc=alim.akhtar@samsung.com \ --cc=asutoshd@codeaurora.org \ --cc=avri.altman@wdc.com \ --cc=beanhuo@micron.com \ --cc=bvanassche@acm.org \ --cc=hongwus@codeaurora.org \ --cc=jaegeuk@kernel.org \ --cc=jejb@linux.ibm.com \ --cc=kernel-team@android.com \ --cc=kwmad.kim@samsung.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mediatek@lists.infradead.org \ --cc=linux-scsi@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=matthias.bgg@gmail.com \ --cc=nguyenb@codeaurora.org \ --cc=satyat@google.com \ --cc=stanley.chu@mediatek.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.