The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: "Manush Prajwal" <manushprajwal555@gmail.com>
To: jassisinghbrar@gmail.com, matthias.bgg@gmail.com,
	angelogioacchino.delregno@collabora.com
Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, Markus.Elfring@web.de
Subject: [PATCH v4] mailbox: mtk-cmdq: fix runtime PM usage counter leak in cmdq_mbox_flush()
Date: 25 Aug 2026 23:04:22 +0530	[thread overview]
Message-ID: <6a8dd21e.b12cec6a.f9ff0.b637@mx.google.com> (raw)

cmdq_mbox_flush() calls pm_runtime_get_sync() and returns its error
code directly on failure. Per Documentation/power/runtime_pm.rst,
pm_runtime_get_sync() does not drop the usage counter on error, so the
caller is expected to release the reference itself. The early return
here skipped that, leaking a runtime PM usage count on cmdq->mbox.dev
on every failed resume. v2 fixed this by switching to
pm_runtime_resume_and_get(), which performs the get-and-put
internally on failure.

Markus Elfring additionally pointed out that the "out:" and "wait:"
labels duplicate the same pm_runtime_mark_last_busy() +
pm_runtime_put_autosuspend() + return sequence. That duplication
hides a second, separate leak: the "wait:" path's timeout branch
(readl_poll_timeout_atomic() failing) returns -EFAULT directly,
without ever calling pm_runtime_mark_last_busy() /
pm_runtime_put_autosuspend(), so the pm_runtime reference taken at
function entry is leaked on every polling timeout too.

Fix both by merging the two exit sequences into one common "out_pm:"
label reached by both "out:" and "wait:", using "ret" to carry the
return value. "ret" is explicitly reset to 0 on both success paths
before reaching "out_pm:", since pm_runtime_resume_and_get() can
return a positive value (e.g. 1) on success, not just 0, and that
stale value must not leak into the function's return value.

Verified the merged exit path with a standalone model of the four
possible outcomes (initial get failure, "out:" success, "wait:"
success, "wait:" timeout): the runtime PM refcount returns to its
starting value and the return code is correct on every path.

Signed-off-by: Manush Prajwal <manushprajwal555@gmail.com>
---
v4:
- Add parentheses to the function name in the summary phrase
  (cmdq_mbox_flush -> cmdq_mbox_flush()), per Markus Elfring's review.

- Reformat this changelog so each version identifier stands on its
  own line and blocks are separated by blank lines, per Markus
  Elfring's review.

- No functional change from v3. Still no Fixes tag: I don't have a
  git history for this file available in my current environment to
  identify the commit that introduced this exit path with confidence,
  and would rather leave the tag off than guess a SHA. Still happy to
  add it in a v5 if Markus or anyone else on Cc can point at the right
  commit.

v3:
- Fix the pm_runtime leak on the "wait:" timeout path found while
  addressing Markus Elfring's duplicate-code comment, by merging the
  "out:" and "wait:" exit sequences into a single "out_pm:" label.

- Add Matthias Brugger and AngeloGioacchino Del Regno plus the
  linux-mediatek and linux-arm-kernel lists to Cc, per MAINTAINERS'
  "ARM/Mediatek SoC support" entry (matches any drivers/mailbox/mtk-*.c
  filename), per Markus Elfring's review.

v2:
- Use pm_runtime_resume_and_get() instead of pm_runtime_get_sync()
  plus a manual pm_runtime_put_noidle() on the error path, per Markus
  Elfring's review.

 drivers/mailbox/mtk-cmdq-mailbox.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index e523c84b4..d4e5f6a7b 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -565,7 +565,7 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
        int ret;

-       ret = pm_runtime_get_sync(cmdq->mbox.dev);
+       ret = pm_runtime_resume_and_get(cmdq->mbox.dev);
        if (ret < 0)
                return ret;

@@ -590,22 +590,21 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
        cmdq_thread_disable(cmdq, thread);

 out:
        spin_unlock_irqrestore(&thread->chan->lock, flags);
-       pm_runtime_mark_last_busy(cmdq->mbox.dev);
-       pm_runtime_put_autosuspend(cmdq->mbox.dev);
-
-       return 0;
+       ret = 0;
+       goto out_pm;

 wait:
        cmdq_thread_resume(thread);
        spin_unlock_irqrestore(&thread->chan->lock, flags);
+       ret = 0;
        if (readl_poll_timeout_atomic(thread->base + CMDQ_THR_ENABLE_TASK,
                                      enable, enable == 0, 1, timeout)) {
                dev_err(cmdq->mbox.dev, "Fail to wait GCE thread 0x%x done\n",
                        (u32)(thread->base - cmdq->base));
-
-               return -EFAULT;
+               ret = -EFAULT;
        }
+
+out_pm:
        pm_runtime_mark_last_busy(cmdq->mbox.dev);
        pm_runtime_put_autosuspend(cmdq->mbox.dev);
-       return 0;
+       return ret;
 }
--
2.46.2.windows.1


             reply	other threads:[~2026-08-25 17:34 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-25 17:34 Manush Prajwal [this message]
2026-08-25 18:27 ` [PATCH v4] mailbox: mtk-cmdq: fix runtime PM usage counter leak in cmdq_mbox_flush() Markus Elfring

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=6a8dd21e.b12cec6a.f9ff0.b637@mx.google.com \
    --to=manushprajwal555@gmail.com \
    --cc=Markus.Elfring@web.de \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=jassisinghbrar@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.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