[PATCH] [v2] media: staging: tegra-vde: fix runtime pm imbalance on error

[PATCH] [v2] media: staging: tegra-vde: fix runtime pm imbalance on error

[Dinghao Liu]

pm_runtime_get_sync() increments the runtime PM usage counter even
the call returns an error code. Thus a pairing decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Dinghao Liu <dinghao.liu-Y5EWUtBUdg4nDS1+zs4M5A@public.gmane.org>
---

Changelog:

v2: - Remove unused label 'unlock'
---
 drivers/staging/media/tegra-vde/vde.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c
index d3e63512a765..3fdf2cd0b99e 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -777,7 +777,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 
 	ret = pm_runtime_get_sync(dev);
 	if (ret < 0)
-		goto unlock;
+		goto put_runtime_pm;
 
 	/*
 	 * We rely on the VDE registers reset value, otherwise VDE
@@ -843,8 +843,6 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 put_runtime_pm:
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
-
-unlock:
 	mutex_unlock(&vde->lock);
 
 release_dpb_frames:
-- 
2.17.1

Re: [PATCH] [v2] media: staging: tegra-vde: fix runtime pm imbalance on error

[Dan Carpenter]

On Thu, May 21, 2020 at 02:27:45PM +0800, Dinghao Liu wrote:
> pm_runtime_get_sync() increments the runtime PM usage counter even
> the call returns an error code. Thus a pairing decrement is needed
> on the error handling path to keep the counter balanced.
> 
> Signed-off-by: Dinghao Liu <dinghao.liu-Y5EWUtBUdg4nDS1+zs4M5A@public.gmane.org>

Let's stop working around the bug in pm_runtime_get_sync() and write
a replacement for it instead.

regards,
dan carpenter

Re: Re: [PATCH] [v2] media: staging: tegra-vde: fix runtime pm imbalance on error

[dinghao.liu-Y5EWUtBUdg4nDS1+zs4M5A]

We need to make sure if pm_runtime_get_sync() is designed with
such behavior before modifying it.  

I received a response from Rafael when I commited a similar patch:
https://lkml.org/lkml/2020/5/20/1100
It seems that this behavior is intentional and needs to be kept.

Regards,
Dinghao

"Dan Carpenter" <dan.carpenter@oracle.com>写道：
> On Thu, May 21, 2020 at 02:27:45PM +0800, Dinghao Liu wrote:
> > pm_runtime_get_sync() increments the runtime PM usage counter even
> > the call returns an error code. Thus a pairing decrement is needed
> > on the error handling path to keep the counter balanced.
> > 
> > Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
> 
> Let's stop working around the bug in pm_runtime_get_sync() and write
> a replacement for it instead.
> 
> regards,
> dan carpenter

Re: Re: [PATCH] [v2] media: staging: tegra-vde: fix runtime pm imbalance on error

[Dan Carpenter]

On Thu, May 21, 2020 at 07:42:56PM +0800, dinghao.liu@zju.edu.cn wrote:
> We need to make sure if pm_runtime_get_sync() is designed with
> such behavior before modifying it.  
> 
> I received a response from Rafael when I commited a similar patch:
> https://lkml.org/lkml/2020/5/20/1100
> It seems that this behavior is intentional and needs to be kept.

Yes.  This is why I have said twice or three times to not change
pm_runtime_get_sync() but instead to write a replacement.

A large percent of the callers are buggy.  The pm_runtime_get_sync() is
a -4 on Rusty's API scale.
http://sweng.the-davies.net/Home/rustys-api-design-manifesto
One could argue that anything above a -4 is really a 2 if you read
the implementation thouroughly enough...

regards,
dan carpenter

Re: Re: Re: [PATCH] [v2] media: staging: tegra-vde: fix runtime pm imbalance on error

[dinghao.liu-Y5EWUtBUdg4nDS1+zs4M5A]

Sorry, I misunderstood your idea before. A new function is 
the best solution for this problem.

Regards,
Dinghao

"Dan Carpenter" <dan.carpenter@oracle.com>写道：
> On Thu, May 21, 2020 at 07:42:56PM +0800, dinghao.liu@zju.edu.cn wrote:
> > We need to make sure if pm_runtime_get_sync() is designed with
> > such behavior before modifying it.  
> > 
> > I received a response from Rafael when I commited a similar patch:
> > https://lkml.org/lkml/2020/5/20/1100
> > It seems that this behavior is intentional and needs to be kept.
> 
> Yes.  This is why I have said twice or three times to not change
> pm_runtime_get_sync() but instead to write a replacement.
> 
> A large percent of the callers are buggy.  The pm_runtime_get_sync() is
> a -4 on Rusty's API scale.
> http://sweng.the-davies.net/Home/rustys-api-design-manifesto
> One could argue that anything above a -4 is really a 2 if you read
> the implementation thouroughly enough...
> 
> regards,
> dan carpenter
>