mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: William Theesfeld <william@theesfeld.net>
To: Xingyu Wu <xingyu.wu@starfivetech.com>
Cc: Ziv Xu <ziv.xu@starfivetech.com>,
	Wim Van Sebroeck <wim@linux-watchdog.org>,
	Guenter Roeck <linux@roeck-us.net>,
	linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v3 3/6] watchdog: starfive: balance PM refcount and disable in probe error paths
Date: Fri,  5 Jun 2026 13:19:39 -0400	[thread overview]
Message-ID: <bc680dec6d45a31a29db46a66968d054e22b19aa.1780666366.git.william@theesfeld.net> (raw)
In-Reply-To: <cover.1780666366.git.william@theesfeld.net>

The probe path takes a runtime PM reference via
pm_runtime_resume_and_get() (or enables the clocks directly when
runtime PM is unavailable), but several error paths after that point
do not release that reference before returning, and the two earliest
error paths return without calling pm_runtime_disable() at all even
though pm_runtime_enable() has already run.

Restructure the error handling into three labels so every failure
path balances exactly the resources it has acquired:

  err_pm_disable:     pm_runtime_enable() ran but no clock/refcount
                      was taken yet (resume_and_get / enable_clock
                      failed).
  err_put_pm:         clock or PM refcount is held; release it, then
                      fall through to disable runtime PM.
  err_unregister_wdt: watchdog_register_device() succeeded and the
                      success-path pm_runtime_put_sync() returned an
                      error.  The put has already decremented the
                      counter, so this path jumps directly to
                      err_pm_disable rather than falling through to
                      err_put_pm; otherwise the counter would be
                      decremented a second time and underflow.

Update the in-function goto targets to use these labels and remove
the early "return ret;" paths so pm_runtime_disable() is always run
once pm_runtime_enable() has been called.

Signed-off-by: William Theesfeld <william@theesfeld.net>
---
 drivers/watchdog/starfive-wdt.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c
index e047f52b0..856e55f04 100644
--- a/drivers/watchdog/starfive-wdt.c
+++ b/drivers/watchdog/starfive-wdt.c
@@ -460,17 +460,17 @@ static int starfive_wdt_probe(struct platform_device *pdev)
 	if (pm_runtime_enabled(&pdev->dev)) {
 		ret = pm_runtime_resume_and_get(&pdev->dev);
 		if (ret < 0)
-			return ret;
+			goto err_pm_disable;
 	} else {
 		/* runtime PM is disabled but clocks need to be enabled */
 		ret = starfive_wdt_enable_clock(wdt);
 		if (ret)
-			return ret;
+			goto err_pm_disable;
 	}
 
 	ret = starfive_wdt_reset_init(&pdev->dev);
 	if (ret)
-		goto err_exit;
+		goto err_put_pm;
 
 	watchdog_set_drvdata(&wdt->wdd, wdt);
 	wdt->wdd.info = &starfive_wdt_info;
@@ -482,7 +482,7 @@ static int starfive_wdt_probe(struct platform_device *pdev)
 	if (!wdt->freq) {
 		dev_err(&pdev->dev, "get clock rate failed.\n");
 		ret = -EINVAL;
-		goto err_exit;
+		goto err_put_pm;
 	}
 
 	wdt->wdd.min_timeout = 1;
@@ -498,7 +498,7 @@ static int starfive_wdt_probe(struct platform_device *pdev)
 	if (early_enable) {
 		ret = starfive_wdt_start(wdt);
 		if (ret)
-			goto err_exit;
+			goto err_put_pm;
 		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
 	} else {
 		starfive_wdt_stop(wdt);
@@ -506,7 +506,7 @@ static int starfive_wdt_probe(struct platform_device *pdev)
 
 	ret = watchdog_register_device(&wdt->wdd);
 	if (ret)
-		goto err_exit;
+		goto err_put_pm;
 
 	if (!early_enable) {
 		if (pm_runtime_enabled(&pdev->dev)) {
@@ -520,8 +520,20 @@ static int starfive_wdt_probe(struct platform_device *pdev)
 
 err_unregister_wdt:
 	watchdog_unregister_device(&wdt->wdd);
-err_exit:
-	starfive_wdt_disable_clock(wdt);
+	/*
+	 * The only path into err_unregister_wdt is the post-register
+	 * pm_runtime_put_sync() that returned an error.  That call already
+	 * decremented the runtime PM usage counter, so falling through to
+	 * err_put_pm would put again and underflow the counter.  Jump
+	 * straight to err_pm_disable.
+	 */
+	goto err_pm_disable;
+err_put_pm:
+	if (pm_runtime_enabled(&pdev->dev))
+		pm_runtime_put_sync(&pdev->dev);
+	else
+		starfive_wdt_disable_clock(wdt);
+err_pm_disable:
 	pm_runtime_disable(&pdev->dev);
 
 	return ret;
-- 
2.54.0


  parent reply	other threads:[~2026-06-05 17:19 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-05 12:23 [PATCH] watchdog: starfive: use pm_runtime_resume_and_get() to fix refcount leak William Theesfeld
2026-06-05 13:11 ` [PATCH v2 0/6] watchdog: starfive: runtime PM cleanup William Theesfeld
2026-06-05 13:11   ` [PATCH v2 1/6] watchdog: starfive: balance PM refcount when start operation fails William Theesfeld
2026-06-05 13:11   ` [PATCH v2 2/6] watchdog: starfive: treat pm_runtime_put_sync() positive return as success William Theesfeld
2026-06-05 13:11   ` [PATCH v2 3/6] watchdog: starfive: balance PM refcount and disable in probe error paths William Theesfeld
2026-06-05 13:11   ` [PATCH v2 4/6] watchdog: starfive: guard system suspend/resume hardware access William Theesfeld
2026-06-05 13:11   ` [PATCH v2 5/6] watchdog: starfive: avoid PM refcount underflow in shutdown William Theesfeld
2026-06-05 13:11   ` [PATCH v2 6/6] watchdog: starfive: release early_enable PM refcount on remove William Theesfeld
2026-06-05 17:19   ` [PATCH v3 0/6] watchdog: starfive: runtime PM cleanup William Theesfeld
2026-06-05 17:19     ` [PATCH v3 1/6] watchdog: starfive: balance PM refcount when start operation fails William Theesfeld
2026-06-08 19:39       ` Guenter Roeck
2026-06-05 17:19     ` [PATCH v3 2/6] watchdog: starfive: treat pm_runtime_put_sync() positive return as success William Theesfeld
2026-06-05 17:19     ` William Theesfeld [this message]
2026-06-05 17:19     ` [PATCH v3 4/6] watchdog: starfive: guard system suspend/resume hardware access William Theesfeld
2026-06-05 17:19     ` [PATCH v3 5/6] watchdog: starfive: avoid PM refcount underflow in shutdown William Theesfeld
2026-06-05 17:19     ` [PATCH v3 6/6] watchdog: starfive: release early_enable PM refcount on remove William Theesfeld
2026-06-08 20:46     ` [PATCH v3 0/6] watchdog: starfive: runtime PM cleanup Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bc680dec6d45a31a29db46a66968d054e22b19aa.1780666366.git.william@theesfeld.net \
    --to=william@theesfeld.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=wim@linux-watchdog.org \
    --cc=xingyu.wu@starfivetech.com \
    --cc=ziv.xu@starfivetech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®