Re: [PATCH -tip] x86: ftrace.c declare prepare_ftrace_return before they get used

[Frederic Weisbecker <fweisbec@gmail.com>]

On Fri, Apr 10, 2009 at 09:59:55AM +0530, Jaswinder Singh Rajput wrote:
> On Thu, 2009-04-09 at 14:55 -0400, Steven Rostedt wrote:
> > 
> > On Thu, 9 Apr 2009, Jaswinder Singh Rajput wrote:
> > 
> > > 
> > > Subject: [PATCH] x86: ftrace.c declare prepare_ftrace_return before they get used
> > > 
> > > Impact: fix sparse warning:
> > >   arch/x86/kernel/ftrace.c:411:6: warning: symbol 'prepare_ftrace_return' was not declared. Should it be static?
> > > 
> > > Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> > > ---
> > >  arch/x86/include/asm/ftrace.h |    5 +++++
> > >  1 files changed, 5 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> > > index bd2c651..ddc9236 100644
> > > --- a/arch/x86/include/asm/ftrace.h
> > > +++ b/arch/x86/include/asm/ftrace.h
> > > @@ -59,6 +59,11 @@ struct dyn_arch_ftrace {
> > >  };
> > >  
> > >  #endif /*  CONFIG_DYNAMIC_FTRACE */
> > > +
> > > +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > > +extern void prepare_ftrace_return(unsigned long *, unsigned long);
> > > +#endif
> > > +
> > 
> > The only caller is from assembly, so this patch is unnecessary. Unless we 
> > have some rule that all functions must have a prototype, even when it is 
> > only called from assembly.
> > 
> 
> Then:
> 
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 18dfa30..75c0682 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -408,6 +408,7 @@ int ftrace_disable_ftrace_graph_caller(void)
>   * Hook the return address and push it in the stack of return addrs
>   * in current thread info.
>   */
> +asmlinkage



No. This is how prepare_ftrace_return was called at first.
But note that prepare_ftrace_return may be called for every
kernel function while tracing, hence every pico optimization
is important. asmlinkage will push the arguments to the stack
while we want to keep the fastcall here by passing them to the
registers.

Sparse is a useful tool, but sometime there are exceptions when its
warnings should be ignored IMO.

Frederic.

>  void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
>  {
>         unsigned long old;
> 
> 
> Josh: Do you think sparse should not warn like for this case.
> What will be the best solution for this case.
> 
> --
> JSR
> 
>