On Mon, May 25, 2020 at 11:31 AM Ulf Hansson <ulf.hans...@linaro.org> wrote: > > On Fri, 22 May 2020 at 20:39, Rafael J. Wysocki <raf...@kernel.org> wrote: > > > > On Fri, May 22, 2020 at 7:19 AM Marek Szyprowski > > <m.szyprow...@samsung.com> wrote: > > > > > > Hi Rafael, > > > > > > On 21.05.2020 19:08, Rafael J. Wysocki wrote: > > > > From: Rafael J. Wysocki <rafael.j.wyso...@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+ <sta...@vger.kernel.org> # 4.15+ > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wyso...@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. > > I agree. > > In hindsight we should have dropped the usage count in > pm_runtime_get_sync(), when it fails. However, that's too late, > especially since there are many cases having no error handling at all > - and in those cases, that would mean the subsequent call to > pm_runtime_put() can mess up the usage count (if pm_runtime_get_sync() > failed and has already dropped the count). > > So, feel free to add: > > Reviewed-by: Ulf Hansson <ulf.hans...@linaro.org>
Thanks! Given the lack of other comments, I'm applying this patch as 5.8 material.
