On Wed, 13 May 2020 18:57:25 -0400 (EDT) Mathieu Desnoyers <[email protected]> wrote:
> ----- On May 5, 2020, at 9:16 AM, Thomas Gleixner [email protected] wrote: > > > This is completely overengineered and definitely not an interface which > > should be made available to anything else than this particular MCE case. > > This patch introduces a significant change under the radar (not explained > in the changelog): it turns preempt_enable_no_resched() into preempt_enable(). > > Why, and why was it a no_resched() in the first place ? Was it for performance > or correctness reasons ? I believe the reason for no_resched, is because it was within a local_irq_disable() section, which means it couldn't schedule anyway. -- Steve > > --- a/arch/x86/kernel/cpu/mce/core.c > > +++ b/arch/x86/kernel/cpu/mce/core.c > > @@ -1352,13 +1352,15 @@ void notrace do_machine_check(struct pt_ > > > > /* Fault was in user mode and we need to take some action */ > > if ((m.cs & 3) == 3) { > > - ist_begin_non_atomic(regs); > > + /* If this triggers there is no way to recover. > > Die hard. */ > > + BUG_ON(!on_thread_stack() || !user_mode(regs)); > > local_irq_enable(); > > + preempt_enable(); > > > > if (kill_it || do_memory_failure(&m)) > > force_sig(SIGBUS); > > + preempt_disable(); > > local_irq_disable(); > > - ist_end_non_atomic(); > > } else { > > if (!fixup_exception(regs, X86_TRAP_MC, error_code, > > -void ist_begin_non_atomic(struct pt_regs *regs) > > -{ > > - BUG_ON(!user_mode(regs)); > > - > > - /* > > - * Sanity check: we need to be on the normal thread > > stack. This > > - * will catch asm bugs and any attempt to use > > ist_preempt_enable > > - * from double_fault. > > - */ > > - BUG_ON(!on_thread_stack()); > > - > > - preempt_enable_no_resched(); > > -} > > - > > -/** > > - * ist_end_non_atomic() - begin a non-atomic section in an IST > > exception > > - * > > - * Ends a non-atomic section started with ist_begin_non_atomic(). > > - */ > > -void ist_end_non_atomic(void) > > -{ > > - preempt_disable(); > > -} > > - > > int is_valid_bugaddr(unsigned long addr) > > { > > unsigned short ud; >
