[PATCH 0/7] UFS driver fixes and cleanups

[PATCH 0/7] UFS driver fixes and cleanups

[Bart Van Assche]

Hi Martin,

This patch series includes several fixes and cleanup patches for the UFS driver.
Please consider this patch series for the next merge window.

Thanks,

Bart.

Bart Van Assche (7):
  scsi: ufs: core: Move the ufshcd_mcq_enable_esi() definition
  scsi: ufs: core: Remove goto statements from
    ufshcd_try_to_abort_task()
  scsi: ufs: core: Simplify ufshcd_try_to_abort_task()
  scsi: ufs: core: Fix ufshcd_exception_event_handler()
  scsi: ufs: core: Simplify ufshcd_err_handling_prepare()
  scsi: ufs: core: Fix ufshcd_mcq_sq_cleanup()
  scsi: ufs: core: Make DMA mask configuration more flexible

 drivers/ufs/core/ufs-mcq.c     | 28 ++++++-------
 drivers/ufs/core/ufshcd.c      | 76 ++++++++++------------------------
 drivers/ufs/host/ufs-renesas.c |  9 +++-
 include/ufs/ufshcd.h           | 13 ++----
 4 files changed, 45 insertions(+), 81 deletions(-)

[PATCH 1/7] scsi: ufs: core: Move the ufshcd_mcq_enable_esi() definition

[Bart Van Assche]

Move the ufshcd_mcq_enable_esi() definition such that it occurs
immediately before the ufshcd_mcq_config_esi() definition.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufs-mcq.c | 14 +++++++-------
 include/ufs/ufshcd.h       |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 5891cdacd0b3..57ced1729b73 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -417,13 +417,6 @@ void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba)
 }
 EXPORT_SYMBOL_GPL(ufshcd_mcq_make_queues_operational);
 
-void ufshcd_mcq_enable_esi(struct ufs_hba *hba)
-{
-	ufshcd_writel(hba, ufshcd_readl(hba, REG_UFS_MEM_CFG) | 0x2,
-		      REG_UFS_MEM_CFG);
-}
-EXPORT_SYMBOL_GPL(ufshcd_mcq_enable_esi);
-
 void ufshcd_mcq_enable(struct ufs_hba *hba)
 {
 	ufshcd_rmwl(hba, MCQ_MODE_SELECT, MCQ_MODE_SELECT, REG_UFS_MEM_CFG);
@@ -437,6 +430,13 @@ void ufshcd_mcq_disable(struct ufs_hba *hba)
 	hba->mcq_enabled = false;
 }
 
+void ufshcd_mcq_enable_esi(struct ufs_hba *hba)
+{
+	ufshcd_writel(hba, ufshcd_readl(hba, REG_UFS_MEM_CFG) | 0x2,
+		      REG_UFS_MEM_CFG);
+}
+EXPORT_SYMBOL_GPL(ufshcd_mcq_enable_esi);
+
 void ufshcd_mcq_config_esi(struct ufs_hba *hba, struct msi_msg *msg)
 {
 	ufshcd_writel(hba, msg->address_lo, REG_UFS_ESILBA);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index a95282b9f743..a0b325a32aca 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1321,8 +1321,8 @@ void ufshcd_mcq_write_cqis(struct ufs_hba *hba, u32 val, int i);
 unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
 					 struct ufs_hw_queue *hwq);
 void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba);
-void ufshcd_mcq_enable_esi(struct ufs_hba *hba);
 void ufshcd_mcq_enable(struct ufs_hba *hba);
+void ufshcd_mcq_enable_esi(struct ufs_hba *hba);
 void ufshcd_mcq_config_esi(struct ufs_hba *hba, struct msi_msg *msg);
 
 int ufshcd_opp_config_clks(struct device *dev, struct opp_table *opp_table,

[PATCH 2/7] scsi: ufs: core: Remove goto statements from ufshcd_try_to_abort_task()

[Bart Van Assche]

The only statement that follows the 'out:' label in
ufshcd_try_to_abort_task() is a return-statement. Simplify this function
by changing 'goto out' statements into return statements.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 9e6d008f4ea4..57ce1910fda0 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7486,7 +7486,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
 int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 {
 	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
-	int err = 0;
+	int err;
 	int poll_cnt;
 	u8 resp = 0xF;
 	u32 reg;
@@ -7516,7 +7516,7 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 				/* command completed already */
 				dev_err(hba->dev, "%s: cmd at tag=%d is cleared.\n",
 					__func__, tag);
-				goto out;
+				return 0;
 			}
 
 			/* Single Doorbell Mode */
@@ -7529,21 +7529,17 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 			/* command completed already */
 			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
 				__func__, tag);
-			goto out;
+			return 0;
 		} else {
 			dev_err(hba->dev,
 				"%s: no response from device. tag = %d, err %d\n",
 				__func__, tag, err);
-			if (!err)
-				err = resp; /* service response error */
-			goto out;
+			return err ? : resp;
 		}
 	}
 
-	if (!poll_cnt) {
-		err = -EBUSY;
-		goto out;
-	}
+	if (!poll_cnt)
+		return -EBUSY;
 
 	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
 			UFS_ABORT_TASK, &resp);
@@ -7553,7 +7549,7 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
 				__func__, tag, err);
 		}
-		goto out;
+		return err;
 	}
 
 	err = ufshcd_clear_cmd(hba, tag);
@@ -7561,7 +7557,6 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
 			__func__, tag, err);
 
-out:
 	return err;
 }
 

[PATCH 3/7] scsi: ufs: core: Simplify ufshcd_try_to_abort_task()

[Bart Van Assche]

The MCQ code is also valid for legacy mode. Hence, remove the legacy
mode code and retain the MCQ code. Since it is not an error if a command
completes while ufshcd_try_to_abort_task() is in progress, use dev_info()
instead of dev_err() to report this.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 57ce1910fda0..76884df580c3 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7489,7 +7489,6 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 	int err;
 	int poll_cnt;
 	u8 resp = 0xF;
-	u32 reg;
 
 	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
 		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
@@ -7504,32 +7503,17 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 			 * cmd not pending in the device, check if it is
 			 * in transition.
 			 */
-			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
+			dev_info(
+				hba->dev,
+				"%s: cmd with tag %d not pending in the device.\n",
 				__func__, tag);
