Re: [PATCH v3] coresight: Fix possible deadlock in coresight_panic_cb

[Sean Anderson <sean.anderson@linux.dev>]

On 9/16/25 12:00, Leo Yan wrote:
> On Mon, Sep 15, 2025 at 10:31:24AM -0400, Sean Anderson wrote:
>> On 9/15/25 05:58, Leo Yan wrote:
>> > On Fri, Sep 12, 2025 at 11:13:14AM -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 may cause a deadlock.
>> > 
>> > I would rephrase it to make it clearer for anyone reading it later:
>> > 
>> >   coresight_panic_cb() is called during panics, which can preempt a flow
>> >   that triggers exceptions (such as data or instruction aborts).
>> 
>> I don't see what exceptions have to do with it. You can also panic
>> during a regular interrupt.
> 
> The commit mentioned "without disabling IRQs" gives the impression that
> the deadlock is caused by IRQ-unsafe locking, which might mislead into
> thinking why the issue cannot be fixed with IRQ-safe locking.
> 
> Regardless of whether IRQs are disabled, and regardless of the context
> (interrupt, bottom-half, or normal thread), the conditions for the
> deadlock are only about:
> 
>   (a) The bus lock has been acquired;
>   (b) A panic is triggered to try to acquire the same lock.
> 
> [...]
> 
>> > When I review this patch, I recognize we can consolidate panic notifier
>> > in coresight-tmc-core.c, so we don't need to distribute the changes
>> > into ETF and ETR drivers (sorry if I misled you in my previous reply).
>> 
>> And this kind of thing is why I went with the straightforward fix
>> initially. I do not want to bikeshed the extent that this gets removed.
>> IMO the whole "panic ops" stuff should be done directly with the panic
>> notifier, hence this patch. If you do not agree with that, then ack v2
>> and send a follow up of your own to fix it how you see fit.
> 
> I would fix it in one go.
> 
> I agree with you that "the whole panic ops stuff should be done directly
> with the panic". The only difference between us is that I would keep the
> `panic_ops` callback. To me, this encapsulates panic callbacks into
> different modules, to make the code more general.
> 
> Could you check if the drafted patch below looks good to you? If so, I

As stated above I disagree with a half-hearted removal. If you want to do that,
then I will resend v2 done with an rcu list and you can make your own follow-up.

--Sean