[PATCH v1] scsi: ufs: core: print trs for pending requests in MCQ mode

[PATCH v1] scsi: ufs: core: print trs for pending requests in MCQ mode

[Ziqi Chen]

We don't have outstanding_reqs bitmap in MCQ mode. And in consideration
of the queue depth may increase beyond 64 in the future, we reworked
ufshcd_print_trs() to get rid of usage of bitmap so that we can print
trs for pending requests in both SDB and MCQ mode.

Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
---
 drivers/ufs/core/ufshcd.c | 68 +++++++++++++++++++++++++++--------------------
 1 file changed, 39 insertions(+), 29 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index e43aee1..a654d66 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -542,47 +542,57 @@ static void ufshcd_print_evt_hist(struct ufs_hba *hba)
 }
 
 static
-void ufshcd_print_trs(struct ufs_hba *hba, unsigned long bitmap, bool pr_prdt)
+void ufshcd_print_tr(struct ufs_hba *hba, int tag, bool pr_prdt)
 {
 	const struct ufshcd_lrb *lrbp;
 	int prdt_length;
-	int tag;
 
-	for_each_set_bit(tag, &bitmap, hba->nutrs) {
-		lrbp = &hba->lrb[tag];
+	lrbp = &hba->lrb[tag];
 
-		dev_err(hba->dev, "UPIU[%d] - issue time %lld us\n",
-				tag, div_u64(lrbp->issue_time_stamp_local_clock, 1000));
-		dev_err(hba->dev, "UPIU[%d] - complete time %lld us\n",
-				tag, div_u64(lrbp->compl_time_stamp_local_clock, 1000));
-		dev_err(hba->dev,
+	dev_err(hba->dev, "UPIU[%d] - issue time %lld us\n",
+			tag, div_u64(lrbp->issue_time_stamp_local_clock, 1000));
+	dev_err(hba->dev, "UPIU[%d] - complete time %lld us\n",
+			tag, div_u64(lrbp->compl_time_stamp_local_clock, 1000));
+	dev_err(hba->dev,
 			"UPIU[%d] - Transfer Request Descriptor phys@0x%llx\n",
 			tag, (u64)lrbp->utrd_dma_addr);
 
-		ufshcd_hex_dump("UPIU TRD: ", lrbp->utr_descriptor_ptr,
-				sizeof(struct utp_transfer_req_desc));
-		dev_err(hba->dev, "UPIU[%d] - Request UPIU phys@0x%llx\n", tag,
+	ufshcd_hex_dump("UPIU TRD: ", lrbp->utr_descriptor_ptr,
+					sizeof(struct utp_transfer_req_desc));
+	dev_err(hba->dev, "UPIU[%d] - Request UPIU phys@0x%llx\n", tag,
 			(u64)lrbp->ucd_req_dma_addr);
-		ufshcd_hex_dump("UPIU REQ: ", lrbp->ucd_req_ptr,
-				sizeof(struct utp_upiu_req));
-		dev_err(hba->dev, "UPIU[%d] - Response UPIU phys@0x%llx\n", tag,
+	ufshcd_hex_dump("UPIU REQ: ", lrbp->ucd_req_ptr,
+					sizeof(struct utp_upiu_req));
+	dev_err(hba->dev, "UPIU[%d] - Response UPIU phys@0x%llx\n", tag,
 			(u64)lrbp->ucd_rsp_dma_addr);
-		ufshcd_hex_dump("UPIU RSP: ", lrbp->ucd_rsp_ptr,
-				sizeof(struct utp_upiu_rsp));
+	ufshcd_hex_dump("UPIU RSP: ", lrbp->ucd_rsp_ptr,
+					sizeof(struct utp_upiu_rsp));
 
-		prdt_length = le16_to_cpu(
-			lrbp->utr_descriptor_ptr->prd_table_length);
-		if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN)
-			prdt_length /= ufshcd_sg_entry_size(hba);
+	prdt_length = le16_to_cpu(
+		lrbp->utr_descriptor_ptr->prd_table_length);
+	if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN)
+		prdt_length /= ufshcd_sg_entry_size(hba);
 
-		dev_err(hba->dev,
+	dev_err(hba->dev,
 			"UPIU[%d] - PRDT - %d entries  phys@0x%llx\n",
 			tag, prdt_length,
 			(u64)lrbp->ucd_prdt_dma_addr);
 
-		if (pr_prdt)
-			ufshcd_hex_dump("UPIU PRDT: ", lrbp->ucd_prdt_ptr,
-				ufshcd_sg_entry_size(hba) * prdt_length);
+	if (pr_prdt)
+		ufshcd_hex_dump("UPIU PRDT: ", lrbp->ucd_prdt_ptr,
+						ufshcd_sg_entry_size(hba) * prdt_length);
+}
+
+static void ufshcd_print_trs_all(struct ufs_hba *hba, bool pr_prdt)
+{
+	const struct ufshcd_lrb *lrbp;
+	int tag;
+
+	for (tag = 0; tag < hba->nutrs; tag++) {
+		lrbp = &hba->lrb[tag];
+
+		if (lrbp->cmd)
+			ufshcd_print_tr(hba, tag, pr_prdt);
 	}
 }
 
@@ -5332,7 +5342,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
 
 	if ((host_byte(result) != DID_OK) &&
 	    (host_byte(result) != DID_REQUEUE) && !hba->silence_err_logs)
-		ufshcd_print_trs(hba, 1 << lrbp->task_tag, true);
+		ufshcd_print_tr(hba, lrbp->task_tag, true);
 	return result;
 }
 
@@ -6406,7 +6416,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 		ufshcd_print_pwr_info(hba);
 		ufshcd_print_evt_hist(hba);
 		ufshcd_print_tmrs(hba, hba->outstanding_tasks);
-		ufshcd_print_trs(hba, hba->outstanding_reqs, pr_prdt);
+		ufshcd_print_trs_all(hba, pr_prdt);
 		spin_lock_irqsave(hba->host->host_lock, flags);
 	}
 
@@ -7435,9 +7445,9 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 		ufshcd_print_evt_hist(hba);
 		ufshcd_print_host_state(hba);
 		ufshcd_print_pwr_info(hba);
-		ufshcd_print_trs(hba, 1 << tag, true);
+		ufshcd_print_tr(hba, tag, true);
 	} else {
-		ufshcd_print_trs(hba, 1 << tag, false);
+		ufshcd_print_tr(hba, tag, false);
 	}
 	hba->req_abort_count++;
 