-			if (hba->mcq_enabled) {
-				/* MCQ mode */
-				if (ufshcd_cmd_inflight(lrbp->cmd)) {
-					/* sleep for max. 200us same delay as in SDB mode */
-					usleep_range(100, 200);
-					continue;
-				}
-				/* command completed already */
-				dev_err(hba->dev, "%s: cmd at tag=%d is cleared.\n",
-					__func__, tag);
+			if (!ufshcd_cmd_inflight(lrbp->cmd)) {
+				dev_info(hba->dev,
+					 "%s: cmd with tag=%d completed.\n",
+					 __func__, tag);
 				return 0;
 			}
-
-			/* Single Doorbell Mode */
-			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
-			if (reg & (1 << tag)) {
-				/* sleep for max. 200us to stabilize */
-				usleep_range(100, 200);
-				continue;
-			}
-			/* command completed already */
-			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
-				__func__, tag);
-			return 0;
+			usleep_range(100, 200);
 		} else {
 			dev_err(hba->dev,
 				"%s: no response from device. tag = %d, err %d\n",

[PATCH 4/7] scsi: ufs: core: Fix ufshcd_exception_event_handler()

[Bart Van Assche]

Use blk_mq_quiesce_tagset() / blk_mq_unquiesce_tagset() instead of
scsi_block_requests() / scsi_unblock_requests() because the former wait
for ongoing SCSI command dispatching to finish while the latter do not.

Fixes: 2e3611e9546c ("scsi: ufs: fix exception event handling")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 76884df580c3..ff1b0af74041 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6195,7 +6195,7 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
 	u32 status = 0;
 	hba = container_of(work, struct ufs_hba, eeh_work);
 
-	ufshcd_scsi_block_requests(hba);
+	blk_mq_quiesce_tagset(&hba->host->tag_set);
 	err = ufshcd_get_ee_status(hba, &status);
 	if (err) {
 		dev_err(hba->dev, "%s: failed to get exception status %d\n",
@@ -6213,7 +6213,7 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
 
 	ufs_debugfs_exception_event(hba, status);
 out:
-	ufshcd_scsi_unblock_requests(hba);
+	blk_mq_unquiesce_tagset(&hba->host->tag_set);
 }
 
 /* Complete requests that have door-bell cleared */

[PATCH 5/7] scsi: ufs: core: Simplify ufshcd_err_handling_prepare()

[Bart Van Assche]

Use blk_mq_quiesce_tagset() instead of ufshcd_scsi_block_requests()
and blk_mq_wait_quiesce_done().

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 19 +++----------------
 include/ufs/ufshcd.h      |  2 --
 2 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index ff1b0af74041..75e00e5b3f79 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -349,18 +349,6 @@ static void ufshcd_configure_wb(struct ufs_hba *hba)
 		ufshcd_wb_toggle_buf_flush(hba, true);
 }
 
-static void ufshcd_scsi_unblock_requests(struct ufs_hba *hba)
-{
-	if (atomic_dec_and_test(&hba->scsi_block_reqs_cnt))
-		scsi_unblock_requests(hba->host);
-}
-
-static void ufshcd_scsi_block_requests(struct ufs_hba *hba)
-{
-	if (atomic_inc_return(&hba->scsi_block_reqs_cnt) == 1)
-		scsi_block_requests(hba->host);
-}
-
 static void ufshcd_add_cmd_upiu_trace(struct ufs_hba *hba, unsigned int tag,
 				      enum ufs_trace_str_t str_t)
 {
@@ -6379,15 +6367,14 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
 			ufshcd_suspend_clkscaling(hba);
 		ufshcd_clk_scaling_allow(hba, false);
 	}
-	ufshcd_scsi_block_requests(hba);
 	/* Wait for ongoing ufshcd_queuecommand() calls to finish. */
-	blk_mq_wait_quiesce_done(&hba->host->tag_set);
+	blk_mq_quiesce_tagset(&hba->host->tag_set);
 	cancel_work_sync(&hba->eeh_work);
 }
 
 static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
 {
-	ufshcd_scsi_unblock_requests(hba);
+	blk_mq_unquiesce_tagset(&hba->host->tag_set);
 	ufshcd_release(hba);
 	if (ufshcd_is_clkscaling_supported(hba))
 		ufshcd_clk_scaling_suspend(hba, false);
@@ -10560,7 +10547,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 	/* Hold auto suspend until async scan completes */
 	pm_runtime_get_sync(dev);
-	atomic_set(&hba->scsi_block_reqs_cnt, 0);
+
 	/*
 	 * We are assuming that device wasn't put in sleep/power-down
 	 * state exclusively during the boot stage before kernel.
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index a0b325a32aca..36bd91ff3593 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -928,7 +928,6 @@ enum ufshcd_mcq_opr {
  * @wb_mutex: used to serialize devfreq and sysfs write booster toggling
  * @clk_scaling_lock: used to serialize device commands and clock scaling
  * @desc_size: descriptor sizes reported by device
- * @scsi_block_reqs_cnt: reference counting for scsi block requests
  * @bsg_dev: struct device associated with the BSG queue
  * @bsg_queue: BSG queue associated with the UFS controller
  * @rpm_dev_flush_recheck_work: used to suspend from RPM (runtime power
@@ -1089,7 +1088,6 @@ struct ufs_hba {
 
 	struct mutex wb_mutex;
 	struct rw_semaphore clk_scaling_lock;
-	atomic_t scsi_block_reqs_cnt;
 
 	struct device		bsg_dev;
 	struct request_queue	*bsg_queue;

[PATCH 6/7] scsi: ufs: core: Fix ufshcd_mcq_sq_cleanup()

[Bart Van Assche]

From the UFSHCI specification: "CleanUp Command Return Code (RTC): host
controller sets this return code to provide more details of the cleanup
process. It is valid only when CUS is 1." Hence, do not read RTC if the
CUS bitfield is zero.

Cc: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Fixes: 8d7290348992 ("scsi: ufs: mcq: Add supporting functions for MCQ abort")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufs-mcq.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 57ced1729b73..bef4c53f9c06 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -538,7 +538,7 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag)
 	struct ufshcd_lrb *lrbp = &hba->lrb[task_tag];
 	struct scsi_cmnd *cmd = lrbp->cmd;
 	struct ufs_hw_queue *hwq;
-	void __iomem *reg, *opr_sqd_base;
+	void __iomem *opr_sqd_base;
 	u32 nexus, id, val;
 	int err;
 
@@ -572,14 +572,12 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag)
 	/* SQRTCy.ICU = 1 */
 	writel(SQ_ICU, opr_sqd_base + REG_SQRTC);
 
-	/* Poll SQRTSy.CUS = 1. Return result from SQRTSy.RTC */
-	reg = opr_sqd_base + REG_SQRTS;
-	err = read_poll_timeout(readl, val, val & SQ_CUS, 20,
-				MCQ_POLL_US, false, reg);
+	/* Wait until SQRTSy.CUS = 1. */
+	err = read_poll_timeout(readl, val, val & SQ_CUS, 20, MCQ_POLL_US,
+				false, opr_sqd_base + REG_SQRTS);
 	if (err)
-		dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%ld\n",
-			__func__, id, task_tag,
-			FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
+		dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%d\n",
+			__func__, id, task_tag, err);
 
 	if (ufshcd_mcq_sq_start(hba, hwq))
 		err = -ETIMEDOUT;

[PATCH 7/7] scsi: ufs: core: Make DMA mask configuration more flexible

[Bart Van Assche]

Replace UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS with
ufs_hba_variant_ops::set_dma_mask. Update the Renesas driver accordingly.
This patch prepares for adding 36-bit DMA support. No functionality has
been changed.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c      | 4 ++--
 drivers/ufs/host/ufs-renesas.c | 9 +++++++--
 include/ufs/ufshcd.h           | 9 +++------
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 75e00e5b3f79..c1d4a9c8480f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2389,8 +2389,6 @@ static inline int ufshcd_hba_capabilities(struct ufs_hba *hba)
 	int err;
 
 	hba->capabilities = ufshcd_readl(hba, REG_CONTROLLER_CAPABILITIES);
-	if (hba->quirks & UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS)
-		hba->capabilities &= ~MASK_64_ADDRESSING_SUPPORT;
 
 	/* nutrs and nutmrs are 0 based values */
 	hba->nutrs = (hba->capabilities & MASK_TRANSFER_REQUESTS_SLOTS_SDB) + 1;
@@ -10280,6 +10278,8 @@ EXPORT_SYMBOL_GPL(ufshcd_dealloc_host);
  */
 static int ufshcd_set_dma_mask(struct ufs_hba *hba)
 {
+	if (hba->vops && hba->vops->set_dma_mask)
+		return hba->vops->set_dma_mask(hba);
 	if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
 		if (!dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(64)))
 			return 0;
diff --git a/drivers/ufs/host/ufs-renesas.c b/drivers/ufs/host/ufs-renesas.c
index 8711e5cbc968..8b0aebf127b6 100644
--- a/drivers/ufs/host/ufs-renesas.c
+++ b/drivers/ufs/host/ufs-renesas.c
@@ -7,6 +7,7 @@
 
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
@@ -364,14 +365,18 @@ static int ufs_renesas_init(struct ufs_hba *hba)
 		return -ENOMEM;
 	ufshcd_set_variant(hba, priv);
 
-	hba->quirks |= UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS | UFSHCD_QUIRK_HIBERN_FASTAUTO;
-
 	return 0;
 }
 
