| Message ID | 20210722033439.26550-1-bvanassche@acm.org |
|---|---|
| Headers | show |
| Series | UFS patches for kernel v5.15 | expand |
> From arch/arm/include/asm/io.h > > #define __iowmb() wmb() > [ ... ] > #define writel(v,c) ({ __iowmb(); writel_relaxed(v,c); }) > > From Documentation/memory-barriers.txt: "Note that, when using writel(), > a > prior wmb() is not needed to guarantee that the cache coherent memory > writes have completed before writing to the MMIO region." > > In other words, calling wmb() before writel() is not necessary. Hence > remove the wmb() calls that precede a writel() call. Remove the wmb() > calls that precede a ufshcd_send_command() call since the latter function > uses writel(). Remove the wmb() call from ufshcd_wait_for_dev_cmd() > since the following chain of events guarantees that the CPU will see > up-to-date LRB values: > * UFS controller writes to host memory. > * UFS controller posts completion interrupt after the memory writes from > the previous step are visible to the CPU. > * complete(hba->dev_cmd.complete) is called from the UFS interrupt > handler. > * The wait_for_completion(hba->dev_cmd.complete) call in > ufshcd_wait_for_dev_cmd() returns. > > Cc: Adrian Hunter <adrian.hunter@intel.com> > Cc: Stanley Chu <stanley.chu@mediatek.com> > Cc: Can Guo <cang@codeaurora.org> > Cc: Asutosh Das <asutoshd@codeaurora.org> > Cc: Avri Altman <avri.altman@wdc.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> Reviewed-by: Avri Altman <avri.altman@wdc.com> Tested-by: Avri altman <avri.altman@wdc.com> > --- > drivers/scsi/ufs/ufshcd.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index f467630be7df..d1c3a984d803 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -2769,8 +2769,6 @@ static int ufshcd_queuecommand(struct Scsi_Host > *host, struct scsi_cmnd *cmd) > ufshcd_release(hba); > goto out; > } > - /* Make sure descriptors are ready before ringing the doorbell */ > - wmb(); > > ufshcd_send_command(hba, tag); > out: > @@ -2880,8 +2878,6 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba > *hba, > time_left = wait_for_completion_timeout(hba->dev_cmd.complete, > msecs_to_jiffies(max_timeout)); > > - /* Make sure descriptors are ready before ringing the doorbell */ > - wmb(); > spin_lock_irqsave(hba->host->host_lock, flags); > hba->dev_cmd.complete = NULL; > if (likely(time_left)) { > @@ -2960,8 +2956,6 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba > *hba, > hba->dev_cmd.complete = &wait; > > ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp- > >ucd_req_ptr); > - /* Make sure descriptors are ready before ringing the doorbell */ > - wmb(); > > ufshcd_send_command(hba, tag); > err = ufshcd_wait_for_dev_cmd(hba, lrbp, timeout); > @@ -6521,9 +6515,6 @@ static int __ufshcd_issue_tm_cmd(struct ufs_hba > *hba, > /* send command to the controller */ > __set_bit(task_tag, &hba->outstanding_tasks); > > - /* Make sure descriptors are ready before ringing the task doorbell */ > - wmb(); > - > ufshcd_writel(hba, 1 << task_tag, REG_UTP_TASK_REQ_DOOR_BELL); > /* Make sure that doorbell is committed immediately */ > wmb(); > @@ -6695,8 +6686,6 @@ static int ufshcd_issue_devman_upiu_cmd(struct > ufs_hba *hba, > hba->dev_cmd.complete = &wait; > > ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp- > >ucd_req_ptr); > - /* Make sure descriptors are ready before ringing the doorbell */ > - wmb(); > > ufshcd_send_command(hba, tag); > /*
Hi Bart, >From Documentation/scheduler/completion.rst: "When a completion is declared >as a local variable within a function, then the initialization should >always use DECLARE_COMPLETION_ONSTACK() explicitly, not just to make >lockdep happy, but also to make it clear that limited scope had been >considered and is intentional." Reviewed-by: Daejun Park <daejun7.park@samsung.com> Thanks, Daejun
Hi Bart, >From arch/arm/include/asm/io.h > > #define __iowmb() wmb() > [ ... ] > #define writel(v,c) ({ __iowmb(); writel_relaxed(v,c); }) > >From Documentation/memory-barriers.txt: "Note that, when using writel(), a >prior wmb() is not needed to guarantee that the cache coherent memory >writes have completed before writing to the MMIO region." > >In other words, calling wmb() before writel() is not necessary. Hence >remove the wmb() calls that precede a writel() call. Remove the wmb() >calls that precede a ufshcd_send_command() call since the latter function >uses writel(). Remove the wmb() call from ufshcd_wait_for_dev_cmd() >since the following chain of events guarantees that the CPU will see >up-to-date LRB values: >* UFS controller writes to host memory. >* UFS controller posts completion interrupt after the memory writes from > the previous step are visible to the CPU. >* complete(hba->dev_cmd.complete) is called from the UFS interrupt handler. >* The wait_for_completion(hba->dev_cmd.complete) call in > ufshcd_wait_for_dev_cmd() returns. Reviewed-by: Daejun Park <daejun7.park@samsung.com> Thanks, Daejun
Bart,
> Please consider the patches in this series for kernel v5.15.
Applied to 5.15/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
On Wed, 21 Jul 2021 20:34:21 -0700, Bart Van Assche wrote: > Please consider the patches in this series for kernel v5.15. > > Thank you, > > Bart. > > Changes compared to v2: > - Included a stack corruption fix. > - Dropped patch "Remove a local variable" and added patches "Revert "Utilize > Transfer Request List Completion Notification Register"" and "Optimize > serialization of setup_xfer_req() calls". > - Added patch "Optimize serialization of setup_xfer_req() calls". > > [...] Applied to 5.15/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering
Hi Martin, Please consider the patches in this series for kernel v5.15. Thank you, Bart. Changes compared to v2: - Included a stack corruption fix. - Dropped patch "Remove a local variable" and added patches "Revert "Utilize Transfer Request List Completion Notification Register"" and "Optimize serialization of setup_xfer_req() calls". - Added patch "Optimize serialization of setup_xfer_req() calls". Changes compared to v1: - Left out the SCSI core patches for the SCSI error handler in order not to delay the UFS patches by the conversation around the SCSI error handler patches. - Restored the WARN_ON_ONCE(tag < 0) statements in the patch that removes ufshcd_valid_tag(). - Split "Fix a race in the completion path" in two patches. - Added a fault injection patch. Bart Van Assche (18): scsi: ufs: Fix memory corruption by ufshcd_read_desc_param() scsi: ufs: Reduce power management code duplication scsi: ufs: Only include power management code if necessary scsi: ufs: Rename the second ufshcd_probe_hba() argument scsi: ufs: Use DECLARE_COMPLETION_ONSTACK() where appropriate scsi: ufs: Remove ufshcd_valid_tag() scsi: ufs: Verify UIC locking requirements at runtime scsi: ufs: Improve static type checking for the host controller state scsi: ufs: Remove several wmb() calls scsi: ufs: Inline ufshcd_outstanding_req_clear() scsi: ufs: Revert "Utilize Transfer Request List Completion Notification Register" scsi: ufs: Optimize serialization of setup_xfer_req() calls scsi: ufs: Optimize SCSI command processing scsi: ufs: Fix the SCSI abort handler scsi: ufs: Request sense data asynchronously scsi: ufs: Synchronize SCSI and UFS error handling scsi: ufs: Retry aborted SCSI commands instead of completing these successfully scsi: ufs: Add fault injection support drivers/scsi/ufs/Kconfig | 7 + drivers/scsi/ufs/Makefile | 1 + drivers/scsi/ufs/cdns-pltfrm.c | 7 +- drivers/scsi/ufs/tc-dwc-g210-pci.c | 32 +- drivers/scsi/ufs/tc-dwc-g210-pltfrm.c | 7 +- drivers/scsi/ufs/ufs-exynos.c | 7 +- drivers/scsi/ufs/ufs-fault-injection.c | 70 ++++ drivers/scsi/ufs/ufs-fault-injection.h | 24 ++ drivers/scsi/ufs/ufs-hisi.c | 7 +- drivers/scsi/ufs/ufs-mediatek.c | 7 +- drivers/scsi/ufs/ufs-qcom.c | 7 +- drivers/scsi/ufs/ufshcd-pci.c | 48 +-- drivers/scsi/ufs/ufshcd-pltfrm.c | 47 --- drivers/scsi/ufs/ufshcd-pltfrm.h | 18 - drivers/scsi/ufs/ufshcd.c | 491 +++++++++++-------------- drivers/scsi/ufs/ufshcd.h | 63 ++-- drivers/scsi/ufs/ufshci.h | 1 - 17 files changed, 373 insertions(+), 471 deletions(-) create mode 100644 drivers/scsi/ufs/ufs-fault-injection.c create mode 100644 drivers/scsi/ufs/ufs-fault-injection.h