| Message ID | 20200521062746.6656-1-dinghao.liu@zju.edu.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] media: staging: tegra-vde: fix runtime pm imbalance on error | expand |
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
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
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
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 >
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:
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> --- Changelog: v2: - Remove unused label 'unlock' --- drivers/staging/media/tegra-vde/vde.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)