+static int ufs_renesas_set_dma_mask(struct ufs_hba *hba)
+{
+	return dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(32));
+}
+
 static const struct ufs_hba_variant_ops ufs_renesas_vops = {
 	.name		= "renesas",
 	.init		= ufs_renesas_init,
+	.set_dma_mask	= ufs_renesas_set_dma_mask,
 	.setup_clocks	= ufs_renesas_setup_clocks,
 	.hce_enable_notify = ufs_renesas_hce_enable_notify,
 	.dbg_register_dump = ufs_renesas_dbg_register_dump,
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 36bd91ff3593..9ea2a7411bb5 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -299,6 +299,8 @@ struct ufs_pwr_mode_info {
  * @max_num_rtt: maximum RTT supported by the host
  * @init: called when the driver is initialized
  * @exit: called to cleanup everything done in init
+ * @set_dma_mask: For setting another DMA mask than indicated by the 64AS
+ *	capability bit.
  * @get_ufs_hci_version: called to get UFS HCI version
  * @clk_scale_notify: notifies that clks are scaled up/down
  * @setup_clocks: called before touching any of the controller registers
@@ -341,6 +343,7 @@ struct ufs_hba_variant_ops {
 	int	(*init)(struct ufs_hba *);
 	void    (*exit)(struct ufs_hba *);
 	u32	(*get_ufs_hci_version)(struct ufs_hba *);
+	int	(*set_dma_mask)(struct ufs_hba *);
 	int	(*clk_scale_notify)(struct ufs_hba *, bool,
 				    enum ufs_notify_change_status);
 	int	(*setup_clocks)(struct ufs_hba *, bool,
@@ -623,12 +626,6 @@ enum ufshcd_quirks {
 	 */
 	UFSHCD_QUIRK_SKIP_PH_CONFIGURATION		= 1 << 16,
 
-	/*
-	 * This quirk needs to be enabled if the host controller has
-	 * 64-bit addressing supported capability but it doesn't work.
-	 */
-	UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS		= 1 << 17,
-
 	/*
 	 * This quirk needs to be enabled if the host controller has
 	 * auto-hibernate capability but it's FASTAUTO only.

RE: [PATCH 2/7] scsi: ufs: core: Remove goto statements from ufshcd_try_to_abort_task()

[Avri Altman]

> 
> The only statement that follows the 'out:' label in
> ufshcd_try_to_abort_task() is a return-statement. Simplify this function by
> changing 'goto out' statements into return statements.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>

RE: [PATCH 3/7] scsi: ufs: core: Simplify ufshcd_try_to_abort_task()

[Avri Altman]

> -                       /* Single Doorbell Mode */
> -                       reg = ufshcd_readl(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL);
> -                       if (reg & (1 << tag)) {
> -                               /* sleep for max. 200us to stabilize */
> -                               usleep_range(100, 200);
> -                               continue;
> -                       }
If we no longer use the doorbell to determine the inflight requests,
I think this should be your patch title, or at least a clear indication in your commit log.

Thanks,
Avri

RE: [PATCH 6/7] scsi: ufs: core: Fix ufshcd_mcq_sq_cleanup()

[Avri Altman]

 
> -       /* Poll SQRTSy.CUS = 1. Return result from SQRTSy.RTC */
> -       reg = opr_sqd_base + REG_SQRTS;
> -       err = read_poll_timeout(readl, val, val & SQ_CUS, 20,
> -                               MCQ_POLL_US, false, reg);
> +       /* Wait until SQRTSy.CUS = 1. */
> +       err = read_poll_timeout(readl, val, val & SQ_CUS, 20, MCQ_POLL_US,
> +                               false, opr_sqd_base + REG_SQRTS);
>         if (err)
Can remove the if (err)

> -               dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%ld\n",
> -                       __func__, id, task_tag,
> -                       FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
> +               dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%d\n",
> +                       __func__, id, task_tag, err);
And report RTC on success or err otherwise:
+                       __func__, id, task_tag, err ? : FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(opr_sqd_base + REG_SQRTS));

Thanks,
Avri

> 
>         if (ufshcd_mcq_sq_start(hba, hwq))
>                 err = -ETIMEDOUT;

Re: [PATCH 3/7] scsi: ufs: core: Simplify ufshcd_try_to_abort_task()

[Bart Van Assche]

On 10/16/24 11:31 PM, Avri Altman wrote:
>> -                       /* Single Doorbell Mode */
>> -                       reg = ufshcd_readl(hba,
>> REG_UTP_TRANSFER_REQ_DOOR_BELL);
>> -                       if (reg & (1 << tag)) {
>> -                               /* sleep for max. 200us to stabilize */
>> -                               usleep_range(100, 200);
>> -                               continue;
>> -                       }
 >
> If we no longer use the doorbell to determine the inflight requests,
> I think this should be your patch title, or at least a clear indication in your commit log.

Hmm ... I see this as an implementation detail. To me, the key
aspect of this patch is that the legacy and MCQ mode code paths
are combined into a single code path.

Thanks,

Bart.

Re: [PATCH 6/7] scsi: ufs: core: Fix ufshcd_mcq_sq_cleanup()

[Bart Van Assche]

On 10/16/24 11:55 PM, Avri Altman wrote:
>   
>> -       /* Poll SQRTSy.CUS = 1. Return result from SQRTSy.RTC */
>> -       reg = opr_sqd_base + REG_SQRTS;
>> -       err = read_poll_timeout(readl, val, val & SQ_CUS, 20,
>> -                               MCQ_POLL_US, false, reg);
>> +       /* Wait until SQRTSy.CUS = 1. */
>> +       err = read_poll_timeout(readl, val, val & SQ_CUS, 20, MCQ_POLL_US,
>> +                               false, opr_sqd_base + REG_SQRTS);
>>          if (err)
> Can remove the if (err)
> 
>> -               dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%ld\n",
>> -                       __func__, id, task_tag,
>> -                       FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
>> +               dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%d\n",
>> +                       __func__, id, task_tag, err);
> And report RTC on success or err otherwise:
> +                       __func__, id, task_tag, err ? : FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(opr_sqd_base + REG_SQRTS));

Hi Avri,

 From the UFSCHI standard about RTC:

0 : Success
1 : Fail – Task Not found
2 : Fail – SQ not stopped
3 : Fail – SQ is disabled
Others : Reserved

Do you agree with changing ufshcd_mcq_sq_cleanup() such that it fails if
RTC is not zero?

Thanks,

Bart.

RE: [PATCH 6/7] scsi: ufs: core: Fix ufshcd_mcq_sq_cleanup()

[Avri Altman]

> On 10/16/24 11:55 PM, Avri Altman wrote:
> >
> >> -       /* Poll SQRTSy.CUS = 1. Return result from SQRTSy.RTC */
> >> -       reg = opr_sqd_base + REG_SQRTS;
> >> -       err = read_poll_timeout(readl, val, val & SQ_CUS, 20,
> >> -                               MCQ_POLL_US, false, reg);
> >> +       /* Wait until SQRTSy.CUS = 1. */
> >> +       err = read_poll_timeout(readl, val, val & SQ_CUS, 20, MCQ_POLL_US,
> >> +                               false, opr_sqd_base + REG_SQRTS);
> >>          if (err)
> > Can remove the if (err)
> >
> >> -               dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%ld\n",
> >> -                       __func__, id, task_tag,
> >> -                       FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
> >> +               dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%d\n",
> >> +                       __func__, id, task_tag, err);
> > And report RTC on success or err otherwise:
> > +                       __func__, id, task_tag, err ? :
> > + FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(opr_sqd_base +
> REG_SQRTS));
> 
> Hi Avri,
> 
>  From the UFSCHI standard about RTC:
> 
> 0 : Success
> 1 : Fail – Task Not found
> 2 : Fail – SQ not stopped
> 3 : Fail – SQ is disabled
> Others : Reserved
> 
> Do you agree with changing ufshcd_mcq_sq_cleanup() such that it fails if RTC
> is not zero?
I was just pointing out that after your change, the extra info of RTC will no longer be available,
And proposed a way in which we can still retain it.

