public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
       [not found] <CGME20250516083723epcas5p216a21ffb81631d35f0a12f7a583ea248@epcas5p2.samsung.com>
@ 2025-05-16  8:38 ` ping.gao
  2025-05-16  9:36   ` Peter Wang (王信友)
                     ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: ping.gao @ 2025-05-16  8:38 UTC (permalink / raw)
  To: alim.akhtar, avri.altman, bvanassche, James.Bottomley,
	martin.petersen, peter.wang, minwoo.im, manivannan.sadhasivam,
	chenyuan0y, ping.gao, cw9316.lee, linux-scsi, linux-kernel

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
  2025-05-16  8:38 ` [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort ping.gao
@ 2025-05-16  9:36   ` Peter Wang (王信友)
  2025-05-16 14:18   ` Bart Van Assche
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Peter Wang (王信友) @ 2025-05-16  9:36 UTC (permalink / raw)
  To: avri.altman@wdc.com, linux-kernel@vger.kernel.org,
	cw9316.lee@samsung.com, chenyuan0y@gmail.com,
	manivannan.sadhasivam@linaro.org, bvanassche@acm.org,
	alim.akhtar@samsung.com, ping.gao@samsung.com,
	minwoo.im@samsung.com, linux-scsi@vger.kernel.org,
	martin.petersen@oracle.com, James.Bottomley@HansenPartnership.com

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>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
  2025-05-16  8:38 ` [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort ping.gao
  2025-05-16  9:36   ` Peter Wang (王信友)
@ 2025-05-16 14:18   ` Bart Van Assche
  2025-05-17  5:28   ` kernel test robot
  2025-05-28  2:20   ` Martin K. Petersen
  3 siblings, 0 replies; 9+ messages in thread
From: Bart Van Assche @ 2025-05-16 14:18 UTC (permalink / raw)
  To: ping.gao, alim.akhtar, avri.altman, James.Bottomley,
	martin.petersen, peter.wang, minwoo.im, manivannan.sadhasivam,
	chenyuan0y, cw9316.lee, linux-scsi, linux-kernel

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
  2025-05-16  8:38 ` [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort ping.gao
  2025-05-16  9:36   ` Peter Wang (王信友)
  2025-05-16 14:18   ` Bart Van Assche
@ 2025-05-17  5:28   ` kernel test robot
  2025-05-28  2:20   ` Martin K. Petersen
  3 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2025-05-17  5:28 UTC (permalink / raw)
  To: ping.gao, alim.akhtar, avri.altman, bvanassche, James.Bottomley,
	martin.petersen, peter.wang, minwoo.im, manivannan.sadhasivam,
	chenyuan0y, cw9316.lee, linux-scsi, linux-kernel
  Cc: llvm, oe-kbuild-all

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
       [not found] <CGME20250519025509epcas5p2bdc884392dafa224289b337c1232daf3@epcas5p2.samsung.com>
@ 2025-05-19  2:55 ` ping.gao
  2025-05-19 21:03   ` Bart Van Assche
                     ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: ping.gao @ 2025-05-19  2:55 UTC (permalink / raw)
  To: alim.akhtar, avri.altman, bvanassche, James.Bottomley,
	martin.petersen, peter.wang, minwoo.im, manivannan.sadhasivam,
	chenyuan0y, ping.gao, cw9316.lee, linux-scsi, linux-kernel

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
  2025-05-19  2:55 ` ping.gao
@ 2025-05-19 21:03   ` Bart Van Assche
  2025-05-20  2:40   ` Peter Wang (王信友)
  2025-05-21  1:39   ` Martin K. Petersen
  2 siblings, 0 replies; 9+ messages in thread
From: Bart Van Assche @ 2025-05-19 21:03 UTC (permalink / raw)
  To: ping.gao, alim.akhtar, avri.altman, James.Bottomley,
	martin.petersen, peter.wang, minwoo.im, manivannan.sadhasivam,
	chenyuan0y, cw9316.lee, linux-scsi, linux-kernel

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>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
  2025-05-19  2:55 ` ping.gao
  2025-05-19 21:03   ` Bart Van Assche
@ 2025-05-20  2:40   ` Peter Wang (王信友)
  2025-05-21  1:39   ` Martin K. Petersen
  2 siblings, 0 replies; 9+ messages in thread
From: Peter Wang (王信友) @ 2025-05-20  2:40 UTC (permalink / raw)
  To: avri.altman@wdc.com, linux-kernel@vger.kernel.org,
	cw9316.lee@samsung.com, chenyuan0y@gmail.com,
	manivannan.sadhasivam@linaro.org, bvanassche@acm.org,
	alim.akhtar@samsung.com, ping.gao@samsung.com,
	minwoo.im@samsung.com, linux-scsi@vger.kernel.org,
	martin.petersen@oracle.com, James.Bottomley@HansenPartnership.com

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>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
  2025-05-19  2:55 ` ping.gao
  2025-05-19 21:03   ` Bart Van Assche
  2025-05-20  2:40   ` Peter Wang (王信友)
@ 2025-05-21  1:39   ` Martin K. Petersen
  2 siblings, 0 replies; 9+ messages in thread
From: Martin K. Petersen @ 2025-05-21  1:39 UTC (permalink / raw)
  To: ping.gao
  Cc: alim.akhtar, avri.altman, bvanassche, James.Bottomley,
	martin.petersen, peter.wang, minwoo.im, manivannan.sadhasivam,
	chenyuan0y, cw9316.lee, linux-scsi, linux-kernel


> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort
  2025-05-16  8:38 ` [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort ping.gao
                     ` (2 preceding siblings ...)
  2025-05-17  5:28   ` kernel test robot
@ 2025-05-28  2:20   ` Martin K. Petersen
  3 siblings, 0 replies; 9+ messages in thread
From: Martin K. Petersen @ 2025-05-28  2:20 UTC (permalink / raw)
  To: alim.akhtar, avri.altman, bvanassche, James.Bottomley, peter.wang,
	minwoo.im, manivannan.sadhasivam, chenyuan0y, cw9316.lee,
	linux-scsi, linux-kernel, ping.gao
  Cc: 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-05-28  2:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20250516083723epcas5p216a21ffb81631d35f0a12f7a583ea248@epcas5p2.samsung.com>
2025-05-16  8:38 ` [PATCH] scsi: ufs: mcq: delete ufshcd_release_scsi_cmd in ufshcd_mcq_abort ping.gao
2025-05-16  9:36   ` Peter Wang (王信友)
2025-05-16 14:18   ` Bart Van Assche
2025-05-17  5:28   ` kernel test robot
2025-05-28  2:20   ` Martin K. Petersen
     [not found] <CGME20250519025509epcas5p2bdc884392dafa224289b337c1232daf3@epcas5p2.samsung.com>
2025-05-19  2:55 ` ping.gao
2025-05-19 21:03   ` Bart Van Assche
2025-05-20  2:40   ` Peter Wang (王信友)
2025-05-21  1:39   ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox