[PATCH 3/9] scsi: ufs: core: Wait for the reserved tag in ufshcd_get_dev_mgmt_cmd()

[Stanley Jhu <stanleyjhu@google.com>]

ufshcd_get_dev_mgmt_cmd() allocates the reserved tag with
BLK_MQ_REQ_NOWAIT and assumes that holding hba->dev_cmd.lock makes the
allocation succeed. That lock only serializes the owners of the tag. A
busy tag iterator holds its own reference on the request, and the tag
is freed only when the last reference is dropped:

1. A PM callback, e.g. __ufshcd_wl_suspend(), sends a query request and
   waits for it in ufshcd_issue_dev_cmd().
2. The request times out. blk_mq_timeout_work() takes a reference on
   it in bt_iter() and calls ufshcd_eh_timed_out(), which completes it
   with scsi_done().
3. The PM callback wakes up, frees the request, releases
   hba->dev_cmd.lock and retries the query from a retry loop such as
   ufshcd_query_attr_retry().
4. The retry calls ufshcd_get_dev_mgmt_cmd() before
   blk_mq_timeout_work() has dropped its reference. The tag is still
   busy, the NOWAIT allocation fails, ufshcd_exec_dev_cmd() triggers
   WARN_ON_ONCE(!cmd) and the query fails with -ENOMEM.

The timeout handler is not the only trigger. Other busy tag iterators,
e.g. scsi_host_busy() called from ufshcd_gate_work(), take the same
reference, so the tag can stay busy after its owner has freed the
request. Holding hba->dev_cmd.lock therefore does not guarantee that
the NOWAIT allocation succeeds.

Fix this by dropping BLK_MQ_REQ_NOWAIT so that the allocation waits
until the reference is dropped and the tag is freed. Since the patch
"scsi: ufs: core: Defer link recovery on reserved PM command timeout",
the timeout handler no longer allocates a reserved tag while it holds
a reference on a reserved request, so the wait ends when
ufshcd_eh_timed_out() returns. Also correct the comment about why
hba->dev_cmd.lock is held, and state that the function must not be
called while holding a reference on a reserved request.

Fixes: 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
Cc: stable@vger.kernel.org # 7.3.x
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 6a5f5fa732d9..6d789f3badf1 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3366,14 +3366,17 @@ static void ufshcd_dev_man_unlock(struct ufs_hba *hba)
 static struct scsi_cmnd *ufshcd_get_dev_mgmt_cmd(struct ufs_hba *hba)
 {
 	/*
-	 * The caller must hold this lock to guarantee that the NOWAIT
-	 * allocation will succeed.
+	 * hba->dev_cmd.lock serializes the owners of the reserved tag. A busy
+	 * tag iterator, e.g. the block layer timeout handler, may still hold a
+	 * reference on the previous request after its owner freed it. Hence
+	 * the allocation may have to wait until that reference is dropped.
+	 * Must not be called while holding a reference on a reserved request.
 	 */
 	lockdep_assert_held(&hba->dev_cmd.lock);
 
 	return scsi_get_internal_cmd(
 		hba->host->pseudo_sdev, DMA_TO_DEVICE,
-		BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT);
+		BLK_MQ_REQ_RESERVED);
 }
 
 static void ufshcd_put_dev_mgmt_cmd(struct scsi_cmnd *cmd)
-- 
2.56.0.567.gdb80ae8cd5-goog