Thanks,
Avri
> 
> Thanks,
> 
> Bart.

Re: [PATCH 6/7] scsi: ufs: core: Fix ufshcd_mcq_sq_cleanup()

[Bart Van Assche]

On 10/17/24 11:07 AM, Avri Altman wrote:
> I was just pointing out that after your change, the extra info of RTC will no longer be available,
> And proposed a way in which we can still retain it.

Something like this change?

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 57ced1729b73..988400500560 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -572,14 +572,18 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int 
task_tag)
  	/* SQRTCy.ICU = 1 */
  	writel(SQ_ICU, opr_sqd_base + REG_SQRTC);

-	/* Poll SQRTSy.CUS = 1. Return result from SQRTSy.RTC */
+	/* Wait until SQRTSy.CUS = 1. Report SQRTSy.RTC. */
  	reg = opr_sqd_base + REG_SQRTS;
  	err = read_poll_timeout(readl, val, val & SQ_CUS, 20,
  				MCQ_POLL_US, false, reg);
  	if (err)
-		dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%ld\n",
-			__func__, id, task_tag,
-			FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
+		dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%d\n",
+			__func__, id, task_tag, err);
+	else
+		dev_info(hba->dev,
+			 "%s, hwq %d: cleanup return code (RTC) %ld\n",
+			 __func__, id,
+			 FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));

  	if (ufshcd_mcq_sq_start(hba, hwq))
  		err = -ETIMEDOUT;

Thanks,

Bart.

RE: [PATCH 6/7] scsi: ufs: core: Fix ufshcd_mcq_sq_cleanup()

[Avri Altman]

> 
> On 10/17/24 11:07 AM, Avri Altman wrote:
> > I was just pointing out that after your change, the extra info of RTC
> > will no longer be available, And proposed a way in which we can still retain
> it.
> 
> Something like this change?
> 
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c index
> 57ced1729b73..988400500560 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -572,14 +572,18 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba,
> int
> task_tag)
>         /* SQRTCy.ICU = 1 */
>         writel(SQ_ICU, opr_sqd_base + REG_SQRTC);
> 
> -       /* Poll SQRTSy.CUS = 1. Return result from SQRTSy.RTC */
> +       /* Wait until SQRTSy.CUS = 1. Report SQRTSy.RTC. */
>         reg = opr_sqd_base + REG_SQRTS;
>         err = read_poll_timeout(readl, val, val & SQ_CUS, 20,
>                                 MCQ_POLL_US, false, reg);
>         if (err)
> -               dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%ld\n",
> -                       __func__, id, task_tag,
> -                       FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
> +               dev_err(hba->dev, "%s: failed. hwq=%d, tag=%d err=%d\n",
> +                       __func__, id, task_tag, err);
> +       else
> +               dev_info(hba->dev,
> +                        "%s, hwq %d: cleanup return code (RTC) %ld\n",
> +                        __func__, id,
> +                        FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
Yes.

Thanks,
Avri

> 
>         if (ufshcd_mcq_sq_start(hba, hwq))
>                 err = -ETIMEDOUT;
> 
> Thanks,
> 
> Bart.

RE: [PATCH 4/7] scsi: ufs: core: Fix ufshcd_exception_event_handler()

[Avri Altman]

> Use blk_mq_quiesce_tagset() / blk_mq_unquiesce_tagset() instead of
> scsi_block_requests() / scsi_unblock_requests() because the former wait for
> ongoing SCSI command dispatching to finish while the latter do not.
> 
> Fixes: 2e3611e9546c ("scsi: ufs: fix exception event handling")
I think that when Maya introduced the scsi_block_requests calls (2018),
the block tagset quiesce api wasn't available yet (2022).

Thanks,
Avri

> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/ufs/core/ufshcd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> 76884df580c3..ff1b0af74041 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -6195,7 +6195,7 @@ static void ufshcd_exception_event_handler(struct
> work_struct *work)
>         u32 status = 0;
>         hba = container_of(work, struct ufs_hba, eeh_work);
> 
> -       ufshcd_scsi_block_requests(hba);
> +       blk_mq_quiesce_tagset(&hba->host->tag_set);
>         err = ufshcd_get_ee_status(hba, &status);
>         if (err) {
>                 dev_err(hba->dev, "%s: failed to get exception status %d\n", @@ -
> 6213,7 +6213,7 @@ static void ufshcd_exception_event_handler(struct
> work_struct *work)
> 
>         ufs_debugfs_exception_event(hba, status);
>  out:
> -       ufshcd_scsi_unblock_requests(hba);
> +       blk_mq_unquiesce_tagset(&hba->host->tag_set);
>  }
> 
>  /* Complete requests that have door-bell cleared */

RE: [PATCH 5/7] scsi: ufs: core: Simplify ufshcd_err_handling_prepare()

[Avri Altman]

> Use blk_mq_quiesce_tagset() instead of ufshcd_scsi_block_requests() and
> blk_mq_wait_quiesce_done().
Maybe also add a sentence indicating that this was the last caller of scsi_block_requests hence it can be removed.

> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>

> ---
>  drivers/ufs/core/ufshcd.c | 19 +++----------------
>  include/ufs/ufshcd.h      |  2 --
>  2 files changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> ff1b0af74041..75e00e5b3f79 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -349,18 +349,6 @@ static void ufshcd_configure_wb(struct ufs_hba
> *hba)
>                 ufshcd_wb_toggle_buf_flush(hba, true);  }
> 
> -static void ufshcd_scsi_unblock_requests(struct ufs_hba *hba) -{
> -       if (atomic_dec_and_test(&hba->scsi_block_reqs_cnt))
> -               scsi_unblock_requests(hba->host);
> -}
> -
> -static void ufshcd_scsi_block_requests(struct ufs_hba *hba) -{
> -       if (atomic_inc_return(&hba->scsi_block_reqs_cnt) == 1)
> -               scsi_block_requests(hba->host);
> -}
> -
>  static void ufshcd_add_cmd_upiu_trace(struct ufs_hba *hba, unsigned int
> tag,
>                                       enum ufs_trace_str_t str_t)  { @@ -6379,15 +6367,14
> @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
>                         ufshcd_suspend_clkscaling(hba);
>                 ufshcd_clk_scaling_allow(hba, false);
>         }
> -       ufshcd_scsi_block_requests(hba);
>         /* Wait for ongoing ufshcd_queuecommand() calls to finish. */
> -       blk_mq_wait_quiesce_done(&hba->host->tag_set);
> +       blk_mq_quiesce_tagset(&hba->host->tag_set);
>         cancel_work_sync(&hba->eeh_work);  }
> 
>  static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)  {
> -       ufshcd_scsi_unblock_requests(hba);
> +       blk_mq_unquiesce_tagset(&hba->host->tag_set);
>         ufshcd_release(hba);
>         if (ufshcd_is_clkscaling_supported(hba))
>                 ufshcd_clk_scaling_suspend(hba, false); @@ -10560,7 +10547,7
> @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base,
> unsigned int irq)
> 
>         /* Hold auto suspend until async scan completes */
>         pm_runtime_get_sync(dev);
> -       atomic_set(&hba->scsi_block_reqs_cnt, 0);
> +
>         /*
>          * We are assuming that device wasn't put in sleep/power-down
>          * state exclusively during the boot stage before kernel.
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index
> a0b325a32aca..36bd91ff3593 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -928,7 +928,6 @@ enum ufshcd_mcq_opr {
>   * @wb_mutex: used to serialize devfreq and sysfs write booster toggling
>   * @clk_scaling_lock: used to serialize device commands and clock scaling
>   * @desc_size: descriptor sizes reported by device
> - * @scsi_block_reqs_cnt: reference counting for scsi block requests
>   * @bsg_dev: struct device associated with the BSG queue
>   * @bsg_queue: BSG queue associated with the UFS controller
>   * @rpm_dev_flush_recheck_work: used to suspend from RPM (runtime
> power @@ -1089,7 +1088,6 @@ struct ufs_hba {
> 
>         struct mutex wb_mutex;
>         struct rw_semaphore clk_scaling_lock;
> -       atomic_t scsi_block_reqs_cnt;
> 
>         struct device           bsg_dev;
>         struct request_queue    *bsg_queue;

RE: [PATCH 7/7] scsi: ufs: core: Make DMA mask configuration more flexible

[Avri Altman]

> Replace UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS with
> ufs_hba_variant_ops::set_dma_mask. Update the Renesas driver accordingly.
> This patch prepares for adding 36-bit DMA support. No functionality has
> been changed.
It looks like this patch belongs to a different series,
e.g. the one that adds the 36-bit DMA support.

> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/ufs/core/ufshcd.c      | 4 ++--
>  drivers/ufs/host/ufs-renesas.c | 9 +++++++--
>  include/ufs/ufshcd.h           | 9 +++------
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> 75e00e5b3f79..c1d4a9c8480f 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2389,8 +2389,6 @@ static inline int ufshcd_hba_capabilities(struct
> ufs_hba *hba)
>         int err;
> 
>         hba->capabilities = ufshcd_readl(hba, REG_CONTROLLER_CAPABILITIES);
> -       if (hba->quirks & UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS)
> -               hba->capabilities &= ~MASK_64_ADDRESSING_SUPPORT;
> 
>         /* nutrs and nutmrs are 0 based values */
>         hba->nutrs = (hba->capabilities &
> MASK_TRANSFER_REQUESTS_SLOTS_SDB) + 1; @@ -10280,6 +10278,8 @@
> EXPORT_SYMBOL_GPL(ufshcd_dealloc_host);
>   */
>  static int ufshcd_set_dma_mask(struct ufs_hba *hba)  {
> +       if (hba->vops && hba->vops->set_dma_mask)
> +               return hba->vops->set_dma_mask(hba);
>         if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
>                 if (!dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(64)))
>                         return 0;
> diff --git a/drivers/ufs/host/ufs-renesas.c b/drivers/ufs/host/ufs-renesas.c
> index 8711e5cbc968..8b0aebf127b6 100644
> --- a/drivers/ufs/host/ufs-renesas.c
> +++ b/drivers/ufs/host/ufs-renesas.c
> @@ -7,6 +7,7 @@
> 
>  #include <linux/clk.h>
>  #include <linux/delay.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/err.h>
>  #include <linux/iopoll.h>
>  #include <linux/kernel.h>
> @@ -364,14 +365,18 @@ static int ufs_renesas_init(struct ufs_hba *hba)
>                 return -ENOMEM;
>         ufshcd_set_variant(hba, priv);
> 
> -       hba->quirks |= UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS |
> UFSHCD_QUIRK_HIBERN_FASTAUTO;
> -
Was it intentional to remove the UFSHCD_QUIRK_HIBERN_FASTAUTO as well?

