Messages in this thread | | | Subject | Re: [RFC PATCH -tip ] [BUGFIX] x86: Remove preempt disabling from kprobes | From | Steven Rostedt <> | Date | Fri, 01 Jul 2011 09:43:44 -0400 |
| |
On Fri, 2011-07-01 at 22:14 +0900, Masami Hiramatsu wrote: > Steven Rostedt reported that putting kprobe on the instruction which > loads preempt_count causes the wrong result, as below. > > Kprobes requires preemption to be disabled as it single steps the code > it replaced with a breakpoint. But because the code that is single > stepped could be reading the preempt count, the kprobe disabling of the > preempt count can cause the wrong value to end up as a result. Here's an > example: > > If we add a kprobe on a inc_preempt_count() call: > > [ preempt_count = 0 ] > > ld preempt_count, %eax <<--- trap > > <trap> > preempt_disable(); > [ preempt_count = 1] > setup_singlestep(); > <trap return> > > [ preempt_count = 1 ] > > ld preempt_count, %eax > > [ %eax = 1 ] > > <trap> > post_kprobe_handler() > preempt_enable_no_resched(); > [ preempt_count = 0 ] > <trap return> > > [ %eax = 1 ] > > add %eax,1 > > [ %eax = 2 ] > > st %eax, preempt_count > > [ preempt_count = 2 ] > > > We just caused preempt count to increment twice when it should have only > incremented once, and this screws everything else up. > > To solve this, I've removed preempt disabling code from kprobes, > since the breakpoint exception and kprobes single step routine > disables interrupts, it doesn't need to disable preemption while > single-stepping anymore. > > This patch is for -tip tree, and it can be applied to linus tree too.
I applied it to v3.0-rc5. And not surprisingly it works. But the question I have is, when we return from the trap, and NEED_RECHED is set, will it schedule? My test placed the probe within the scheduler where preemption is already disabled. Let me do this in places that has preemption and interrupts enabled.
I'll also try a kernel mod that adds a probe handler that does a udelay() loop, forcing the timer interrupt to be set on return of the trap, and see what happens there.
-- Steve
| |