On Fri, Jan 11, 2013 at 09:01:01AM -0800, David Daney wrote: > > I thought all CPUs were in stop_machine() when the modifications > were done, so that there is no issue with multi-word instruction > patching. > > Am I wrong about this? > > So really I think you can do two NOP just as easily. The problem with double NOPs is that it can only work if there's no problem executing one nop and a non NOP. Which I think is an issue here. If you have something like: bl _mcount addiu sp,sp,-8 And you convert that to: nop nop Now if you convert that back to: bl ftrace_caller addiu sp,sp,-8 then you can have an issue if the task was preempted after that first nop. Because stop_machine() doesn't wait for tasks to exit kernel space. If you have a CONFIG_PREEMPT kernel, a task can be sleeping anywhere. Thus you have a task execute the first nop, get preempted. You update the code to be: bl ftrace_caller addiu sp,sp,-8 When that task gets scheduled back in, it will act like it just executed: nop addiu sp,sp,-8 Which is the problem you're trying to solve in the first place. Now that said, There's no reason we need that addiu sp,sp,-8 there. That's just what the mips defined mcount requires. But as you can see above, with dynamic ftrace, the defined mcount is only called at boot up, and never again. That means at boot up you can convert to: nop nop and then when you enable tracing just convert it to: bl ftrace_caller nop There's nothing that states what the ftrace caller must be. We can have it do a proper stack update. That is, only at boot up do we need to handle the defined mcount. After that, those instructions are just place holders for our own algorithms. If the addiu was needed for the defined mcount, there's no reason to keep it for our own ftrace_caller. Would that work? -- Steve
