Re: [RFC PATCH 2/2] powerpc: nop trap instruction after WARN_ONCE fires
Le 04/10/2022 à 06:31, Nicholas Piggin a écrit : > On Sat Sep 24, 2022 at 2:47 AM AEST, Christophe Leroy wrote: >> >> >> Le 23/09/2022 à 17:41, Nicholas Piggin a écrit : >>> WARN_ONCE and similar are often used in frequently executed code, and >>> should not crash the system. The program check interrupt caused by >>> WARN_ON_ONCE can be a significant overhead even when nothing is being >>> printed. This can cause performance to become unacceptable, having the >>> same effective impact to the user as a BUG_ON(). >>> >>> Avoid this overhead by patching the trap with a nop instruction after a >>> "once" trap fires. Conditional warnings that return a result must have >>> equivalent compare and branch instructions after the trap, so when it is >>> nopped the statement will behave the same way. It's possible the asm >>> goto should be removed entirely and this comparison just done in C now. >> >> You mean, just like PPC32 ? (Since db87a7199229 ("powerpc/bug: Remove >> specific powerpc BUG_ON() and WARN_ON() on PPC32")) >> >> But I'm having hard time with your change. >> >> You change only WARN_ON() >> But WARN_ON_ONCE() calls __WARN_FLAGS() >> And WARN_ONCE() calls WARN() via DO_ONCE_LITE_IF() >> >> So I don't see any ..._ONCE something going with WARN_ON(). >> >> Am I missing something ? > > Hmm, no I must have missed something. I guess it is the EMIT_WARN_ENTRY > in asm which is the main problem I've seen. Although we could remove the > DO_ONCE_LITE_IF code generation from our WARN_ON_ONCE as well if we did > this patching. > Yes, I guess having now the recovery address in the bug table instead of the extable is rather more efficient. Maybe DO_ONCE_LITE could be replaced by DO_ONCE which uses jump_label ? Not sure it is worth a specific patching implementation, is it ? Christophe
Re: [RFC PATCH 2/2] powerpc: nop trap instruction after WARN_ONCE fires
On Sat Sep 24, 2022 at 2:47 AM AEST, Christophe Leroy wrote: > > > Le 23/09/2022 à 17:41, Nicholas Piggin a écrit : > > WARN_ONCE and similar are often used in frequently executed code, and > > should not crash the system. The program check interrupt caused by > > WARN_ON_ONCE can be a significant overhead even when nothing is being > > printed. This can cause performance to become unacceptable, having the > > same effective impact to the user as a BUG_ON(). > > > > Avoid this overhead by patching the trap with a nop instruction after a > > "once" trap fires. Conditional warnings that return a result must have > > equivalent compare and branch instructions after the trap, so when it is > > nopped the statement will behave the same way. It's possible the asm > > goto should be removed entirely and this comparison just done in C now. > > You mean, just like PPC32 ? (Since db87a7199229 ("powerpc/bug: Remove > specific powerpc BUG_ON() and WARN_ON() on PPC32")) > > But I'm having hard time with your change. > > You change only WARN_ON() > But WARN_ON_ONCE() calls __WARN_FLAGS() > And WARN_ONCE() calls WARN() via DO_ONCE_LITE_IF() > > So I don't see any ..._ONCE something going with WARN_ON(). > > Am I missing something ? Hmm, no I must have missed something. I guess it is the EMIT_WARN_ENTRY in asm which is the main problem I've seen. Although we could remove the DO_ONCE_LITE_IF code generation from our WARN_ON_ONCE as well if we did this patching. Thanks, Nick
RE: [RFC PATCH 2/2] powerpc: nop trap instruction after WARN_ONCE fires
From: Nicholas Piggin > Sent: 23 September 2022 16:42 > > WARN_ONCE and similar are often used in frequently executed code, and > should not crash the system. The program check interrupt caused by > WARN_ON_ONCE can be a significant overhead even when nothing is being > printed. This can cause performance to become unacceptable, having the > same effective impact to the user as a BUG_ON(). > > Avoid this overhead by patching the trap with a nop instruction after a > "once" trap fires. Conditional warnings that return a result must have > equivalent compare and branch instructions after the trap, so when it is > nopped the statement will behave the same way. It's possible the asm > goto should be removed entirely and this comparison just done in C now. > > XXX: possibly this should schedule the patching to run in a different > context than the program check. I'm pretty sure WARN_ON_ONCE() is valid everywhere printk() is allowed. In many cases this means you can't call mutex_enter(). So you need a different scheme. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [RFC PATCH 2/2] powerpc: nop trap instruction after WARN_ONCE fires
Le 23/09/2022 à 17:41, Nicholas Piggin a écrit : > WARN_ONCE and similar are often used in frequently executed code, and > should not crash the system. The program check interrupt caused by > WARN_ON_ONCE can be a significant overhead even when nothing is being > printed. This can cause performance to become unacceptable, having the > same effective impact to the user as a BUG_ON(). > > Avoid this overhead by patching the trap with a nop instruction after a > "once" trap fires. Conditional warnings that return a result must have > equivalent compare and branch instructions after the trap, so when it is > nopped the statement will behave the same way. It's possible the asm > goto should be removed entirely and this comparison just done in C now. You mean, just like PPC32 ? (Since db87a7199229 ("powerpc/bug: Remove specific powerpc BUG_ON() and WARN_ON() on PPC32")) But I'm having hard time with your change. You change only WARN_ON() But WARN_ON_ONCE() calls __WARN_FLAGS() And WARN_ONCE() calls WARN() via DO_ONCE_LITE_IF() So I don't see any ..._ONCE something going with WARN_ON(). Am I missing something ? Otherwise, what you want to do is to patch the 'twi' in __WARN_FLAGS() by a non conditional branch to __label_warn_on . Christophe