Re: [tip:perfcounters/core] x86: Add NMI types for kmap_atomic

[Ingo Molnar]

* Peter Zijlstra <a.p.zijlstra@xxxxxxxxx> wrote:

> > I've not been following the background to this,
> 
> We need/want to do a user-space stack walk from IRQ/NMI context. 
> The NMI bit means we cannot simply use __copy_from_user_inatomic() 
> since that will still fault (albeit not page), and the fault 
> return path invokes IRET which will terminate the NMI context.

The goal is to allow 'perf' (see tools/perf/) non-flat 
categorizations like the sample output in the (pending) commit (see 
it attached further below). Here's the kind of output it allows:

 $ perf record -g -m 512 -f -- make -j32 kernel
 $ perf report -s s --syscalls | grep '\[k\]' | grep -v nmi

     4.14%  [k] do_page_fault
     1.20%  [k] sys_write
     1.10%  [k] sys_open
     0.63%  [k] sys_exit_group
     0.48%  [k] smp_apic_timer_interrupt
     0.37%  [k] sys_read
     0.37%  [k] sys_execve
     0.20%  [k] sys_mmap
     0.18%  [k] sys_close
     0.14%  [k] sys_munmap
     0.13%  [k] sys_poll

Note that Oprofile uses the same method of __copy_user_inatomic() in 
arch/x86/oprofile/backtrace.c, but i believe that code is broken - i 
doubt the call-chain support for user-space stacks ever worked in 
oprofile - with perfcounters i can make this method crash under 
load. (we re-enter the NMI which due to IST executes over the exact 
same, still pending NMI frame. Kaboom.)

I saw you being involved with the Oprofile code 3 years ago:

| commit c34d1b4d165c67b966bca4aba026443d7ff161eb
| Author: Hugh Dickins <hugh@xxxxxxxxxxx>
| Date:   Sat Oct 29 18:16:32 2005 -0700
|
|    [PATCH] mm: kill check_user_page_readable

That method of __copy_user_inatomic(), while elegant, is subtly 
wrong in an NMI context. We really must avoid taking faults there.

	Ingo

------------>