* [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error
@ 2020-05-22 4:59 Dinghao Liu
2020-05-22 8:36 ` [EXT] " Bean Huo (beanhuo)
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Dinghao Liu @ 2020-05-22 4:59 UTC (permalink / raw)
To: dinghao.liu, kjlu
Cc: Alim Akhtar, Avri Altman, James E.J. Bottomley,
Martin K. Petersen, Bean Huo, Bart Van Assche, Can Guo,
linux-scsi, linux-kernel
When ufs_bsg_alloc_desc_buffer() returns an error code,
a pairing runtime PM usage counter decrement is needed
to keep the counter balanced.
Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
---
drivers/scsi/ufs/ufs_bsg.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
index 53dd87628cbe..516a7f573942 100644
--- a/drivers/scsi/ufs/ufs_bsg.c
+++ b/drivers/scsi/ufs/ufs_bsg.c
@@ -106,8 +106,10 @@ static int ufs_bsg_request(struct bsg_job *job)
desc_op = bsg_request->upiu_req.qr.opcode;
ret = ufs_bsg_alloc_desc_buffer(hba, job, &desc_buff,
&desc_len, desc_op);
- if (ret)
+ if (ret) {
+ pm_runtime_put_sync(hba->dev);
goto out;
+ }
/* fall through */
case UPIU_TRANSACTION_NOP_OUT:
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [EXT] [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error
2020-05-22 4:59 [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error Dinghao Liu
@ 2020-05-22 8:36 ` Bean Huo (beanhuo)
2020-05-22 8:54 ` dinghao.liu
2020-06-10 2:11 ` Martin K. Petersen
2020-06-16 3:46 ` Martin K. Petersen
2 siblings, 1 reply; 8+ messages in thread
From: Bean Huo (beanhuo) @ 2020-05-22 8:36 UTC (permalink / raw)
To: Dinghao Liu, kjlu
Cc: Alim Akhtar, Avri Altman, James E.J. Bottomley,
Martin K. Petersen, Bart Van Assche, Can Guo, linux-scsi,
linux-kernel
> 1 file changed, 3 insertions(+), 1 deletion(-)
Hi, Dinghao
>
> diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c index
> 53dd87628cbe..516a7f573942 100644
> --- a/drivers/scsi/ufs/ufs_bsg.c
> +++ b/drivers/scsi/ufs/ufs_bsg.c
> @@ -106,8 +106,10 @@ static int ufs_bsg_request(struct bsg_job *job)
> desc_op = bsg_request->upiu_req.qr.opcode;
> ret = ufs_bsg_alloc_desc_buffer(hba, job, &desc_buff,
> &desc_len, desc_op);
> - if (ret)
> + if (ret) {
> + pm_runtime_put_sync(hba->dev);
No need to add pm_runtime_put_sync() here, you can change "goto out" to "break",
Or move original pm_runtime_put_sync() to after goto label.
> goto out;
> + }
>
> /* fall through */
> case UPIU_TRANSACTION_NOP_OUT:
> --
> 2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: RE: [EXT] [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error
2020-05-22 8:36 ` [EXT] " Bean Huo (beanhuo)
@ 2020-05-22 8:54 ` dinghao.liu
2020-05-22 14:55 ` Bean Huo (beanhuo)
0 siblings, 1 reply; 8+ messages in thread
From: dinghao.liu @ 2020-05-22 8:54 UTC (permalink / raw)
To: Bean Huo (beanhuo)
Cc: kjlu, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Martin K. Petersen, Bart Van Assche, Can Guo, linux-scsi,
linux-kernel
Hi, Bean
Thank you for your advice! Moving original pm_runtime_put_sync()
to after "out" label will influence an error path branched from
ups_bsg_verify_query_size(). So I think changing "goto out" to
"break" is a good idea. But in this case we may execute an extra
sg_copy_from_buffer() and an extra kfree() compared with unpatched
version. Does this matter?
Regards,
Dinghao
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> Hi, Dinghao
> >
> > diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c index
> > 53dd87628cbe..516a7f573942 100644
> > --- a/drivers/scsi/ufs/ufs_bsg.c
> > +++ b/drivers/scsi/ufs/ufs_bsg.c
> > @@ -106,8 +106,10 @@ static int ufs_bsg_request(struct bsg_job *job)
> > desc_op = bsg_request->upiu_req.qr.opcode;
> > ret = ufs_bsg_alloc_desc_buffer(hba, job, &desc_buff,
> > &desc_len, desc_op);
> > - if (ret)
> > + if (ret) {
> > + pm_runtime_put_sync(hba->dev);
>
> No need to add pm_runtime_put_sync() here, you can change "goto out" to "break",
> Or move original pm_runtime_put_sync() to after goto label.
>
> > goto out;
> > + }
> >
> > /* fall through */
> > case UPIU_TRANSACTION_NOP_OUT:
> > --
> > 2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: RE: [EXT] [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error
2020-05-22 8:54 ` dinghao.liu
@ 2020-05-22 14:55 ` Bean Huo (beanhuo)
2020-05-23 10:10 ` dinghao.liu
0 siblings, 1 reply; 8+ messages in thread
From: Bean Huo (beanhuo) @ 2020-05-22 14:55 UTC (permalink / raw)
To: dinghao.liu
Cc: kjlu, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Martin K. Petersen, Bart Van Assche, Can Guo, linux-scsi,
linux-kernel
Hi, Dinghao
> Thank you for your advice! Moving original pm_runtime_put_sync() to after
> "out" label will influence an error path branched from
> ups_bsg_verify_query_size(). So I think changing "goto out" to "break" is a good
> idea. But in this case we may execute an extra
> sg_copy_from_buffer() and an extra kfree() compared with unpatched version.
> Does this matter?
>
What do you mean " unpatched version "?
I see, below goto will bypass sg_copy_from_buffer() and an extra kfree()
In case ufs_bsg_alloc_desc_buffer() fails.
Bean
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: RE: RE: [EXT] [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error
2020-05-22 14:55 ` Bean Huo (beanhuo)
@ 2020-05-23 10:10 ` dinghao.liu
0 siblings, 0 replies; 8+ messages in thread
From: dinghao.liu @ 2020-05-23 10:10 UTC (permalink / raw)
To: Bean Huo (beanhuo)
Cc: kjlu, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Martin K. Petersen, Bart Van Assche, Can Guo, linux-scsi,
linux-kernel
> Hi, Dinghao
>
> > Thank you for your advice! Moving original pm_runtime_put_sync() to after
> > "out" label will influence an error path branched from
> > ups_bsg_verify_query_size(). So I think changing "goto out" to "break" is a good
> > idea. But in this case we may execute an extra
> > sg_copy_from_buffer() and an extra kfree() compared with unpatched version.
> > Does this matter?
> >
> What do you mean " unpatched version "?
>
> I see, below goto will bypass sg_copy_from_buffer() and an extra kfree()
> In case ufs_bsg_alloc_desc_buffer() fails.
>
That's exactly what I want to express. If using "break" is OK I will send
a new patch to fix this problem.
> Bean
>
Regaeds,
Dinghao
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error
2020-05-22 4:59 [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error Dinghao Liu
2020-05-22 8:36 ` [EXT] " Bean Huo (beanhuo)
@ 2020-06-10 2:11 ` Martin K. Petersen
2020-06-10 7:16 ` Avri Altman
2020-06-16 3:46 ` Martin K. Petersen
2 siblings, 1 reply; 8+ messages in thread
From: Martin K. Petersen @ 2020-06-10 2:11 UTC (permalink / raw)
To: Avri Altman
Cc: kjlu, Alim Akhtar, James E.J. Bottomley, Martin K. Petersen,
Bean Huo, Bart Van Assche, Can Guo, linux-scsi, linux-kernel,
Dinghao Liu
Avri: Please review!
> When ufs_bsg_alloc_desc_buffer() returns an error code,
> a pairing runtime PM usage counter decrement is needed
> to keep the counter balanced.
>
> Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
> ---
> drivers/scsi/ufs/ufs_bsg.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
> index 53dd87628cbe..516a7f573942 100644
> --- a/drivers/scsi/ufs/ufs_bsg.c
> +++ b/drivers/scsi/ufs/ufs_bsg.c
> @@ -106,8 +106,10 @@ static int ufs_bsg_request(struct bsg_job *job)
> desc_op = bsg_request->upiu_req.qr.opcode;
> ret = ufs_bsg_alloc_desc_buffer(hba, job, &desc_buff,
> &desc_len, desc_op);
> - if (ret)
> + if (ret) {
> + pm_runtime_put_sync(hba->dev);
> goto out;
> + }
>
> /* fall through */
> case UPIU_TRANSACTION_NOP_OUT:
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error
2020-06-10 2:11 ` Martin K. Petersen
@ 2020-06-10 7:16 ` Avri Altman
0 siblings, 0 replies; 8+ messages in thread
From: Avri Altman @ 2020-06-10 7:16 UTC (permalink / raw)
To: Martin K. Petersen
Cc: kjlu, Alim Akhtar, James E.J. Bottomley, Bean Huo,
Bart Van Assche, Can Guo, linux-scsi, linux-kernel, Dinghao Liu
Hi,
> Avri: Please review!
>
> > When ufs_bsg_alloc_desc_buffer() returns an error code,
> > a pairing runtime PM usage counter decrement is needed
> > to keep the counter balanced.
> >
> > Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
Please add:
Fixes: 74e5e468b664 (scsi: ufs-bsg: Wake the device before sending raw upiu commands)
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Thanks,
Avri
> > ---
> > drivers/scsi/ufs/ufs_bsg.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
> > index 53dd87628cbe..516a7f573942 100644
> > --- a/drivers/scsi/ufs/ufs_bsg.c
> > +++ b/drivers/scsi/ufs/ufs_bsg.c
> > @@ -106,8 +106,10 @@ static int ufs_bsg_request(struct bsg_job *job)
> > desc_op = bsg_request->upiu_req.qr.opcode;
> > ret = ufs_bsg_alloc_desc_buffer(hba, job, &desc_buff,
> > &desc_len, desc_op);
> > - if (ret)
> > + if (ret) {
> > + pm_runtime_put_sync(hba->dev);
> > goto out;
> > + }
> >
> > /* fall through */
> > case UPIU_TRANSACTION_NOP_OUT:
>
> --
> Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error
2020-05-22 4:59 [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error Dinghao Liu
2020-05-22 8:36 ` [EXT] " Bean Huo (beanhuo)
2020-06-10 2:11 ` Martin K. Petersen
@ 2020-06-16 3:46 ` Martin K. Petersen
2 siblings, 0 replies; 8+ messages in thread
From: Martin K. Petersen @ 2020-06-16 3:46 UTC (permalink / raw)
To: Dinghao Liu, kjlu
Cc: Martin K . Petersen, Avri Altman, Can Guo, Bean Huo, linux-scsi,
James E.J. Bottomley, linux-kernel, Bart Van Assche, Alim Akhtar
On Fri, 22 May 2020 12:59:29 +0800, Dinghao Liu wrote:
> When ufs_bsg_alloc_desc_buffer() returns an error code,
> a pairing runtime PM usage counter decrement is needed
> to keep the counter balanced.
Applied to 5.8/scsi-fixes, thanks!
[1/1] scsi: ufs-bsg: Fix runtime PM imbalance on error
https://git.kernel.org/mkp/scsi/c/a1e17eb03e69
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-06-16 3:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-22 4:59 [PATCH] scsi: ufs-bsg: Fix runtime PM imbalance on error Dinghao Liu
2020-05-22 8:36 ` [EXT] " Bean Huo (beanhuo)
2020-05-22 8:54 ` dinghao.liu
2020-05-22 14:55 ` Bean Huo (beanhuo)
2020-05-23 10:10 ` dinghao.liu
2020-06-10 2:11 ` Martin K. Petersen
2020-06-10 7:16 ` Avri Altman
2020-06-16 3:46 ` 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;
as well as URLs for NNTP newsgroup(s).