Re: [for-next][PATCH 2/4] ftrace/x86: Show trampoline call function in enabled_functions

[Steven Rostedt]

On Wed, 5 Nov 2014 11:42:07 +0100
Borislav Petkov <bp@xxxxxxxxx> wrote:


> > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> > index e4d48f6cad86..ca17c20a1010 100644
> > --- a/arch/x86/kernel/ftrace.c
> > +++ b/arch/x86/kernel/ftrace.c
> > @@ -48,7 +48,7 @@ int ftrace_arch_code_modify_post_process(void)
> >  union ftrace_code_union {
> >  	char code[MCOUNT_INSN_SIZE];
> >  	struct {
> > -		char e8;
> > +		unsigned char e8;
> 
> u8?
> 
> "u8 e8;" looks supa dupa though :-)

And compete with PowerPC's eieio command.

> 
> >  		int offset;
> >  	} __attribute__((packed));
> >  };
> > @@ -797,12 +797,26 @@ static unsigned long create_trampoline(struct ftrace_ops *ops)
> >  	return (unsigned long)trampoline;
> >  }
> >  
> > +static unsigned long calc_trampoline_call_offset(bool save_regs)
> > +{
> > +	unsigned long start_offset;
> > +	unsigned long call_offset;
> > +
> > +	if (save_regs) {
> > +		start_offset = (unsigned long)ftrace_regs_caller;
> > +		call_offset = (unsigned long)ftrace_regs_call;
> > +	} else {
> > +		start_offset = (unsigned long)ftrace_caller;
> > +		call_offset = (unsigned long)ftrace_call;
> > +	}
> > +
> > +	return call_offset - start_offset;
> > +}
> > +
> >  void arch_ftrace_update_trampoline(struct ftrace_ops *ops)
> >  {
> >  	ftrace_func_t func;
> >  	unsigned char *new;
> > -	unsigned long start_offset;
> > -	unsigned long call_offset;
> >  	unsigned long offset;
> >  	unsigned long ip;
> >  	int ret;
> > @@ -820,15 +834,7 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops)
> >  			return;
> >  	}
> >  
> > -	if (ops->flags & FTRACE_OPS_FL_SAVE_REGS) {
> > -		start_offset = (unsigned long)ftrace_regs_caller;
> > -		call_offset = (unsigned long)ftrace_regs_call;
> > -	} else {
> > -		start_offset = (unsigned long)ftrace_caller;
> > -		call_offset = (unsigned long)ftrace_call;
> > -	}
> > -
> > -	offset = call_offset - start_offset;
> > +	offset = calc_trampoline_call_offset(ops->flags & FTRACE_OPS_FL_SAVE_REGS);
> >  	ip = ops->trampoline + offset;
> >  
> >  	func = ftrace_ops_get_func(ops);
> > @@ -840,6 +846,74 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops)
> >  	/* The update should never fail */
> >  	WARN_ON(ret);
> >  }
> > +
> > +/* Return the address of the function the trampoline calls */
> > +static void *addr_from_call(void *ptr)
> > +{
> > +	union ftrace_code_union calc;
> > +	int ret;
> > +
> > +	ret = probe_kernel_read(&calc, ptr, MCOUNT_INSN_SIZE);
> > +	if (WARN_ON_ONCE(ret < 0))
> > +		return NULL;
> > +
> > +	/* Make sure this is a call */
> > +	if (WARN_ON_ONCE(calc.e8 != 0xe8)) {
> > +		pr_warn("Expected e8, got %x\n", calc.e8);
> 
> Simply WARN_ONCE gives you both.

Ah, could do that.

> 
> > +		return NULL;
> > +	}
> > +
> > +	return ptr + MCOUNT_INSN_SIZE + calc.offset;
> > +}
> > +
> > +void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
> > +			   unsigned long frame_pointer);
> > +
> > +/*
> > + * If the ops->trampoline was not allocated, then it probably
> > + * has a static trampoline func, or is the ftrace caller itself.
> > + */
> > +static void *static_tramp_func(struct ftrace_ops *ops, struct dyn_ftrace *rec)
> 
> I guess you can merge this one into its only caller below - it is not
> that huge... yet! :-)
> 

Hmm, it's big enough not to be in that function as a if statement that
will return anyway.

gcc will just make it inlined anyway. I think it's better separate for
readability.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/