Re: [PATCH 9/9] tracing: MIPS: cleanup of the address space checking

[Wu Zhangjin <wuzhangjin@gmail.com>]

On Wed, 2010-05-12 at 10:13 -0700, David Daney wrote:
> On 05/12/2010 06:23 AM, Wu Zhangjin wrote:
> > From: Wu Zhangjin<wuzhangjin@gmail.com>
> >
> > This patch adds an inline function in_module() to check which space the
> > instruction pointer in, kernel space or module space.
> >
> > Note: This may not work when the kernel is compiled with -msym32.
> >
> 
> The kernel is always compiled with -msym32, so the patch is a bit pointless.
> 
> 

In reality, with -msym32, it works well on my machine, but seems you and
some other people told me that it may not work when the kernel and
module space are the same with -msym32 and some other options. perhaps I
need to change the comments to just:

Note: This will not work when the kernel and module space are the same.

If the kernel and module space are the same, We just need to modify the
recording of the calling site to _mcount in scripts/recordmcount.pl and
tune the related address handling in
ftrace_make_nop()/ftrace_make_call().

But I have no such situation to test, how can I simply let the module
has the same address space as the kernel. without -mlong-calls? and
should we change arch/mips/kernel/vmlinux.lds.S and
arch/mips/kernel/module.c?

If the module space and kernel space are the same, we may remove the
long call from the module space to kernel space to speedup the function
call.

> >
> > diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
> > index 628e90b..37f15b6 100644
> > --- a/arch/mips/kernel/ftrace.c
> > +++ b/arch/mips/kernel/ftrace.c
> > @@ -17,6 +17,17 @@
> >   #include<asm/cacheflush.h>
> >   #include<asm/uasm.h>
> >
> > +/*
> > + * If the Instruction Pointer is in module space (0xc0000000), return true;
> > + * otherwise, it is in kernel space (0x80000000), return false.
> > + *
> > + * FIXME: This may not work when the kernel is compiled with -msym32.
> > + */
> > +static inline int in_module(unsigned long ip)
> > +{
> > +	return ip&  0x40000000;
> > +}
> > +
> 
> How about (untested):
> 
> 
> static inline int in_module(unsigned long ip)
> {
> 	return ip < _text || ip > _etext;
> }
> 

It may work, thanks!

> 
> But why do we even care?  Can't we just probe the function prologue and 
> determine from that what needs to be done?

Yes, it should work via checking the instructions in the memory before
the ip but I think a lighter solution is better.

Thanks & Regards,
	Wu Zhangjin