Messages in this thread | | | Subject | Re: [PATCH] ftrace: x86 use copy to and from user functions | From | Benjamin Herrenschmidt <> | Date | Thu, 21 Aug 2008 08:08:58 +1000 |
| |
On Wed, 2008-08-20 at 17:43 -0400, Steven Rostedt wrote: > > > On Thu, 21 Aug 2008, Benjamin Herrenschmidt wrote: > > > On Wed, 2008-08-20 at 12:55 -0400, Steven Rostedt wrote: > > > The modification of code is performed either by kstop_machine, before > > > SMP starts, or on module code before the module is executed. There is > > > no reason to do the modifications from assembly. The copy to and from > > > user functions are sufficient and produces cleaner and easier to read > > > code. > > > > > > Thanks to Benjamin Herrenschmidt for suggesting the idea. > > > > Haven't we lost the dcache/icache synchronisation somewhere ? > > This is the x86 version, sync_core should be fine.
Oh oops, I didn't look well enough :-)
Ben.
> -- Steve > > > > > > Signed-off-by: Steven Rostedt <srostedt@redhat.com> > > > --- > > > arch/x86/kernel/ftrace.c | 38 +++++++++++++------------------------- > > > 1 file changed, 13 insertions(+), 25 deletions(-) > > > > > > Index: linux-tip.git/arch/x86/kernel/ftrace.c > > > =================================================================== > > > --- linux-tip.git.orig/arch/x86/kernel/ftrace.c 2008-08-20 12:39:41.000000000 -0400 > > > +++ linux-tip.git/arch/x86/kernel/ftrace.c 2008-08-20 12:40:17.000000000 -0400 > > > @@ -11,6 +11,7 @@ > > > > > > #include <linux/spinlock.h> > > > #include <linux/hardirq.h> > > > +#include <linux/uaccess.h> > > > #include <linux/ftrace.h> > > > #include <linux/percpu.h> > > > #include <linux/init.h> > > > @@ -60,11 +61,7 @@ notrace int > > > ftrace_modify_code(unsigned long ip, unsigned char *old_code, > > > unsigned char *new_code) > > > { > > > - unsigned replaced; > > > - unsigned old = *(unsigned *)old_code; /* 4 bytes */ > > > - unsigned new = *(unsigned *)new_code; /* 4 bytes */ > > > - unsigned char newch = new_code[4]; > > > - int faulted = 0; > > > + unsigned char replaced[MCOUNT_INSN_SIZE]; > > > > > > /* > > > * Note: Due to modules and __init, code can > > > @@ -72,29 +69,20 @@ ftrace_modify_code(unsigned long ip, uns > > > * as well as code changing. > > > * > > > * No real locking needed, this code is run through > > > - * kstop_machine. > > > + * kstop_machine, or before SMP starts. > > > */ > > > - asm volatile ( > > > - "1: lock\n" > > > - " cmpxchg %3, (%2)\n" > > > - " jnz 2f\n" > > > - " movb %b4, 4(%2)\n" > > > - "2:\n" > > > - ".section .fixup, \"ax\"\n" > > > - "3: movl $1, %0\n" > > > - " jmp 2b\n" > > > - ".previous\n" > > > - _ASM_EXTABLE(1b, 3b) > > > - : "=r"(faulted), "=a"(replaced) > > > - : "r"(ip), "r"(new), "c"(newch), > > > - "0"(faulted), "a"(old) > > > - : "memory"); > > > - sync_core(); > > > + if (__copy_from_user(replaced, (char __user *)ip, MCOUNT_INSN_SIZE)) > > > + return 1; > > > > > > - if (replaced != old && replaced != new) > > > - faulted = 2; > > > + if (memcmp(replaced, old_code, MCOUNT_INSN_SIZE) != 0) > > > + return 2; > > > > > > - return faulted; > > > + WARN_ON_ONCE(__copy_to_user((char __user *)ip, new_code, > > > + MCOUNT_INSN_SIZE)); > > > + > > > + sync_core(); > > > + > > > + return 0; > > > } > > > > > > notrace int ftrace_update_ftrace_func(ftrace_func_t func) > > > >
| |