Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Fri, 22 May 2020 20:39:38 +0200 | Subject | Re: [PATCH] PM: runtime: clk: Fix clk_pm_runtime_get() error path |
| |
On Fri, May 22, 2020 at 7:19 AM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi Rafael, > > On 21.05.2020 19:08, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > clk_pm_runtime_get() assumes that the PM-runtime usage counter will > > be dropped by pm_runtime_get_sync() on errors, which is not the case, > > so PM-runtime references to devices acquired by the former are leaked > > on errors returned by the latter. > > > > Fix this by modifying clk_pm_runtime_get() to drop the reference if > > pm_runtime_get_sync() returns an error. > > > > Fixes: 9a34b45397e5 clk: Add support for runtime PM > > Cc: 4.15+ <stable@vger.kernel.org> # 4.15+ > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Frankly, I would rather fix the runtime_get_sync() instead of fixing the > return path everywhere in the kernel. The current behavior of the > pm_runtime_get_sync() is completely counter-intuitive then. I bet that > in the 99% of the places where it is being called assume that no special > fixup is needed in case of failure. This is one of the most common > runtime PM related function and it is really a common pattern in the > drivers to call: > > pm_runtime_get_sync() > > do something with the hardware > > pm_runtime_put() > > Do you really want to fix the error paths of the all such calls?
No, I don't, and that's why I'm proposing this patch.
The caller that does what you said above is OK now and if the behavior of pm_runtime_get_sync() changed, that caller would need to be updated.
OTOH, a caller that fails to drop the reference on an error returned by pm_runtime_get_sync() is buggy (and has ever been so).
I'd rather update the buggy callers than the ones that are OK.
Thanks!
> > > > --- > > drivers/clk/clk.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > Index: linux-pm/drivers/clk/clk.c > > =================================================================== > > --- linux-pm.orig/drivers/clk/clk.c > > +++ linux-pm/drivers/clk/clk.c > > @@ -114,7 +114,11 @@ static int clk_pm_runtime_get(struct clk > > return 0; > > > > ret = pm_runtime_get_sync(core->dev); > > - return ret < 0 ? ret : 0; > > + if (ret < 0) { > > + pm_runtime_put_noidle(core->dev); > > + return ret; > > + } > > + return 0; > > } > > > > static void clk_pm_runtime_put(struct clk_core *core) > > > > > > > > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland >
| |