Thanks,
Avri

>         return 0;
>  }
> 
> +static int ufs_renesas_set_dma_mask(struct ufs_hba *hba) {
> +       return dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(32)); }
> +
>  static const struct ufs_hba_variant_ops ufs_renesas_vops = {
>         .name           = "renesas",
>         .init           = ufs_renesas_init,
> +       .set_dma_mask   = ufs_renesas_set_dma_mask,
>         .setup_clocks   = ufs_renesas_setup_clocks,
>         .hce_enable_notify = ufs_renesas_hce_enable_notify,
>         .dbg_register_dump = ufs_renesas_dbg_register_dump, diff --git
> a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index
> 36bd91ff3593..9ea2a7411bb5 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -299,6 +299,8 @@ struct ufs_pwr_mode_info {
>   * @max_num_rtt: maximum RTT supported by the host
>   * @init: called when the driver is initialized
>   * @exit: called to cleanup everything done in init
> + * @set_dma_mask: For setting another DMA mask than indicated by the
> 64AS
> + *     capability bit.
>   * @get_ufs_hci_version: called to get UFS HCI version
>   * @clk_scale_notify: notifies that clks are scaled up/down
>   * @setup_clocks: called before touching any of the controller registers @@
> -341,6 +343,7 @@ struct ufs_hba_variant_ops {
>         int     (*init)(struct ufs_hba *);
>         void    (*exit)(struct ufs_hba *);
>         u32     (*get_ufs_hci_version)(struct ufs_hba *);
> +       int     (*set_dma_mask)(struct ufs_hba *);
>         int     (*clk_scale_notify)(struct ufs_hba *, bool,
>                                     enum ufs_notify_change_status);
>         int     (*setup_clocks)(struct ufs_hba *, bool,
> @@ -623,12 +626,6 @@ enum ufshcd_quirks {
>          */
>         UFSHCD_QUIRK_SKIP_PH_CONFIGURATION              = 1 << 16,
> 
> -       /*
> -        * This quirk needs to be enabled if the host controller has
> -        * 64-bit addressing supported capability but it doesn't work.
> -        */
> -       UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS               = 1 << 17,
> -
>         /*
>          * This quirk needs to be enabled if the host controller has
>          * auto-hibernate capability but it's FASTAUTO only.

Re: [PATCH 4/7] scsi: ufs: core: Fix ufshcd_exception_event_handler()

[Bart Van Assche]

On 10/17/24 10:50 PM, Avri Altman wrote:
>> Use blk_mq_quiesce_tagset() / blk_mq_unquiesce_tagset() instead of
>> scsi_block_requests() / scsi_unblock_requests() because the former wait for
>> ongoing SCSI command dispatching to finish while the latter do not.
>>
>> Fixes: 2e3611e9546c ("scsi: ufs: fix exception event handling")
 >
> I think that when Maya introduced the scsi_block_requests calls (2018),
> the block tagset quiesce api wasn't available yet (2022).

Hi Avri,

Do you perhaps want me to integrate that information in the patch 
description?

Thanks,

Bart.

Re: [PATCH 7/7] scsi: ufs: core: Make DMA mask configuration more flexible

[Bart Van Assche]

On 10/17/24 11:03 PM, Avri Altman wrote:
>> Replace UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS with
>> ufs_hba_variant_ops::set_dma_mask. Update the Renesas driver accordingly.
>> This patch prepares for adding 36-bit DMA support. No functionality has
>> been changed.
 >
> It looks like this patch belongs to a different series,
> e.g. the one that adds the 36-bit DMA support.

OK, I will separate this patch from this patch series.

>> -       hba->quirks |= UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS |
>> UFSHCD_QUIRK_HIBERN_FASTAUTO;
>> -
> Was it intentional to remove the UFSHCD_QUIRK_HIBERN_FASTAUTO as well?

I will fix this.

Thanks,

Bart.

RE: [PATCH 4/7] scsi: ufs: core: Fix ufshcd_exception_event_handler()

[Avri Altman]

> On 10/17/24 10:50 PM, Avri Altman wrote:
> >> Use blk_mq_quiesce_tagset() / blk_mq_unquiesce_tagset() instead of
> >> scsi_block_requests() / scsi_unblock_requests() because the former
> >> wait for ongoing SCSI command dispatching to finish while the latter do
> not.
> >>
> >> Fixes: 2e3611e9546c ("scsi: ufs: fix exception event handling")
>  >
> > I think that when Maya introduced the scsi_block_requests calls
> > (2018), the block tagset quiesce api wasn't available yet (2022).
> 
> Hi Avri,
> 
> Do you perhaps want me to integrate that information in the patch
> description?
No. But the Fixes tag seems strange, isn't it?

Thanks,
Avri

> 
> Thanks,
> 
> Bart.

Re: [PATCH 4/7] scsi: ufs: core: Fix ufshcd_exception_event_handler()

[Bart Van Assche]

On 10/18/24 10:25 AM, Avri Altman wrote:
>> On 10/17/24 10:50 PM, Avri Altman wrote:
>>>> Use blk_mq_quiesce_tagset() / blk_mq_unquiesce_tagset() instead of
>>>> scsi_block_requests() / scsi_unblock_requests() because the former
>>>> wait for ongoing SCSI command dispatching to finish while the latter do
>> not.
>>>>
>>>> Fixes: 2e3611e9546c ("scsi: ufs: fix exception event handling")
>>   >
>>> I think that when Maya introduced the scsi_block_requests calls
>>> (2018), the block tagset quiesce api wasn't available yet (2022).
>>
>> Hi Avri,
>>
>> Do you perhaps want me to integrate that information in the patch
>> description?
 >
> No. But the Fixes tag seems strange, isn't it?

Right, that patch did not introduce the issue that 
ufshcd_exception_event_handler() doesn't wait for ongoing SCSI command
dispatching calls. Let me look up which patch introduced that issue.

Thanks,

Bart.

Re: [PATCH 4/7] scsi: ufs: core: Fix ufshcd_exception_event_handler()

[Bart Van Assche]

On 10/18/24 10:25 AM, Avri Altman wrote:
> No. But the Fixes tag seems strange, isn't it?

How about replacing the entire patch with the patch below?

Thanks,

Bart.


scsi: ufs: core: Simplify ufshcd_exception_event_handler()

The ufshcd_scsi_block_requests() and ufshcd_scsi_unblock_requests()
calls were introduced in ufshcd_exception_event_handler() to prevent
that querying the exception event information would time out. Commit
10fe5888a40e ("scsi: ufs: increase the scsi query response timeout")
increased the timeout for querying exception information from 30 ms to
1.5 s and thereby eliminated the risk that a timeout would happen.
Hence, the calls to block and unblock SCSI requests are superfluous.
Remove these calls.

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 76884df580c3..2fde1b0a6086 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6195,12 +6195,11 @@ static void 
ufshcd_exception_event_handler(struct work_struct *work)
  	u32 status = 0;
  	hba = container_of(work, struct ufs_hba, eeh_work);

-	ufshcd_scsi_block_requests(hba);
  	err = ufshcd_get_ee_status(hba, &status);
  	if (err) {
  		dev_err(hba->dev, "%s: failed to get exception status %d\n",
  				__func__, err);
-		goto out;
+		return;
  	}

  	trace_ufshcd_exception_event(dev_name(hba->dev), status);
@@ -6212,8 +6211,6 @@ static void ufshcd_exception_event_handler(struct 
work_struct *work)
  		ufshcd_temp_exception_event_handler(hba, status);

  	ufs_debugfs_exception_event(hba, status);
-out:
-	ufshcd_scsi_unblock_requests(hba);
  }

  /* Complete requests that have door-bell cleared */

Re: [PATCH 5/7] scsi: ufs: core: Simplify ufshcd_err_handling_prepare()

[Bart Van Assche]

On 10/17/24 10:53 PM, Avri Altman wrote:
> Maybe also add a sentence indicating that this was the last caller of
> scsi_block_requests hence it can be removed.

I will do that.

Bart.

RE: [PATCH 4/7] scsi: ufs: core: Fix ufshcd_exception_event_handler()

[Avri Altman]

> 
> On 10/18/24 10:25 AM, Avri Altman wrote:
> > No. But the Fixes tag seems strange, isn't it?
> 
> How about replacing the entire patch with the patch below?
Right.
Looks like it was introduced in the first place when the query timeout was 100ms:

"..When trying to check the exception event status (for
    finding the cause for the exception event), the device may be busy with
    additional SCSI commands handling and may not respond within the 100ms
    timeout..."

Looks good to me.  I can also test it on Sunday.

Thanks,
Avri
> 
> Thanks,
> 
> Bart.
> 
> 
> scsi: ufs: core: Simplify ufshcd_exception_event_handler()
> 
> The ufshcd_scsi_block_requests() and ufshcd_scsi_unblock_requests() calls
> were introduced in ufshcd_exception_event_handler() to prevent that
> querying the exception event information would time out. Commit
> 10fe5888a40e ("scsi: ufs: increase the scsi query response timeout")
> increased the timeout for querying exception information from 30 ms to
> 1.5 s and thereby eliminated the risk that a timeout would happen.
> Hence, the calls to block and unblock SCSI requests are superfluous.
> Remove these calls.
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> 76884df580c3..2fde1b0a6086 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -6195,12 +6195,11 @@ static void
> ufshcd_exception_event_handler(struct work_struct *work)
>         u32 status = 0;
>         hba = container_of(work, struct ufs_hba, eeh_work);
> 
> -       ufshcd_scsi_block_requests(hba);
>         err = ufshcd_get_ee_status(hba, &status);
>         if (err) {
>                 dev_err(hba->dev, "%s: failed to get exception status %d\n",
>                                 __func__, err);
> -               goto out;
> +               return;
>         }
> 
>         trace_ufshcd_exception_event(dev_name(hba->dev), status); @@ -
> 6212,8 +6211,6 @@ static void ufshcd_exception_event_handler(struct
> work_struct *work)
>                 ufshcd_temp_exception_event_handler(hba, status);
> 
>         ufs_debugfs_exception_event(hba, status);
> -out:
> -       ufshcd_scsi_unblock_requests(hba);
>   }
> 
>   /* Complete requests that have door-bell cleared */

RE: [PATCH 4/7] scsi: ufs: core: Fix ufshcd_exception_event_handler()

[Avri Altman]

> > scsi: ufs: core: Simplify ufshcd_exception_event_handler()
> >
> > The ufshcd_scsi_block_requests() and ufshcd_scsi_unblock_requests()
> > calls were introduced in ufshcd_exception_event_handler() to prevent
> > that querying the exception event information would time out. Commit
> > 10fe5888a40e ("scsi: ufs: increase the scsi query response timeout")
> > increased the timeout for querying exception information from 30 ms to
> > 1.5 s and thereby eliminated the risk that a timeout would happen.
> > Hence, the calls to block and unblock SCSI requests are superfluous.
> > Remove these calls.
> >
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index
> > 76884df580c3..2fde1b0a6086 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -6195,12 +6195,11 @@ static void
> > ufshcd_exception_event_handler(struct work_struct *work)
> >         u32 status = 0;
> >         hba = container_of(work, struct ufs_hba, eeh_work);
> >
> > -       ufshcd_scsi_block_requests(hba);
> >         err = ufshcd_get_ee_status(hba, &status);
> >         if (err) {
> >                 dev_err(hba->dev, "%s: failed to get exception status %d\n",
> >                                 __func__, err);
> > -               goto out;
> > +               return;
> >         }
> >
> >         trace_ufshcd_exception_event(dev_name(hba->dev), status); @@ -
> > 6212,8 +6211,6 @@ static void ufshcd_exception_event_handler(struct
> > work_struct *work)
> >                 ufshcd_temp_exception_event_handler(hba, status);
> >
> >         ufs_debugfs_exception_event(hba, status);
> > -out:
> > -       ufshcd_scsi_unblock_requests(hba);
> >   }
> >
> >   /* Complete requests that have door-bell cleared */
Tested-by: Avri Altman <avri.altman@wdc.com>

Re: [PATCH 1/7] scsi: ufs: core: Move the ufshcd_mcq_enable_esi() definition

[Peter Wang (王信友)]

On Wed, 2024-10-16 at 14:11 -0700, Bart Van Assche wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Move the ufshcd_mcq_enable_esi() definition such that it occurs
> immediately before the ufshcd_mcq_config_esi() definition.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/ufs/core/ufs-mcq.c | 14 +++++++-------
>  include/ufs/ufshcd.h       |  2 +-
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 2/7] scsi: ufs: core: Remove goto statements from ufshcd_try_to_abort_task()

[Peter Wang (王信友)]

On Wed, 2024-10-16 at 14:11 -0700, Bart Van Assche wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  The only statement that follows the 'out:' label in
> ufshcd_try_to_abort_task() is a return-statement. Simplify this
> function
> by changing 'goto out' statements into return statements.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/ufs/core/ufshcd.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 3/7] scsi: ufs: core: Simplify ufshcd_try_to_abort_task()

