Messages in this thread | | | Subject | Re: [PATCH 2/5] ftrace: Use breakpoint method to update ftrace caller | From | Steven Rostedt <> | Date | Thu, 31 May 2012 10:19:49 -0400 |
| |
On Thu, 2012-05-31 at 13:18 +0200, Peter Zijlstra wrote: > On Wed, 2012-05-30 at 21:28 -0400, Steven Rostedt wrote: > > From: Steven Rostedt <srostedt@redhat.com> > > > > On boot up and module load, it is fine to modify the code directly, > > without the use of breakpoints. This is because boot up modification > > is done before SMP is initialized, thus the modification is serial, > > and module load is done before the module executes. > > > > But after that we must use a SMP safe method to modify running code. > > > > This has been done to change the nops at all the functions, but > > the change of the ftrace callback handler itself was still using a > > direct modification. If tracing was enabled and the function callback > > was changed then another CPU could fault if it was currently calling > > the original callback. This modification must use the breakpoint method > > too. > > > > Note, the direct method is still used for boot up and module load. > > The changelog isn't clear if this is a fix or optimization. I suspect > the latter.
It's a bug fix. There's a GPF kernel crash creeping around in the current code. I can add to the change log what I put into my cover letter:
"The second fixes a bug where we don't do the breakpoint update of the function trace callback (the one called in the mcount trampoline). This could cause a GPF when tracing is running and the function tracer changes."
> > Still, you're now re-inventing text_poke() and text_poke_early().
I explained this in my other email. Also, it wasn't really re-invented, as I believe ftrace did this first (back in 2007). text_poke() looks to have come into play around 2008. Ftrace was the first to do live updates of code while the system was up and running. Before that, the alternatives code just changed the system at boot up (or going from SMP to UP or vice versa). It never did the change in an SMP environment.
This patchset is not a implementation of text_poke() but a fix to what was done earlier. The breakpoint method was something that text_poke() tried earlier, but H. Peter, warned that Intel has not said it was safe to do so. Thus we waited. When we got the unofficial OK, I added the breakpoint method to get rid of stop_machine() from ftrace. That was the main motivation for this.
> > Why are you keeping all this inside of ftrace?
I'm not ;-) Well, actually, I wont be. As I explained in my last email, I talked to Masami about it, and we are discussing ways to change text poke to do basically what ftrace is doing. I'm doing this with ftrace first because it removes stop_machine() from it, and it also a greater test case. If something is to go wrong, it will more likely go wrong with 30,000 changes at once, than just 100.
-- Steve
| |