| Message ID | 20200520133432.19738-1-dinghao.liu@zju.edu.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Input: omap-keypad - fix runtime pm imbalance on error | expand |
Hi Dinghao, On Wed, May 20, 2020 at 6:35 AM Dinghao Liu <dinghao.liu@zju.edu.cn> 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. This is a very surprising behavior and I wonder if this should be fixed in the PM core (or the required cleanup steps need to be called out in the function description). I also see that a few drivers that handle this situation correctly (?) call pm_runtime_put_noidle() instead of pm_runtime_put_sync() in the error path. Rafael, do you have any guidance here? Thanks.
On Wed, May 20, 2020 at 7:02 PM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > Hi Dinghao, > > On Wed, May 20, 2020 at 6:35 AM Dinghao Liu <dinghao.liu@zju.edu.cn> 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. > > This is a very surprising behavior and I wonder if this should be > fixed in the PM core (or the required cleanup steps need to be called > out in the function description). It has been like that forever and it's intentional, because it allows certain pieces of code to do things like pm_runtime_get_sync(dev); /* do something regardless of whether or not PM-runtime is enabled for dev */ pm_runtime_put(dev); So I wouldn't really call it surprising. > I also see that a few drivers that > handle this situation correctly (?) call pm_runtime_put_noidle() > instead of pm_runtime_put_sync() in the error path. > > Rafael, do you have any guidance here? Feel free to improve the kerneldoc comment of __pm_runtime_resume(), although it is clear enough to me, and fix the callers that leak the refcount.
Fixing this in the PM core will influence all callers of pm_runtime_get_sync(). Therefore I think the better solution is to fix its misused callers. Regards, Dinghao "Dmitry Torokhov" <dmitry.torokhov@gmail.com>写道： > Hi Dinghao, > > On Wed, May 20, 2020 at 6:35 AM Dinghao Liu <dinghao.liu@zju.edu.cn> 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. > > This is a very surprising behavior and I wonder if this should be > fixed in the PM core (or the required cleanup steps need to be called > out in the function description). I also see that a few drivers that > handle this situation correctly (?) call pm_runtime_put_noidle() > instead of pm_runtime_put_sync() in the error path. > > Rafael, do you have any guidance here? > > Thanks. > > -- > Dmitry
diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c index 94c94d7f5155..d13d81c796d2 100644 --- a/drivers/input/keyboard/omap4-keypad.c +++ b/drivers/input/keyboard/omap4-keypad.c @@ -371,8 +371,8 @@ static int omap4_keypad_probe(struct platform_device *pdev) err_free_input: input_free_device(input_dev); err_pm_put_sync: - pm_runtime_put_sync(&pdev->dev); err_unmap: + pm_runtime_put_sync(&pdev->dev); iounmap(keypad_data->base); err_release_mem: release_mem_region(res->start, resource_size(res));
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> --- drivers/input/keyboard/omap4-keypad.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)