Re: [PATCH v3 5/7] tracing: Add 'hist' event trigger command

[Paul Bolle <pebolle@tiscali.nl>]

Tom Zanussi schreef op za 04-04-2015 om 15:09 [-0500]:
> [PATCH] kallsyms: Include module.h
> 
> KSYM_SYMBOL_LEN references MODULE_NAME_LEN, defined in module.h, so
> have kallsyms.h include module.h instead of expecting kallsysms users
> to do it.
> 
> Also fix up existing kallsyms users that apparently include module.h
> for only that reason.
> 
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> ---
>  include/linux/ftrace.h           | 1 -
>  include/linux/kallsyms.h         | 1 +
>  kernel/trace/trace_events_hist.c | 2 +-
>  kernel/trace/trace_kprobe.c      | 1 +
>  kernel/trace/trace_output.c      | 1 +
>  kernel/trace/trace_syscalls.c    | 2 +-
>  6 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 1da6029..8554dd5 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -7,7 +7,6 @@
>  #define _LINUX_FTRACE_H
>  
>  #include <linux/trace_clock.h>
> -#include <linux/kallsyms.h>
>  #include <linux/linkage.h>
>  #include <linux/bitops.h>
>  #include <linux/ptrace.h>

The commit explanation is silent about this change. Are you sure you
want to include it?

> diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
> index 6883e19..e1550a4 100644
> --- a/include/linux/kallsyms.h
> +++ b/include/linux/kallsyms.h
> @@ -8,6 +8,7 @@
>  #include <linux/errno.h>
>  #include <linux/kernel.h>
>  #include <linux/stddef.h>
> +#include <linux/module.h>
>  
>  #define KSYM_NAME_LEN 128
>  #define KSYM_SYMBOL_LEN (sizeof("%s+%#lx/%#lx [%s]") + (KSYM_NAME_LEN - 1) + \
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 3d648ee..c290379 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -20,7 +20,7 @@
>   * http://www.azulsystems.com/events/javaone_2007/2007_LockFreeHash.pdf
>   */
>  
> -#include <linux/module.h>
> +#include <linux/kallsyms.h>
>  #include <linux/ctype.h>
>  #include <linux/mutex.h>
>  #include <linux/slab.h>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 9ba3f43..39019ec 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -19,6 +19,7 @@
>  
>  #include <linux/module.h>

Note to self: needed for "struct module". But this file has two
references to THIS_MODULE that look useless. 

>  #include <linux/uaccess.h>
> +#include <linux/kallsyms.h>
>  
>  #include "trace_probe.h"
>  
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 692bf71..5ec310e 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -8,6 +8,7 @@
>  #include <linux/module.h>

Can't this be dropped too?

>  #include <linux/mutex.h>
>  #include <linux/ftrace.h>
> +#include <linux/kallsyms.h>
>  
>  #include "trace_output.h"
>  
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index d137e0a..72af617 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -3,7 +3,7 @@
>  #include <linux/syscalls.h>
>  #include <linux/slab.h>
>  #include <linux/kernel.h>
> -#include <linux/module.h>	/* for MODULE_NAME_LEN via KSYM_SYMBOL_LEN */
> +#include <linux/kallsyms.h>
>  #include <linux/ftrace.h>
>  #include <linux/perf_event.h>
>  #include <asm/syscall.h>

Thanks,


Paul Bolle