[Peter Wang (王信友)]

On Wed, 2024-10-16 at 14:11 -0700, Bart Van Assche wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  The MCQ code is also valid for legacy mode. Hence, remove the legacy
> mode code and retain the MCQ code. Since it is not an error if a
> command
> completes while ufshcd_try_to_abort_task() is in progress, use
> dev_info()
> instead of dev_err() to report this.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/ufs/core/ufshcd.c | 32 ++++++++------------------------
>  1 file changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> 

Hi Bart,

As the DBR clear event occurs before !ufshcd_cmd_inflight, 
this could potentially cause an additional usleep_range wait. 
If an additional wait of 100~200 us is not a concern, then 
I think it should be fine.

Thanks
Peter

Re: [PATCH 4/7] scsi: ufs: core: Fix ufshcd_exception_event_handler()

[Peter Wang (王信友)]

On Fri, 2024-10-18 at 12:35 -0700, Bart Van Assche wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  
> On 10/18/24 10:25 AM, Avri Altman wrote:
> > No. But the Fixes tag seems strange, isn't it?
> 
> How about replacing the entire patch with the patch below?
> 
> Thanks,
> 
> Bart.
> 
> 
> scsi: ufs: core: Simplify ufshcd_exception_event_handler()
> 
> The ufshcd_scsi_block_requests() and ufshcd_scsi_unblock_requests()
> calls were introduced in ufshcd_exception_event_handler() to prevent
> that querying the exception event information would time out. Commit
> 10fe5888a40e ("scsi: ufs: increase the scsi query response timeout")
> increased the timeout for querying exception information from 30 ms
> to
> 1.5 s and thereby eliminated the risk that a timeout would happen.
> Hence, the calls to block and unblock SCSI requests are superfluous.
> Remove these calls.
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 76884df580c3..2fde1b0a6086 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -6195,12 +6195,11 @@ static void 
> ufshcd_exception_event_handler(struct work_struct *work)
>   u32 status = 0;
>   hba = container_of(work, struct ufs_hba, eeh_work);
> 
> -ufshcd_scsi_block_requests(hba);
>   err = ufshcd_get_ee_status(hba, &status);
>   if (err) {
>   dev_err(hba->dev, "%s: failed to get exception status %d\n",
>   __func__, err);
> -goto out;
> +return;
>   }
> 
>   trace_ufshcd_exception_event(dev_name(hba->dev), status);
> @@ -6212,8 +6211,6 @@ static void
> ufshcd_exception_event_handler(struct 
> work_struct *work)
>   ufshcd_temp_exception_event_handler(hba, status);
> 
>   ufs_debugfs_exception_event(hba, status);
> -out:
> -ufshcd_scsi_unblock_requests(hba);
>   }
> 
>   /* Complete requests that have door-bell cleared */
> 

