[PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort

[PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort

[ping.gao]

after ufs UFS_ABORT_TASK process successfully , host will generate
mcq irq for abort tag with response OCS_ABORTED
ufshcd_compl_one_cqe ->
    ufshcd_release_scsi_cmd

But in ufshcd_mcq_abort already do ufshcd_release_scsi_cmd, this means
 __ufshcd_release will be done twice.

This means hba->clk_gating.active_reqs also will be decrease twice, it
will be negtive, so delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
function.

static void __ufshcd_release(struct ufs_hba *hba)
{
    if (!ufshcd_is_clkgating_allowed(hba))
        return;

    hba->clk_gating.active_reqs--;

    if (hba->clk_gating.active_reqs < 0) {
        panic("ufs abnormal active_reqs!!!!!!");
    }

    ...
}
Signed-off-by: ping.gao <ping.gao@samsung.com>
---
 drivers/ufs/core/ufs-mcq.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index f1294c29f484..2106c63db5ca 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -713,10 +713,5 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
 		return FAILED;
 	}
 
-	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);
-
 	return SUCCESS;
 }
-- 
2.25.1

Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort

[Peter Wang (王信友)]

On Fri, 2025-05-16 at 16:38 +0800, ping.gao wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> after ufs UFS_ABORT_TASK process successfully , host will generate
> mcq irq for abort tag with response OCS_ABORTED
> ufshcd_compl_one_cqe ->
>     ufshcd_release_scsi_cmd
> 
> But in ufshcd_mcq_abort already do ufshcd_release_scsi_cmd, this
> means
>  __ufshcd_release will be done twice.
> 
> This means hba->clk_gating.active_reqs also will be decrease twice,
> it
> will be negtive, so delete ufshcd_release_scsi_cmd in
> ufshcd_mcq_abort
> function.
> 
> static void __ufshcd_release(struct ufs_hba *hba)
> {
>     if (!ufshcd_is_clkgating_allowed(hba))
>         return;
> 
>     hba->clk_gating.active_reqs--;
> 
>     if (hba->clk_gating.active_reqs < 0) {
>         panic("ufs abnormal active_reqs!!!!!!");
>     }
> 
>     ...
> }
> Signed-off-by: ping.gao <ping.gao@samsung.com>
> ---
>  drivers/ufs/core/ufs-mcq.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index f1294c29f484..2106c63db5ca 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -713,10 +713,5 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
>                 return FAILED;
>         }
> 
> -       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);
> -
>         return SUCCESS;
>  }
> --
> 2.25.1
> 


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

Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort

[Bart Van Assche]

On 5/16/25 1:38 AM, ping.gao wrote:
> after ufs UFS_ABORT_TASK process successfully , host will generate
> mcq irq for abort tag with response OCS_ABORTED
> ufshcd_compl_one_cqe ->
>      ufshcd_release_scsi_cmd
> 
> But in ufshcd_mcq_abort already do ufshcd_release_scsi_cmd, this means
>   __ufshcd_release will be done twice.
> 
> This means hba->clk_gating.active_reqs also will be decrease twice, it
> will be negtive, so delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
> function.
> 
> static void __ufshcd_release(struct ufs_hba *hba)
> {
>      if (!ufshcd_is_clkgating_allowed(hba))
>          return;
> 
>      hba->clk_gating.active_reqs--;
> 
>      if (hba->clk_gating.active_reqs < 0) {
>          panic("ufs abnormal active_reqs!!!!!!");
>      }
> 
>      ...
> }
> Signed-off-by: ping.gao <ping.gao@samsung.com>
A blank line is required between a patch description and a Signed-off-by 
tag. Additionally, please add a Fixes: tag to the patch description.

Thanks,

Bart.

Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort

[kernel test robot]