-- 
2.7.4

Re: [PATCH v1] scsi: ufs: core: print trs for pending requests in MCQ mode

[Bart Van Assche]

On 3/8/23 21:01, Ziqi Chen wrote:
> +static void ufshcd_print_trs_all(struct ufs_hba *hba, bool pr_prdt)
> +{
> +	const struct ufshcd_lrb *lrbp;
> +	int tag;
> +
> +	for (tag = 0; tag < hba->nutrs; tag++) {
> +		lrbp = &hba->lrb[tag];
> +
> +		if (lrbp->cmd)
> +			ufshcd_print_tr(hba, tag, pr_prdt);
>   	}
>   }

There is a race in this code: lrbp->cmd may be cleared after it has been 
read and before or while ufshcd_print_tr() is in progress. How about 
preventing that race by blk_mq_tagset_busy_iter() to iterate over 
pending SCSI commands?

Thanks,

Bart.

Re: [PATCH v1] scsi: ufs: core: print trs for pending requests in MCQ mode

[Ziqi Chen]

Hi Bart,

Good idea, let met re-design it.

Thanks,
Ziqi

On 3/10/2023 1:28 AM, Bart Van Assche wrote:
> On 3/8/23 21:01, Ziqi Chen wrote:
>> +static void ufshcd_print_trs_all(struct ufs_hba *hba, bool pr_prdt)
>> +{
>> +    const struct ufshcd_lrb *lrbp;
>> +    int tag;
>> +
>> +    for (tag = 0; tag < hba->nutrs; tag++) {
>> +        lrbp = &hba->lrb[tag];
>> +
>> +        if (lrbp->cmd)
>> +            ufshcd_print_tr(hba, tag, pr_prdt);
>>       }
>>   }
> 
> There is a race in this code: lrbp->cmd may be cleared after it has been 
> read and before or while ufshcd_print_tr() is in progress. How about 
> preventing that race by blk_mq_tagset_busy_iter() to iterate over 
> pending SCSI commands?
> 
> Thanks,
> 
> Bart.