Hi Bart,

This patch looks good to me as well.

Thanks
Peter

Re: [PATCH 5/7] scsi: ufs: core: Simplify ufshcd_err_handling_prepare()

[Peter Wang (王信友)]

On Wed, 2024-10-16 at 14:11 -0700, Bart Van Assche wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Use blk_mq_quiesce_tagset() instead of ufshcd_scsi_block_requests()
> and blk_mq_wait_quiesce_done().
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> 

Hi Bart,

Using blk_mq_quiesce_tagset instead of ufshcd_scsi_block_requests 
could cause issues. After the patch below was merged, Mediatek 
received three cases of IO hang.
77691af484e2 ("scsi: ufs: core: Quiesce request queues before checking
pending cmds")
I think this patch might need to be reverted first.

Here is backtrace of IO hang.
ppid=3952 pid=3952 D cpu=6 prio=120 wait=188s kworker/u16:0
	vmlinux __synchronize_srcu() + 216
</proc/self/cwd/common/kernel/rcu/srcutree.c:1386>
	vmlinux synchronize_srcu() + 276
</proc/self/cwd/common/kernel/rcu/srcutree.c:0>
	vmlinux blk_mq_wait_quiesce_done() + 20
</proc/self/cwd/common/block/blk-mq.c:226>
	vmlinux blk_mq_quiesce_tagset() + 156
</proc/self/cwd/common/block/blk-mq.c:286>
	vmlinux ufshcd_clock_scaling_prepare(timeout_us=1000000) + 16
</proc/self/cwd/common/drivers/ufs/core/ufshcd.c:1276>
	vmlinux ufshcd_devfreq_scale() + 52
</proc/self/cwd/common/drivers/ufs/core/ufshcd.c:1322>
	vmlinux ufshcd_devfreq_target() + 384
</proc/self/cwd/common/drivers/ufs/core/ufshcd.c:1440>
	vmlinux devfreq_set_target(flags=0) + 184
</proc/self/cwd/common/drivers/devfreq/devfreq.c:363>
	vmlinux devfreq_update_target(freq=0) + 296
</proc/self/cwd/common/drivers/devfreq/devfreq.c:429>
	vmlinux update_devfreq() + 8
</proc/self/cwd/common/drivers/devfreq/devfreq.c:444>
	vmlinux devfreq_monitor() + 48
</proc/self/cwd/common/drivers/devfreq/devfreq.c:460>
	vmlinux process_one_work() + 476
</proc/self/cwd/common/kernel/workqueue.c:2643>
	vmlinux process_scheduled_works() + 580
</proc/self/cwd/common/kernel/workqueue.c:2717>
	vmlinux worker_thread() + 576
</proc/self/cwd/common/kernel/workqueue.c:2798>
	vmlinux kthread() + 272
</proc/self/cwd/common/kernel/kthread.c:388>
	vmlinux 0xFFFFFFE239A164EC()
</proc/self/cwd/common/arch/arm64/kernel/entry.S:846>

Thanks
Peter

Re: [PATCH 3/7] scsi: ufs: core: Simplify ufshcd_try_to_abort_task()

[Bart Van Assche]

On 10/21/24 1:58 AM, Peter Wang (王信友) wrote:
> As the DBR clear event occurs before !ufshcd_cmd_inflight,
> this could potentially cause an additional usleep_range wait.
> If an additional wait of 100~200 us is not a concern, then
> I think it should be fine.

Hi Peter,

ufshcd_try_to_abort_task() is typically called if no completion will be
received from the UFS device. I think that the potential additional loop
iteration is acceptable since it is rare that a UFS device reports a
completion while ufshcd_try_to_abort_task() is in progress and since
this is an error path and not the hot path.

Thanks,

Bart.

Re: [PATCH 5/7] scsi: ufs: core: Simplify ufshcd_err_handling_prepare()

[Bart Van Assche]

On 10/21/24 2:43 AM, Peter Wang (王信友) wrote:
> Using blk_mq_quiesce_tagset instead of ufshcd_scsi_block_requests
> could cause issues. After the patch below was merged, Mediatek
> received three cases of IO hang.
> 77691af484e2 ("scsi: ufs: core: Quiesce request queues before checking
> pending cmds")
> I think this patch might need to be reverted first.
> 
> Here is backtrace of IO hang.
> ppid=3952 pid=3952 D cpu=6 prio=120 wait=188s kworker/u16:0
> 	vmlinux __synchronize_srcu() + 216
> </proc/self/cwd/common/kernel/rcu/srcutree.c:1386>
> 	vmlinux synchronize_srcu() + 276
> </proc/self/cwd/common/kernel/rcu/srcutree.c:0>
> 	vmlinux blk_mq_wait_quiesce_done() + 20
> </proc/self/cwd/common/block/blk-mq.c:226>
> 	vmlinux blk_mq_quiesce_tagset() + 156
> </proc/self/cwd/common/block/blk-mq.c:286>
> 	vmlinux ufshcd_clock_scaling_prepare(timeout_us=1000000) + 16
> </proc/self/cwd/common/drivers/ufs/core/ufshcd.c:1276>
> 	vmlinux ufshcd_devfreq_scale() + 52
> </proc/self/cwd/common/drivers/ufs/core/ufshcd.c:1322>
> 	vmlinux ufshcd_devfreq_target() + 384
> </proc/self/cwd/common/drivers/ufs/core/ufshcd.c:1440>
> 	vmlinux devfreq_set_target(flags=0) + 184
> </proc/self/cwd/common/drivers/devfreq/devfreq.c:363>
> 	vmlinux devfreq_update_target(freq=0) + 296
> </proc/self/cwd/common/drivers/devfreq/devfreq.c:429>
> 	vmlinux update_devfreq() + 8
> </proc/self/cwd/common/drivers/devfreq/devfreq.c:444>
> 	vmlinux devfreq_monitor() + 48
> </proc/self/cwd/common/drivers/devfreq/devfreq.c:460>
> 	vmlinux process_one_work() + 476
> </proc/self/cwd/common/kernel/workqueue.c:2643>
> 	vmlinux process_scheduled_works() + 580
> </proc/self/cwd/common/kernel/workqueue.c:2717>
> 	vmlinux worker_thread() + 576
> </proc/self/cwd/common/kernel/workqueue.c:2798>
> 	vmlinux kthread() + 272
> </proc/self/cwd/common/kernel/kthread.c:388>
> 	vmlinux 0xFFFFFFE239A164EC()
> </proc/self/cwd/common/arch/arm64/kernel/entry.S:846>

Hi Peter,

Thank you very much for having reported this hang early. Would it be
possible for you to test the patch below on top of this patch series?
I think the root cause of the hang that you reported is in the block
layer.

Thanks,

Bart.


diff --git a/block/blk-mq.c b/block/blk-mq.c
index 7b02188feed5..7482e682deca 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -283,8 +283,9 @@ void blk_mq_quiesce_tagset(struct blk_mq_tag_set *set)
  		if (!blk_queue_skip_tagset_quiesce(q))
  			blk_mq_quiesce_queue_nowait(q);
  	}
-	blk_mq_wait_quiesce_done(set);
  	mutex_unlock(&set->tag_list_lock);
+
+	blk_mq_wait_quiesce_done(set);
  }
  EXPORT_SYMBOL_GPL(blk_mq_quiesce_tagset);

