Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path

2022-05-11 Thread Petr Mladek
On Tue 2022-05-10 21:46:38, John Ogness wrote: > On 2022-05-10, Steven Rostedt wrote: > >> As already mentioned in the other reply, panic() sometimes stops the > >> other CPUs using NMI, for example, see kdump_nmi_shootdown_cpus(). > >> > >> Another situation is when the CPU using the lock ends i

Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path

2022-05-10 Thread John Ogness
On 2022-05-10, Steven Rostedt wrote: >> As already mentioned in the other reply, panic() sometimes stops the >> other CPUs using NMI, for example, see kdump_nmi_shootdown_cpus(). >> >> Another situation is when the CPU using the lock ends in some >> infinite loop because something went wrong. The

Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path

2022-05-10 Thread Steven Rostedt
On Tue, 10 May 2022 13:38:39 +0200 Petr Mladek wrote: > As already mentioned in the other reply, panic() sometimes stops > the other CPUs using NMI, for example, see kdump_nmi_shootdown_cpus(). > > Another situation is when the CPU using the lock ends in some > infinite loop because something we

Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path

2022-05-10 Thread Guilherme G. Piccoli
On 10/05/2022 08:38, Petr Mladek wrote: > [...] > I see two more alternative solutions: > > 1st variant is a trick already used in console write() callbacks. > They do trylock() when oops_in_progress is set. They remember > the result to prevent double unlock when printing Oops messages and > the

Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path

2022-05-10 Thread Petr Mladek
On Tue 2022-05-03 16:12:09, Guilherme G. Piccoli wrote: > On 03/05/2022 15:03, Evan Green wrote: > > [...] > > gsmi_shutdown_reason() is a common function called in other scenarios > > as well, like reboot and thermal trip, where it may still make sense > > to wait to acquire a spinlock. Maybe we s

Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path

2022-05-04 Thread Guilherme G. Piccoli
On 03/05/2022 18:56, Evan Green wrote: > Hi Guilherme, > [...] >> Do you agree with that, or prefer really a parameter in >> gsmi_shutdown_reason() ? I'll follow your choice =) > > I'm fine with either, thanks for the link. Mostly I want to make sure > other paths to gsmi_shutdown_reason() aren't

Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path

2022-05-03 Thread Evan Green
On Wed, Apr 27, 2022 at 3:51 PM Guilherme G. Piccoli wrote: > > Currently the gsmi driver registers a panic notifier as well as > reboot and die notifiers. The callbacks registered are called in > atomic and very limited context - for instance, panic disables > preemption, local IRQs and all other

Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path

2022-05-03 Thread Evan Green
Hi Guilherme, On Tue, May 3, 2022 at 12:12 PM Guilherme G. Piccoli wrote: > > On 03/05/2022 15:03, Evan Green wrote: > > [...] > > gsmi_shutdown_reason() is a common function called in other scenarios > > as well, like reboot and thermal trip, where it may still make sense > > to wait to acquire

Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path

2022-05-03 Thread Guilherme G. Piccoli
On 03/05/2022 15:03, Evan Green wrote: > [...] > gsmi_shutdown_reason() is a common function called in other scenarios > as well, like reboot and thermal trip, where it may still make sense > to wait to acquire a spinlock. Maybe we should add a parameter to > gsmi_shutdown_reason() so that you can

[PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path

2022-04-27 Thread Guilherme G. Piccoli
Currently the gsmi driver registers a panic notifier as well as reboot and die notifiers. The callbacks registered are called in atomic and very limited context - for instance, panic disables preemption, local IRQs and all other CPUs that aren't running the current panic function. With that said,