lkml.org 
[lkml]   [2008]   [Jul]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 0/5] ftrace: do not trace NMI handlers
* Mathieu Desnoyers (mathieu.desnoyers@polymtl.ca) wrote:
> * Peter Zijlstra (peterz@infradead.org) wrote:
> > On Tue, 2008-07-29 at 21:29 -0400, Steven Rostedt wrote:
> > > The dynamic ftrace code modifies code text at run time. Arjan informed
> > > me that there is no safe way to modify code text on an SMP system when
> > > the other CPUs might execute that same code. The reason has to do with
> > > pipeline caches and CPUs might do funny things if the code being pushed
> > > in the pipeline also happens to be modified at that same time. (Arjan
> > > correct me if I'm wrong here).
> >
> > How does the immediate value stuff get around this issue?
> >
>
> By inserting a temporary breakpoint in the code about to be replaced and
> by issuing an IPI to every CPU, issuing a cpuid instruction to
> synchronize the cores, before reverting the breakpoint to the first byte
> of the new instruction. Therefore we are sure that any CPU which could
> have seen the old instruction will flush its pipeline cache (cpuid
> instruction synchronizes the core) before seeing the new instruction.
> Meanwhile, the breakpoint handler executes the original instruction,
> dealing with non-relocatable instructions if necessary by emulating them
> (e.g. by playing with the return IP). We have some constraints to
> follow: every instruction being replaced must stay a valid instruction
> to the CPU, because there could be preemption in the middle of the
> modified area. Immediate values does not change the size of the
> instructions, so we don't run in this problem, but changing
> { 0x90, 0x90, 0x90, 0x90, 0x90 } (nops)
> into
> { 0xe9, 0xXX, 0xXX, 0xXX, 0xXX } (5-bytes jump)
> would be problematic because a preempted IP could point right in the
> middle of those nops. As a general rule, never try to combine smaller
> instructions into a bigger one, except in the case of adding a
> lock-prefix to an instruction : this case insures that the non-lock
> prefixed instruction is still valid after the change has been done. We
> could however run into a nasty non-synchronized atomic instruction use
> in SMP mode if a thread happens to be scheduled out right after the lock
> prefix. Hopefully the alternative code uses the refrigerator... (hrm, it
> doesn't).
>

Actually, alternative.c lock-prefix modification is O.K. for spinlocks
because they execute with preemption off, but not for other atomic
operations which may execute with preemption on.

Mathieu

> All that is documentented thoroughly in arch/x86/kernel/immediate.c
> comments.
>
> Mathieu
>
>
>
> > > We use kstop_machine to put the system into a UP like mode. This prevents
> > > other CPUs from executing code while we modify it. Under stress testing
> > > Ingo discovered that NMIs can cause the system to crash. This was due
> > > to NMIs calling code that is being modified. Some boxes are more prone to
> > > failure than others.
> > >
> > > This series of patches performs two tasks:
> > >
> > > 1) Add notrace to functions called by NMI, or simply remove the tracing
> > > completely from files that are primarily used by NMI.
> > >
> > > 2) Add a warning when code that will be modified is called by an NMI.
> > > This also disables ftraced when it is detected, to prevent the
> > > race with the NMI and code modification from happneing.
> > >
> > > The warning looks something like this:
> > >
> > > --------------- cut here ---------------
> > > WARNING: ftraced code called from NMI context lapic_wd_event+0xd/0x65
> > > Please report this to the ftrace maintainer.
> > > Disabling ftraced. Boot with ftrace_keep_on_nmi to not disable.
> > > Pid: 0, comm: swapper Not tainted 2.6.26-tip #96
> > >
> > > Call Trace:
> > > <NMI> [<ffffffff8021c6d0>] ? lapic_wd_event+0xd/0x65
> > > [<ffffffff8027b9c1>] ftrace_record_ip+0xa3/0x357
> > > [<ffffffff8020c0f4>] mcount_call+0x5/0x31
> > > [<ffffffff8021c6d5>] ? lapic_wd_event+0x12/0x65
> > > [<ffffffff804b90d4>] nmi_watchdog_tick+0x21b/0x230
> > > [<ffffffff804b8487>] default_do_nmi+0x73/0x1e0
> > > [<ffffffff804b8a04>] do_nmi+0x64/0x91
> > > [<ffffffff804b80bf>] nmi+0x7f/0x80
> > > [<ffffffff80212c14>] ? default_idle+0x35/0x4f
> > > <<EOE>> [<ffffffff8020ae42>] cpu_idle+0x8a/0xc9
> > > [<ffffffff804b15a6>] start_secondary+0x172/0x177
> > >
> > > --------------- end cut here ---------------
> > >
> > >
> > > This appears once when it is caught. We are hoping that this will not
> > > appear often, and are running code to catch it as it does.
> > >
> > > -- Steve
> > >
> >
>
> --
> Mathieu Desnoyers
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68


\
 
 \ /
  Last update: 2008-07-30 16:27    [W:0.037 / U:0.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site