Hi ping.gao,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jejb-scsi/for-next]
[also build test WARNING on mkp-scsi/for-next linus/master v6.15-rc6 next-20250516]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/ping-gao/scsi-ufs-mcq-delete-ufshcd_release_scsi_cmd-in-ufshcd_mcq_abort/20250516-163807
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
patch link:    https://lore.kernel.org/r/20250516083812.3894396-1-ping.gao%40samsung.com
patch subject: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
config: arm-randconfig-004-20250517 (https://download.01.org/0day-ci/archive/20250517/202505171237.VZSTTv75-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250517/202505171237.VZSTTv75-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505171237.VZSTTv75-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/ufs/core/ufs-mcq.c:677:16: warning: unused variable 'flags' [-Wunused-variable]
     677 |         unsigned long flags;
         |                       ^~~~~
   1 warning generated.


vim +/flags +677 drivers/ufs/core/ufs-mcq.c

f1304d4420777f Bao D. Nguyen   2023-05-29  663  
f1304d4420777f Bao D. Nguyen   2023-05-29  664  /**
f1304d4420777f Bao D. Nguyen   2023-05-29  665   * ufshcd_mcq_abort - Abort the command in MCQ.
317a38045ab763 Yang Li         2023-07-12  666   * @cmd: The command to be aborted.
f1304d4420777f Bao D. Nguyen   2023-05-29  667   *
3a17fefe0f1960 Bart Van Assche 2023-07-27  668   * Return: SUCCESS or FAILED error codes
f1304d4420777f Bao D. Nguyen   2023-05-29  669   */
f1304d4420777f Bao D. Nguyen   2023-05-29  670  int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
f1304d4420777f Bao D. Nguyen   2023-05-29  671  {
f1304d4420777f Bao D. Nguyen   2023-05-29  672  	struct Scsi_Host *host = cmd->device->host;
f1304d4420777f Bao D. Nguyen   2023-05-29  673  	struct ufs_hba *hba = shost_priv(host);
f1304d4420777f Bao D. Nguyen   2023-05-29  674  	int tag = scsi_cmd_to_rq(cmd)->tag;
f1304d4420777f Bao D. Nguyen   2023-05-29  675  	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
f1304d4420777f Bao D. Nguyen   2023-05-29  676  	struct ufs_hw_queue *hwq;
27900d7119c464 Peter Wang      2023-11-06 @677  	unsigned long flags;

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort

[Martin K. Petersen]

On Fri, 16 May 2025 16:38:12 +0800, ping.gao wrote:

> after ufs UFS_ABORT_TASK process successfully , host will generate
> mcq irq for abort tag with response OCS_ABORTED
> ufshcd_compl_one_cqe ->
>     ufshcd_release_scsi_cmd
> 
> But in ufshcd_mcq_abort already do ufshcd_release_scsi_cmd, this means
>  __ufshcd_release will be done twice.
> 
> [...]

Applied to 6.16/scsi-queue, thanks!

[1/1] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
      https://git.kernel.org/mkp/scsi/c/53755903b935

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort

[ping.gao]

after ufs UFS_ABORT_TASK process successfully , host will generate
mcq irq for abort tag with response OCS_ABORTED
ufshcd_compl_one_cqe ->
    ufshcd_release_scsi_cmd

But in ufshcd_mcq_abort already do ufshcd_release_scsi_cmd, this means
 __ufshcd_release will be done twice.

This means hba->clk_gating.active_reqs also will be decrease twice, it
will be negtive, so delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
function.

static void __ufshcd_release(struct ufs_hba *hba)
{
    if (!ufshcd_is_clkgating_allowed(hba))
        return;

    hba->clk_gating.active_reqs--;

    if (hba->clk_gating.active_reqs < 0) {
        panic("ufs abnormal active_reqs!!!!!!");
    }

    ...
}

Fixes: f1304d442077 ("scsi: ufs: mcq: Added ufshcd_mcq_abort()")
Signed-off-by: ping.gao <ping.gao@samsung.com>
---
 drivers/ufs/core/ufs-mcq.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index f1294c29f484..1e50675772fe 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -674,7 +674,6 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
 	int tag = scsi_cmd_to_rq(cmd)->tag;
 	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
 	struct ufs_hw_queue *hwq;
-	unsigned long flags;
 	int err;
 
 	/* Skip task abort in case previous aborts failed and report failure */
@@ -713,10 +712,5 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
 		return FAILED;
 	}
 
-	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);
-
 	return SUCCESS;
 }
-- 
2.25.1

Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort

[Bart Van Assche]

On 5/18/25 7:55 PM, ping.gao wrote:
> after ufs UFS_ABORT_TASK process successfully , host will generate
> mcq irq for abort tag with response OCS_ABORTED
> ufshcd_compl_one_cqe ->
>      ufshcd_release_scsi_cmd

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort

[Peter Wang (王信友)]

On Mon, 2025-05-19 at 10:55 +0800, ping.gao wrote:
> after ufs UFS_ABORT_TASK process successfully , host will generate
> mcq irq for abort tag with response OCS_ABORTED
> ufshcd_compl_one_cqe ->
>     ufshcd_release_scsi_cmd
> 
> But in ufshcd_mcq_abort already do ufshcd_release_scsi_cmd, this
> means
>  __ufshcd_release will be done twice.
> 
> This means hba->clk_gating.active_reqs also will be decrease twice,
> it
> will be negtive, so delete ufshcd_release_scsi_cmd in
> ufshcd_mcq_abort
> function.
> 
> static void __ufshcd_release(struct ufs_hba *hba)
> {
>     if (!ufshcd_is_clkgating_allowed(hba))
>         return;
> 
>     hba->clk_gating.active_reqs--;
> 
>     if (hba->clk_gating.active_reqs < 0) {
>         panic("ufs abnormal active_reqs!!!!!!");
>     }
> 
>     ...
> }
> 
> Fixes: f1304d442077 ("scsi: ufs: mcq: Added ufshcd_mcq_abort()")
> Signed-off-by: ping.gao <ping.gao@samsung.com>

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

Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort

[Martin K. Petersen]

> after ufs UFS_ABORT_TASK process successfully , host will generate
> mcq irq for abort tag with response OCS_ABORTED
> ufshcd_compl_one_cqe ->
>     ufshcd_release_scsi_cmd
>
> But in ufshcd_mcq_abort already do ufshcd_release_scsi_cmd, this means
>  __ufshcd_release will be done twice.
>
> This means hba->clk_gating.active_reqs also will be decrease twice, it
> will be negtive, so delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
> function.

Applied to 6.16/scsi-staging, thanks!

-- 
Martin K. Petersen