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
next 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.