From: Yeoreum Yun <yeoreum.yun@arm.com>
To: Sean Anderson <sean.anderson@linux.dev>
Cc: Leo Yan <leo.yan@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Mike Leach <mike.leach@linaro.org>,
Linu Cherian <lcherian@marvell.com>,
James Clark <james.clark@linaro.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] coresight: Fix possible deadlock in coresight_panic_cb
Date: Sat, 13 Sep 2025 05:30:44 +0100 [thread overview]
Message-ID: <aMTzdJRGdTCBZyo1@e129823.arm.com> (raw)
In-Reply-To: <6d833ea6-328f-4743-abfa-fb09b168849e@linux.dev>
Hi,
> > Hi,
> >
> >> Hi Sean,
> >>
> >> On Thu, Sep 11, 2025 at 11:33:15AM -0400, Sean Anderson wrote:
> >> > coresight_panic_cb is called with interrupts disabled during panics.
> >> > However, bus_for_each_dev calls bus_to_subsys which takes
> >> > bus_kset->list_lock without disabling IRQs. This will cause a deadlock
> >> > if a panic occurs while one of the other coresight functions that uses
> >> > bus_for_each_dev is running.
> >>
> >> The decription is a bit misleading. Even when IRQ is disabled, if an
> >> exception happens, a CPU still can be trapped for handling kernel panic.
> >>
> >> > Maintain a separate list of coresight devices to access during a panic.
> >>
> >> Rather than maintaining a separate list and introducing a new spinlock,
> >> I would argue if we can simply register panic notifier in TMC ETR and
> >> ETF drviers (see tmc_panic_sync_etr() and tmc_panic_sync_etf()).
> >>
> >> If there is no dependency between CoreSight modules in panic sync flow,
> >> it is not necessary to maintain list (and lock) for these modules.
>
> Yeah, I was thinking about this as I was preparing v2 of this patch.
>
> > +1 for this.
> > and using the spinlock in the panic_cb doesn't work on PREEMPT_RT side.
>
> What do you mean by this? I am using lockdep and it did not warn about this,
> so I assume that on PREEMPT_RT IRQs remain enabled in this path.
Hmm, I don't believe this.
When you see the panic(), it explicitly disable irq.
and preempt_disabled() before
calling atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
also, atomic_nofier_call_chain() is rcu critical section.
As you know, since the spinlock becomes sleepable lock in PREEMPT_RT
this is problem.
The reason why lockdep doesn't report this problem since it was disabled
before panic notifier chain by calling debug_locks_off();
Thanks.
--
Sincerely,
Yeoreum Yun
next prev parent reply other threads:[~2025-09-13 4:31 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-11 15:33 [PATCH v2] coresight: Fix possible deadlock in coresight_panic_cb Sean Anderson
2025-09-12 9:35 ` Leo Yan
2025-09-12 11:03 ` Yeoreum Yun
2025-09-12 14:38 ` Sean Anderson
2025-09-13 4:30 ` Yeoreum Yun [this message]
2025-09-15 14:32 ` Sean Anderson
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=aMTzdJRGdTCBZyo1@e129823.arm.com \
--to=yeoreum.yun@arm.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=coresight@lists.linaro.org \
--cc=james.clark@linaro.org \
--cc=lcherian@marvell.com \
--cc=leo.yan@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mike.leach@linaro.org \
--cc=sean.anderson@linux.dev \
--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.