Re: [PATCH v3 1/6] watchdog: starfive: balance PM refcount when start operation fails

[Guenter Roeck]

On Fri, Jun 05, 2026 at 01:19:37PM -0400, William Theesfeld wrote:
> After pm_runtime_resume_and_get() succeeds in starfive_wdt_pm_start(),
> the runtime PM usage counter is incremented.  If starfive_wdt_start()
> subsequently fails, the function returns the error without releasing
> that reference, leaking the usage counter.
> 
> Call pm_runtime_put_sync() on the start failure path so the counter
> is balanced.
> 
> This also folds in the v1 conversion of pm_runtime_get_sync() to
> pm_runtime_resume_and_get(), which was the original purpose of this
> patch; both fixes together close the refcount-leak window in this
> function as flagged by the maintainer on the v1 thread.

That description doesn't really match the code. The purpose of this patch
is to fix a refcount leak in the upstream code, not to fix the refcount
leak introduced by the first version of this patch. That the first version
of the patch introduced a new refcount leak is irrelevant (and actually
confusing).

Guenter

> 
> Signed-off-by: William Theesfeld <william@xxxxxxxxxxxxx>
> ---
>  drivers/watchdog/starfive-wdt.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c
> index af55adc4a..ed8c5711a 100644
> --- a/drivers/watchdog/starfive-wdt.c
> +++ b/drivers/watchdog/starfive-wdt.c
> @@ -371,12 +371,16 @@ static void starfive_wdt_stop(struct starfive_wdt *wdt)
>  static int starfive_wdt_pm_start(struct watchdog_device *wdd)
>  {
>  	struct starfive_wdt *wdt = watchdog_get_drvdata(wdd);
> -	int ret = pm_runtime_get_sync(wdd->parent);
> +	int ret = pm_runtime_resume_and_get(wdd->parent);
>  
>  	if (ret < 0)
>  		return ret;
>  
> -	return starfive_wdt_start(wdt);
> +	ret = starfive_wdt_start(wdt);
> +	if (ret)
> +		pm_runtime_put_sync(wdd->parent);
> +
> +	return ret;
>  }
>  
>  static int starfive_wdt_pm_stop(struct watchdog_device *wdd)