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
next prev parent reply other threads:[~2026-07-15 6:58 UTC|newest]
Thread overview: 10+ 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-12 21:17 ` sashiko-bot
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-13 23:17 ` sashiko-bot
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]
2026-07-16 3:07 ` MOHAMED AYMAN
2026-07-16 12:32 ` Sebastian Andrzej Siewior
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 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.