Re: [Xen-devel] [PATCH v8 13/20] xenctx: change is_kernel_text() into kernel_addr().

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-03-27 at 15:05 -0400, Don Slutz wrote:
> A new enum has been added to allow the caller to determine if this
> kernel address is a text, data, or module address.  This can help
> understand what is going on.
> 
> Kernel modules are only found when they are more then 1MiB after

s/then/than/

But where does this magic assumption come from? Where does 1MiB come
from?

> kernel_end or the symbol "__brk_limit".  The symbol "vgettimeofday"
> is used to mark the end of where modules can be loaded.

I can just about see from the vmlinux.lds.S why __brk_limit might be
somewhat relevant, but where does the use of vgettimeofday come from?

The 3.2 kernel on my desktop doesn't list vgettimeofday in System.map
for example and in any case you can't rely on it not being moved around.
(you can't really rely on __brk_limit either, but it seems less likely
to move).

(TBH, I'd suggest that this new functionality be left out for now, this
series has gone on for long enough as it is)

> ffff88003b759dc0:  [<ffffffffa005f19d>]
> ffff88003b759e50:  [<ffffffffa005f1e8>]
> ffff88003b759ee0:  [<ffffffffa005f1e8>]
> ffff88003b759f70:  [<ffffffffa005f1e8>]
> 
> This shows that a kernel module is using a lot of stack.

Does it? All I see is the four additional entries at the base, consuming
about 0.5k which I suppose is a lot. I suppose those are stack frames
corresponding to some module? Perhaps mark them as such with "unknown
module function"?

> 
> Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
> ---
> v8:
>   Added more to commit message.
>   Added some basic linux kernel module reporting.
>   Moved kernel_end into define tree in case no symbol found.

kernel_start is overridable on the command line, so kernel_end probably
should be too.

>   At some point __bss_stop was added and _end was dropped. Both
>   mean end of named kernel data.

Hrm I suppose we already encode a bunch of Linux kernel specific symbols
in this tool so a couple more don't hurt.

>  
> -static int is_kernel_text(guest_word_t addr)
> +static type_of_addr kernel_addr(guest_word_t addr)
>  {
> -    if (symbol_table == NULL)
> -        return (addr > kernel_start);
> +    if ( symbol_table == NULL )
> +    {
> +        if ( addr > kernel_start )
> +            return KERNEL_TEXT_ADDR;
> +        else
> +            return NOT_KERNEL_ADDR;
> +    }
>  
>      if (addr >= kernel_stext &&
>          addr <= kernel_etext)
> -        return 1;
> +        return KERNEL_TEXT_ADDR;
>      if ( kernel_hypercallpage &&
>           (addr >= kernel_hypercallpage &&
>            addr <= kernel_hypercallpage + 4096) )
> -        return 1;
> -    if (addr >= kernel_sinittext &&
> -        addr <= kernel_einittext)
> -        return 1;
> -    return 0;
> +        return KERNEL_TEXT_ADDR;
> +    if ( kernel_sinittext &&
> +         (addr >= kernel_sinittext &&
> +          addr <= kernel_einittext) )
> +        return KERNEL_TEXT_ADDR;
> +    if ( xenctx.kernel_start_set )
> +    {
> +        if ( addr >= kernel_start )

Has this logic changed since last time? Why no check of kernel_end?

Previously we only used kernel_start if no symbol table was provided.
That seemed logical enough to me, is there a reason to change that?

> +            return KERNEL_TEXT_ADDR;
> +    }
> +    else
> +    {
> +        if ( kernel_text &&
> +             (addr >= kernel_text &&
> +              addr <= kernel_end) )
> +            return KERNEL_DATA_ADDR;
> +        if ( kernel_mod_start &&
> +             (addr >= kernel_mod_start &&
> +              addr <= kernel_mod_end) )
> +            return KERNEL_MOD_ADDR;
> +    }
> +    return NOT_KERNEL_ADDR;
>  }
>  
> @@ -180,7 +223,14 @@ static void print_symbol(guest_word_t addr)
>      if (addr==s->address)
>          printf(" %s", s->name);
>      else
> -        printf(" %s+%#x", s->name, (unsigned int)(addr - s->address));
> +    {
> +        unsigned long long offset = addr - s->address;
> +
> +        if ( addr_type == KERNEL_MOD_ADDR &&
> +             offset > MAX_MOD_OFFSET )
> +            return;

Shouldn't kernel_addr have not returned KERNEL_MOD_ADDR in that case?

> +        printf(" %s+%#llx", s->name, offset);
> +    }
>  }
>  
>  static void read_symbol_table(const char *symtab)

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel