Messages in this thread | | | Date | Wed, 11 May 2022 13:13:20 +0200 | From | Petr Mladek <> | Subject | Re: [PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path |
| |
On Tue 2022-05-10 21:46:38, John Ogness wrote: > On 2022-05-10, Steven Rostedt <rostedt@goodmis.org> 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 system is in > >> an unpredictable state during panic(). > >> > >> I am not sure if this is possible with the code under gsmi_dev.lock > >> but such things really happen during panic() in other subsystems. > >> Using trylock in the panic() code path is a good practice. > > > > I believe that Peter Zijlstra had a special spin lock for NMIs or > > early printk, where it would not block if the lock was held on the > > same CPU. That is, if an NMI happened and paniced while this lock was > > held on the same CPU, it would not deadlock. But it would block if the > > lock was held on another CPU. > > Yes. And starting with 5.19 it will be carrying the name that _you_ came > up with (cpu_sync): > > printk_cpu_sync_get_irqsave() > printk_cpu_sync_put_irqrestore()
There is a risk that this lock might become a big kernel lock.
This special lock would need to be used even during normal system operation. It does not make sense to suddenly start using another lock during panic.
So I think that we should think twice before using it. I would prefer using trylock of the original lock when possible during panic.
It is possible that I miss something.
Best Regards, Petr
| |