On Thu, 2011-01-20 at 14:03 +0300, Sergei Shtylyov wrote: > > diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c > > index bc91e4a..62775d7 100644 > > --- a/arch/mips/kernel/ftrace.c > > +++ b/arch/mips/kernel/ftrace.c > [...] > > @@ -304,7 +304,14 @@ void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra, > > return; > > } > > > > - trace.func = self_ra; > > + /* > > + * Get the recorded ip of the current mcount calling site in the > > + * __mcount_loc section, which will be used to filter the function > > + * entries configured through the tracing/set_graph_function interface. > > + */ > > + > > + insns = (in_kernel_space(self_ra)) ? 2 : (MCOUNT_OFFSET_INSNS + 1); > > Unneeded parens. agreed > > > + trace.func = self_ra - (MCOUNT_INSN_SIZE * insns); > > Here too. I like the parens here. Yes, it is basic math precedence, but it stands out to a reviewer who has their brain more focused on correct code than thinking about which op has precedence. Reviewing code that has: trace.func = self_ra - MCOUNT_INSN_SIZE * insns; And as I my mother language reads left to right, my thought process goes: subtract MCOUNT_INSN_SIZE from self_ra and then times insns... oh wait, times goes first; stop reset, restart... subtract MCOUNT_INSN_SIZE times insns from self_ra. That stop reset and restart of the thought process breaks the train of thought and could waste a lot more time than just the moment it happened. All this is avoided by the parenthesis that automatically trigger the brain to think those go first. I say, leave these in. -- Steve
