[v2,09/12] scsi: ufs: Simplify ufshcd_abort_all()

Unify the MCQ and legacy code paths. This patch reworks code introduced by
commit ab248643d3d6 ("scsi: ufs: core: Add error handling for MCQ mode").

Cc: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 46 +++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

On 11/14/23 22:57, Peter Wang (王信友) wrote:
> On Thu, 2023-07-27 at 12:41 -0700, Bart Van Assche wrote:
>> Unify the MCQ and legacy code paths. This patch reworks code
>> introduced by
>> commit ab248643d3d6 ("scsi: ufs: core: Add error handling for MCQ
>> mode").
>>
>> Cc: Bao D. Nguyen <quic_nguyenb@quicinc.com>
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> ---
>>   drivers/ufs/core/ufshcd.c | 46 +++++++++++++++++------------------
>> ----
>>   1 file changed, 20 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index c0031cf8855c..bf76ea59ba6c 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -6387,6 +6387,22 @@ static bool
>> ufshcd_is_pwr_mode_restore_needed(struct ufs_hba *hba)
>>   	return false;
>>   }
>>   
>> +static bool ufshcd_abort_one(struct request *rq, void *priv)
>> +{
>> +	int *ret = priv;
>> +	u32 tag = rq->tag;
>> +	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
>> +	struct scsi_device *sdev = cmd->device;
>> +	struct Scsi_Host *shost = sdev->host;
>> +	struct ufs_hba *hba = shost_priv(shost);
>> +
>> +	*ret = ufshcd_try_to_abort_task(hba, tag);
>> +	dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
>> +		hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
>> +		*ret ? "failed" : "succeeded");
>> +	return *ret == 0;
>> +}
>> +
>>   /**
>>    * ufshcd_abort_all - Abort all pending commands.
>>    * @hba: Host bus adapter pointer.
>> @@ -6395,34 +6411,12 @@ static bool
>> ufshcd_is_pwr_mode_restore_needed(struct ufs_hba *hba)
>>    */
>>   static bool ufshcd_abort_all(struct ufs_hba *hba)
>>   {
>> -	int tag, ret;
>> +	int tag, ret = 0;
>>   
>> -	if (is_mcq_enabled(hba)) {
>> -		struct ufshcd_lrb *lrbp;
>> -		int tag;
>> +	blk_mq_tagset_busy_iter(&hba->host->tag_set, ufshcd_abort_one,
>> &ret);
>> +	if (ret)
>> +		goto out;
>>   
>> -		for (tag = 0; tag < hba->nutrs; tag++) {
>> -			lrbp = &hba->lrb[tag];
>> -			if (!ufshcd_cmd_inflight(lrbp->cmd))
>> -				continue;
>> -			ret = ufshcd_try_to_abort_task(hba, tag);
>> -			dev_err(hba->dev, "Aborting tag %d / CDB %#02x
>> %s\n", tag,
>> -				hba->lrb[tag].cmd ? hba->lrb[tag].cmd-
>>> cmnd[0] : -1,
>> -				ret ? "failed" : "succeeded");
>> -			if (ret)
>> -				goto out;
>> -		}
>> -	} else {
>> -		/* Clear pending transfer requests */
>> -		for_each_set_bit(tag, &hba->outstanding_reqs, hba-
>>> nutrs) {
>> -			ret = ufshcd_try_to_abort_task(hba, tag);
>> -			dev_err(hba->dev, "Aborting tag %d / CDB %#02x
>> %s\n", tag,
>> -				hba->lrb[tag].cmd ? hba->lrb[tag].cmd-
>>> cmnd[0] : -1,
>> -				ret ? "failed" : "succeeded");
>> -			if (ret)
>> -				goto out;
>> -		}
>> -	}
>>   	/* Clear pending task management requests */
>>   	for_each_set_bit(tag, &hba->outstanding_tasks, hba->nutmrs) {
>>   		ret = ufshcd_clear_tm_cmd(hba, tag);
> 
> Hi Bart,
> 
> Previous ufshcd_try_to_abort_task retrun fail will break tag iterate
> and return true to tell caller need reset.
> But this patch only return last tag ufshcd_try_to_abort_task return
> value, if some tag abort fail and last tag abort success, will not
> retrun true to tell caller need reset, am I right?

Hi Peter,

blk_mq_tagset_busy_iter() keeps iterating as long as the callback function
that has been passed as the second argument returns true. ufshcd_abort_one()
returns false if ufshcd_try_to_abort_task() fails.

The behavior that a host reset is triggered if ufshcd_try_to_abort_task()
fails has been preserved.

Bart.