Re: [PATCH 4/7] scsi: ufs: core: Fix ufshcd_exception_event_handler()

[Bart Van Assche]

On 10/21/24 1:59 AM, Peter Wang (王信友) wrote:
> This patch looks good to me as well.

Does this count as a Reviewed-by?

Thanks,

Bart.

Re: [PATCH 4/7] scsi: ufs: core: Fix ufshcd_exception_event_handler()

[Peter Wang (王信友)]

On Mon, 2024-10-21 at 16:25 -0700, Bart Van Assche wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 10/21/24 1:59 AM, Peter Wang (王信友) wrote:
> > This patch looks good to me as well.
> 
> Does this count as a Reviewed-by?
> 
> Thanks,
> 
> Bart.
> 

Hi Bart,

Yes, I also think that an extra loop shouldn't have too much 
impact. It's just to ensure that you haven't overlooked 
this aspect or that others might have different opinions.

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Thanks
Peter

Re: [PATCH 5/7] scsi: ufs: core: Simplify ufshcd_err_handling_prepare()

[Peter Wang (王信友)]

On Mon, 2024-10-21 at 13:41 -0700, Bart Van Assche wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 10/21/24 2:43 AM, Peter Wang (王信友) wrote:
> > Using blk_mq_quiesce_tagset instead of ufshcd_scsi_block_requests
> > could cause issues. After the patch below was merged, Mediatek
> > received three cases of IO hang.
> > 77691af484e2 ("scsi: ufs: core: Quiesce request queues before
> checking
> > pending cmds")
> > I think this patch might need to be reverted first.
> > 
> > Here is backtrace of IO hang.
> > ppid=3952 pid=3952 D cpu=6 prio=120 wait=188s kworker/u16:0
> > vmlinux __synchronize_srcu() + 216
> > </proc/self/cwd/common/kernel/rcu/srcutree.c:1386>
> > vmlinux synchronize_srcu() + 276
> > </proc/self/cwd/common/kernel/rcu/srcutree.c:0>
> > vmlinux blk_mq_wait_quiesce_done() + 20
> > </proc/self/cwd/common/block/blk-mq.c:226>
> > vmlinux blk_mq_quiesce_tagset() + 156
> > </proc/self/cwd/common/block/blk-mq.c:286>
> > vmlinux ufshcd_clock_scaling_prepare(timeout_us=1000000) + 16
> > </proc/self/cwd/common/drivers/ufs/core/ufshcd.c:1276>
> > vmlinux ufshcd_devfreq_scale() + 52
> > </proc/self/cwd/common/drivers/ufs/core/ufshcd.c:1322>
> > vmlinux ufshcd_devfreq_target() + 384
> > </proc/self/cwd/common/drivers/ufs/core/ufshcd.c:1440>
> > vmlinux devfreq_set_target(flags=0) + 184
> > </proc/self/cwd/common/drivers/devfreq/devfreq.c:363>
> > vmlinux devfreq_update_target(freq=0) + 296
> > </proc/self/cwd/common/drivers/devfreq/devfreq.c:429>
> > vmlinux update_devfreq() + 8
> > </proc/self/cwd/common/drivers/devfreq/devfreq.c:444>
> > vmlinux devfreq_monitor() + 48
> > </proc/self/cwd/common/drivers/devfreq/devfreq.c:460>
> > vmlinux process_one_work() + 476
> > </proc/self/cwd/common/kernel/workqueue.c:2643>
> > vmlinux process_scheduled_works() + 580
> > </proc/self/cwd/common/kernel/workqueue.c:2717>
> > vmlinux worker_thread() + 576
> > </proc/self/cwd/common/kernel/workqueue.c:2798>
> > vmlinux kthread() + 272
> > </proc/self/cwd/common/kernel/kthread.c:388>
> > vmlinux 0xFFFFFFE239A164EC()
> > </proc/self/cwd/common/arch/arm64/kernel/entry.S:846>
> 
> Hi Peter,
> 
> Thank you very much for having reported this hang early. Would it be
> possible for you to test the patch below on top of this patch series?
> I think the root cause of the hang that you reported is in the block
> layer.
> 
> Thanks,
> 
> Bart.
> 
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 7b02188feed5..7482e682deca 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -283,8 +283,9 @@ void blk_mq_quiesce_tagset(struct blk_mq_tag_set
> *set)
>   if (!blk_queue_skip_tagset_quiesce(q))
>   blk_mq_quiesce_queue_nowait(q);
>   }
> -blk_mq_wait_quiesce_done(set);
>   mutex_unlock(&set->tag_list_lock);
> +
> +blk_mq_wait_quiesce_done(set);
>   }
>   EXPORT_SYMBOL_GPL(blk_mq_quiesce_tagset);
> 
> 

Hi Bart,

We can test this patch, but because the low probability of the 
issue reproduce rate, I'm concerned that not encountering it 
during testing doesn't necessarily mean the problem has been 
truly resolved. 
However, from my understanding, this patch should be able to 
resolve the deadlock.

Thanks
Peter