From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: MOHAMED AYMAN <mohamedaymanworkspace@gmail.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>,
Mike Leach <mike.leach@arm.com>,
James Clark <james.clark@linaro.org>, Leo Yan <leo.yan@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Clark Williams <clrkwllms@kernel.org>,
Steven Rostedt <rostedt@goodmis.org>,
"moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS"
<coresight@lists.linaro.org>,
"moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS"
<linux-arm-kernel@lists.infradead.org>,
open list <linux-kernel@vger.kernel.org>,
"open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT"
<linux-rt-devel@lists.linux.dev>
Subject: Re: [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()
Date: Wed, 15 Jul 2026 08:58:32 +0200 [thread overview]
Message-ID: <20260715065832.qI7c2Px5@linutronix.de> (raw)
In-Reply-To: <CABWZVho1=oJHU6U16Kc-Nh1bQbn07oz7PPkM9Diy=JS0beGjFQ@mail.gmail.com>
On 2026-07-14 22:42:12 [+0300], MOHAMED AYMAN wrote:
> Hi Sebastian,
Hi Mohamed,
> Thank you for the review and the feedback.
>
> Regarding the commit message:
> You are completely right. I will update the wording in the v4 patch to
> explicitly state that it "uses a spinlock_t for locking which becomes
> a sleeping lock on PREEMPT_RT" instead of calling it an rt_mutex
> directly.
>
> Regarding the module-ref counter:
> We don't explicitly bump the module reference count for each new
> device. Instead, we rely on `destroy_workqueue(coresight_wq)` inside
> `coresight_exit()`. `destroy_workqueue()` synchronously drains all
> pending work items before returning, which ensures no deferred puts
> are executed after the module is unmapped.
But what stops the module unload before all devices are released?
> Regarding deferring coresight_device_release() vs put_device():
> My initial v1 patch did exactly what you suggested, it only deferred
> the body of `coresight_device_release()`. However, it was pointed out
> that `put_device()` synchronously recurses into `kobject_cleanup()`,
> which invokes the child's release function and immediately afterwards
> calls `kobject_put(parent)`.
>
> If we only defer the child's release callback, the `put_device()` call
> itself will still execute in the atomic CPU_PM notifier context. If
> the parent device's release path acquires any sleeping locks, we will
> still hit a "scheduling while atomic" panic. Deferring `put_device()`
> entirely protects against this parent cascade.
>
> If it is strictly guaranteed that Coresight parent devices (liike
> AMBA) will never sleep during their release paths, I can happily
> revert to the simpler approach of just deferring
> `coresight_device_release()`.
>
> Would you prefer I revert to deferring just the release function, or
> keep the current architecture to safeguard the parent put?
Well deferring as I suggested if the kobj goes away is a not good.
That I part I didn't get: You have the call chain:
| coresight_cpu_pm_notify() (IRQs off)
| -> coresight_put_percpu_source_ref()
| -> put_device()
| -> coresight_device_release()
| -> free_percpu()
What you skipped is coresight_cpu_get_active_path() and this one has a
get and a put. Your put has a irqlock on coresight_dev_lock which I am
not sure you need. But more importantly, why is the reference going back
to 0? There would have to be a coresight_clear_percpu_source() in
between, right?
If you could avoid grabbing a reference in the coresight_cpu_pm_notify()
path then we wouldn't have that problem or is there more to it?
> Best regards,,
> Mohamed Ayman
Sebastian
prev parent reply other threads:[~2026-07-15 6:58 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-12 21:04 [PATCH] coresight: Fix scheduling while atomic in coresight_device_release() Mohamed Ayman
2026-07-13 3:07 ` Jie Gan
2026-07-13 23:00 ` [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref() Mohamed Ayman
2026-07-14 10:42 ` Sebastian Andrzej Siewior
2026-07-14 19:42 ` MOHAMED AYMAN
2026-07-15 6:58 ` Sebastian Andrzej Siewior [this message]
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=20260715065832.qI7c2Px5@linutronix.de \
--to=bigeasy@linutronix.de \
--cc=alexander.shishkin@linux.intel.com \
--cc=clrkwllms@kernel.org \
--cc=coresight@lists.linaro.org \
--cc=james.clark@linaro.org \
--cc=leo.yan@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rt-devel@lists.linux.dev \
--cc=mike.leach@arm.com \
--cc=mohamedaymanworkspace@gmail.com \
--cc=rostedt@goodmis.org \
--cc=suzuki.poulose@arm.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