2024-05-24 01:59:22

by Chanwoo Lee

Subject: [PATCH v2] ufs:mcq:Fixing Error Output and cleanup for ufshcd_mcq_abort

An error unrelated to ufshcd_try_to_abort_task is being output and
can cause confusion. So, I modified it to output the result of abort
fail.
* dev_err(hba->dev, "%s: device abort failed %d\n", __func__, err);

And for readability,I modified it to return immediately instead of 'goto'.

Fixes: f1304d442077 ("scsi: ufs: mcq: Added ufshcd_mcq_abort()")
Signed-off-by: Chanwoo Lee <[email protected]>

---
v1->v2:
- Change title and description
- Change all 'goto out' statements into 'return FAILED'.
- Add 'return SUCCESS' at the end.
v1 : https://patchwork.kernel.org/project/linux-scsi/patch/[email protected]/
---
drivers/ufs/core/ufs-mcq.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 005d63ab1f44..8944548c30fa 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -634,20 +634,20 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
struct ufshcd_lrb *lrbp = &hba->lrb[tag];
struct ufs_hw_queue *hwq;
unsigned long flags;
- int err = FAILED;
+ int err;

if (!ufshcd_cmd_inflight(lrbp->cmd)) {
dev_err(hba->dev,
"%s: skip abort. cmd at tag %d already completed.\n",
__func__, tag);
- goto out;
+ return FAILED;
}

/* Skip task abort in case previous aborts failed and report failure */
if (lrbp->req_abort_skip) {
dev_err(hba->dev, "%s: skip abort. tag %d failed earlier\n",
__func__, tag);
- goto out;
+ return FAILED;
}

hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
@@ -659,7 +659,7 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
*/
dev_err(hba->dev, "%s: cmd found in sq. hwq=%d, tag=%d\n",
__func__, hwq->id, tag);
- goto out;
+ return FAILED;
}

/*
@@ -667,18 +667,17 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
* in the completion queue either. Query the device to see if
* the command is being processed in the device.
*/
- if (ufshcd_try_to_abort_task(hba, tag)) {
+ err = ufshcd_try_to_abort_task(hba, tag);
+ if (err) {
dev_err(hba->dev, "%s: device abort failed %d\n", __func__, err);
lrbp->req_abort_skip = true;
- goto out;
+ return FAILED;
}

- err = SUCCESS;
spin_lock_irqsave(&hwq->cq_lock, flags);
if (ufshcd_cmd_inflight(lrbp->cmd))
ufshcd_release_scsi_cmd(hba, lrbp);
spin_unlock_irqrestore(&hwq->cq_lock, flags);

-out:
- return err;
+ return SUCCESS;
}
--
2.34.1

2024-05-24 13:58:18

by Bart Van Assche

Subject: Re: [PATCH v2] ufs:mcq:Fixing Error Output and cleanup for ufshcd_mcq_abort

On 5/23/24 18:59, Chanwoo Lee wrote:
> An error unrelated to ufshcd_try_to_abort_task is being output and
> can cause confusion. So, I modified it to output the result of abort
> fail.
> * dev_err(hba->dev, "%s: device abort failed %d\n", __func__, err);
>
> And for readability,I modified it to return immediately instead of 'goto'.

Reviewed-by: Bart Van Assche <[email protected]>

2024-05-31 01:16:46

by Martin K. Petersen

Subject: Re: [PATCH v2] ufs:mcq:Fixing Error Output and cleanup for ufshcd_mcq_abort

On Fri, 24 May 2024 10:59:04 +0900, Chanwoo Lee wrote:

> An error unrelated to ufshcd_try_to_abort_task is being output and
> can cause confusion. So, I modified it to output the result of abort
> fail.
> * dev_err(hba->dev, "%s: device abort failed %d\n", __func__, err);
>
> And for readability,I modified it to return immediately instead of 'goto'.
>
> [...]

Applied to 6.10/scsi-fixes, thanks!

[1/1] ufs:mcq:Fixing Error Output and cleanup for ufshcd_mcq_abort
https://git.kernel.org/mkp/scsi/c/d53b681ce9ca

--
Martin K. Petersen Oracle Linux Engineering