[PATCH 0/4] kallsyms header cleanup

[PATCH 0/4] kallsyms header cleanup

[Tom Zanussi]

During review of an unrelated patchset, the question was asked why
module.h was included in code that wouldn't be a module and didn't do
anything with modules [1].

The reason is that it uses kallsyms defines, but kallsyms.h doesn't
include module.h, though it should because it uses MODULE_NAME_LEN.
The code in question also didn't include kallsyms.h but relied on
ftrace.h to pull it in.  But ftrace.h no longer contains anything that
needs kallsyms.h, and continues to include it only because other code
expects it to.

This patchset is the start of cleaning all that up.  It also adds
explicit kallsyms.h includes and removes module.h includes for obvious
cases in kernel/trace and lib/, where I happened to be working.

There are a bunch of other files that probably include module.h only
for kallsyms, but I haven't gone through them yet.  I can either
resubmit this patchset after looking at all those, or can submit a
follow-on patchset to this one assuming this is best way to do it...

[1] https://lkml.org/lkml/2015/4/4/70

The following changes since commit 3cfb2f7976a2f9cfe48eda4e6d7e4ce74fee09e3:

  Merge tag 'pci-v4.0-fixes-3' of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci (2015-04-09 10:17:44 -0700)

are available in the git repository at:


  git://git.yoctoproject.org/linux-yocto-contrib.git tzanussi/kallsyms-header-cleanup
  http://git.yoctoproject.org/cgit/cgit.cgi/linux-yocto-contrib/log/?h=tzanussi/kallsyms-header-cleanup

Tom Zanussi (4):
  tracing: Remove kallsyms.h include from linux/ftrace.h
  kallsyms: Add module.h include
  tracing: Remove redundant module.h includes
  lib: Remove redundant module.h includes

 include/linux/ftrace.h            | 1 -
 include/linux/kallsyms.h          | 1 +
 kernel/trace/trace.c              | 1 -
 kernel/trace/trace_branch.c       | 1 -
 kernel/trace/trace_export.c       | 1 -
 kernel/trace/trace_irqsoff.c      | 1 -
 kernel/trace/trace_kprobe.c       | 1 +
 kernel/trace/trace_output.c       | 2 +-
 kernel/trace/trace_sched_switch.c | 1 -
 kernel/trace/trace_sched_wakeup.c | 1 -
 kernel/trace/trace_stack.c        | 1 -
 kernel/trace/trace_syscalls.c     | 2 +-
 lib/vsprintf.c                    | 2 +-
 13 files changed, 5 insertions(+), 11 deletions(-)

-- 
1.9.3

[PATCH 1/4] tracing: Remove kallsyms.h include from linux/ftrace.h

[Tom Zanussi]

kallsyms.h was included by ftrace.h for KSYM_NAME_LEN, but that usage
was removed by commit 3f5ec13696f [tracing/fastboot: move boot tracer
structs and funcs into their own header].

Remove kallsyms.h and have users relying on ftrace.h to include it for
them include it explicitly.

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
---
 include/linux/ftrace.h        | 1 -
 kernel/trace/trace_kprobe.c   | 1 +
 kernel/trace/trace_output.c   | 1 +
 kernel/trace/trace_syscalls.c | 1 +
 4 files changed, 3 insertions(+), 1 deletion(-)

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>
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index d73f565..52abd46 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -18,6 +18,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/kallsyms.h>
 #include <linux/uaccess.h>
 
 #include "trace_probe.h"
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 692bf71..135dbd5 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/kallsyms.h>
 #include <linux/mutex.h>
 #include <linux/ftrace.h>
 
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index f97f6e3..3cb7355 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -4,6 +4,7 @@
 #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>
-- 
1.9.3

[PATCH 2/4] kallsyms: Add module.h include

[Tom Zanussi]

KSYM_SYMBOL_LEN uses MODULE_NAME_LEN defined in module.h, so include
module.h so users don't have to.

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
---
 include/linux/kallsyms.h | 1 +
 1 file changed, 1 insertion(+)

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) + \
-- 
1.9.3

[PATCH 3/4] tracing: Remove redundant module.h includes

[Tom Zanussi]

kallsyms.h now includes module.h, so remove module.h includes that
were apparently there only to satisfy kallsyms use of MODULE_NAME_LEN.

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
---
 kernel/trace/trace.c              | 1 -
 kernel/trace/trace_branch.c       | 1 -
 kernel/trace/trace_export.c       | 1 -
 kernel/trace/trace_irqsoff.c      | 1 -
 kernel/trace/trace_output.c       | 1 -
 kernel/trace/trace_sched_switch.c | 1 -
 kernel/trace/trace_sched_wakeup.c | 1 -
 kernel/trace/trace_stack.c        | 1 -
 kernel/trace/trace_syscalls.c     | 1 -
 9 files changed, 9 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 62c6506..537c60d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -26,7 +26,6 @@
 #include <linux/uaccess.h>
 #include <linux/kprobes.h>
 #include <linux/ftrace.h>
-#include <linux/module.h>
 #include <linux/percpu.h>
 #include <linux/splice.h>
 #include <linux/kdebug.h>
diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c
index 57cbf1e..05a69d3 100644
--- a/kernel/trace/trace_branch.c
+++ b/kernel/trace/trace_branch.c
@@ -8,7 +8,6 @@
 #include <linux/spinlock.h>
 #include <linux/irqflags.h>
 #include <linux/uaccess.h>
-#include <linux/module.h>
 #include <linux/ftrace.h>
 #include <linux/hash.h>
 #include <linux/fs.h>
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index 12e2b99..8909cd7 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -8,7 +8,6 @@
 #include <linux/seq_file.h>
 #include <linux/uaccess.h>
 #include <linux/ftrace.h>
-#include <linux/module.h>
 #include <linux/init.h>
 
 #include "trace_output.h"
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index 8523ea3..312a412 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -11,7 +11,6 @@
  */
 #include <linux/kallsyms.h>
 #include <linux/uaccess.h>
-#include <linux/module.h>
 #include <linux/ftrace.h>
 
 #include "trace.h"
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 135dbd5..7cc8e96 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -5,7 +5,6 @@
  *
  */
 
-#include <linux/module.h>
 #include <linux/kallsyms.h>
 #include <linux/mutex.h>
 #include <linux/ftrace.h>
diff --git a/kernel/trace/trace_sched_switch.c b/kernel/trace/trace_sched_switch.c
index 419ca37..e084ccf 100644
--- a/kernel/trace/trace_sched_switch.c
+++ b/kernel/trace/trace_sched_switch.c
@@ -4,7 +4,6 @@
  * Copyright (C) 2007 Steven Rostedt <srostedt@redhat.com>
  *
  */
-#include <linux/module.h>
 #include <linux/kallsyms.h>
 #include <linux/uaccess.h>
 #include <linux/ftrace.h>
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index d6e1003..05b9f29 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -9,7 +9,6 @@
  *  Copyright (C) 2004-2006 Ingo Molnar
  *  Copyright (C) 2004 Nadia Yvette Chambers
  */
-#include <linux/module.h>
 #include <linux/kallsyms.h>
 #include <linux/uaccess.h>
 #include <linux/ftrace.h>
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index c3e4fcf..6179cfd 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -8,7 +8,6 @@
 #include <linux/spinlock.h>
 #include <linux/uaccess.h>
 #include <linux/ftrace.h>
-#include <linux/module.h>
 #include <linux/sysctl.h>
 #include <linux/init.h>
 
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 3cb7355..da74ff7 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -3,7 +3,6 @@
 #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>
-- 
1.9.3

[PATCH 4/4] lib: Remove redundant module.h includes

[Tom Zanussi]

kallsyms.h now includes module.h, so remove module.h includes that
were apparently there only to satisfy kallsyms use of MODULE_NAME_LEN.

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
---
 lib/vsprintf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index b235c96..a573500 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -17,7 +17,7 @@
  */
 
 #include <stdarg.h>
-#include <linux/module.h>	/* for KSYM_SYMBOL_LEN */
+#include <linux/module.h>      /* for KSYM_SYMBOL_LEN */
 #include <linux/types.h>
 #include <linux/string.h>
 #include <linux/ctype.h>
-- 
1.9.3

Re: [PATCH 3/4] tracing: Remove redundant module.h includes

[Steven Rostedt]

On Fri, 10 Apr 2015 11:05:02 -0500
Tom Zanussi <tom.zanussi@linux.intel.com> wrote:

> kallsyms.h now includes module.h, so remove module.h includes that
> were apparently there only to satisfy kallsyms use of MODULE_NAME_LEN.

Only remove it for those that need it for MODULE_NAME_LEN because it
was used in KSYM_SYMBOL_LEN. But not the rest. Lots of these files use
module code directly. Now you made it require kallsyms.h to get to that
code. If for some reason we no longer need kallsyms.h, or that
kallsyms.h no longer needs module.h, these files will not build.

If code uses module code, it must include module.h.

So NACK for this patch.

To see what needs module.h, before reverting this patch, remove the
module.h from kallsyms.h, and also remove the MODULE_NAME_LEN from it as
well. Compile all the tracing code, and see what does not build. Those
files will be the ones that you need to add back the module.h to.

-- Steve

Re: [PATCH 4/4] lib: Remove redundant module.h includes

[Steven Rostedt]

On Fri, 10 Apr 2015 11:05:03 -0500
Tom Zanussi <tom.zanussi@linux.intel.com> wrote:

> kallsyms.h now includes module.h, so remove module.h includes that
> were apparently there only to satisfy kallsyms use of MODULE_NAME_LEN.
> 
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> ---
>  lib/vsprintf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index b235c96..a573500 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -17,7 +17,7 @@
>   */
>  
>  #include <stdarg.h>
> -#include <linux/module.h>	/* for KSYM_SYMBOL_LEN */
> +#include <linux/module.h>      /* for KSYM_SYMBOL_LEN */

Hmm.

-- Steve

>  #include <linux/types.h>
>  #include <linux/string.h>
>  #include <linux/ctype.h>

Re: [PATCH 3/4] tracing: Remove redundant module.h includes

[Tom Zanussi]

On Fri, 2015-04-10 at 12:27 -0400, Steven Rostedt wrote:
> On Fri, 10 Apr 2015 11:05:02 -0500
> Tom Zanussi <tom.zanussi@linux.intel.com> wrote:
> 
> > kallsyms.h now includes module.h, so remove module.h includes that
> > were apparently there only to satisfy kallsyms use of MODULE_NAME_LEN.
> 
> Only remove it for those that need it for MODULE_NAME_LEN because it
> was used in KSYM_SYMBOL_LEN. But not the rest. Lots of these files use
> module code directly. Now you made it require kallsyms.h to get to that
> code. If for some reason we no longer need kallsyms.h, or that
> kallsyms.h no longer needs module.h, these files will not build.
> 
> If code uses module code, it must include module.h.
> 

I had grepped all those files for 'module' and didn't find anything
other than comments.  The only files of those affected that use
KSYM_SYMBOL_LEN are trace_output and trace_syscalls, so I can respin the
patch including just those.

But I don't see that any of the other files really use module code, and
looking at those again, none of them seem to need kallsyms either so
they could remove those includes as well...  But I agree all that that
probably doesn't belong as part of this patchset.

> So NACK for this patch.
> 
> To see what needs module.h, before reverting this patch, remove the
> module.h from kallsyms.h, and also remove the MODULE_NAME_LEN from it as
> well. Compile all the tracing code, and see what does not build. Those
> files will be the ones that you need to add back the module.h to.
> 

Yeah, doing that I didn't see any build errors, but I'll stick with just
the explicit KSYM_SYMBOL_LEN-using files for this patchset to keep
things restricted to the specific case at hand.

Tom

> -- Steve
> 

Re: [PATCH 4/4] lib: Remove redundant module.h includes

[Tom Zanussi]

On Fri, 2015-04-10 at 12:28 -0400, Steven Rostedt wrote:
> On Fri, 10 Apr 2015 11:05:03 -0500
> Tom Zanussi <tom.zanussi@linux.intel.com> wrote:
> 
> > kallsyms.h now includes module.h, so remove module.h includes that
> > were apparently there only to satisfy kallsyms use of MODULE_NAME_LEN.
> > 
> > Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> > ---
> >  lib/vsprintf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index b235c96..a573500 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -17,7 +17,7 @@
> >   */
> >  
> >  #include <stdarg.h>
> > -#include <linux/module.h>	/* for KSYM_SYMBOL_LEN */
> > +#include <linux/module.h>      /* for KSYM_SYMBOL_LEN */
> 
> Hmm.

Hmm, yeah, I originally had everything in the same patch then moved
things around - obviously something went wrong ;-)  Will fix in the
updated set.

Tom

> 
> -- Steve
> 
> >  #include <linux/types.h>
> >  #include <linux/string.h>
> >  #include <linux/ctype.h>
> 

Re: [PATCH 3/4] tracing: Remove redundant module.h includes

[Steven Rostedt]

On Fri, 10 Apr 2015 13:25:04 -0500
Tom Zanussi <tom.zanussi@linux.intel.com> wrote:

> On Fri, 2015-04-10 at 12:27 -0400, Steven Rostedt wrote:
> > On Fri, 10 Apr 2015 11:05:02 -0500
> > Tom Zanussi <tom.zanussi@linux.intel.com> wrote:
> > 
> > > kallsyms.h now includes module.h, so remove module.h includes that
> > > were apparently there only to satisfy kallsyms use of MODULE_NAME_LEN.
> > 
> > Only remove it for those that need it for MODULE_NAME_LEN because it
> > was used in KSYM_SYMBOL_LEN. But not the rest. Lots of these files use
> > module code directly. Now you made it require kallsyms.h to get to that
> > code. If for some reason we no longer need kallsyms.h, or that
> > kallsyms.h no longer needs module.h, these files will not build.
> > 
> > If code uses module code, it must include module.h.
> > 
> 
> I had grepped all those files for 'module' and didn't find anything
> other than comments.  The only files of those affected that use
> KSYM_SYMBOL_LEN are trace_output and trace_syscalls, so I can respin the
> patch including just those.
> 

Hmm, I thought I did have some stuff in there for modules. Maybe not
anymore.

> But I don't see that any of the other files really use module code, and
> looking at those again, none of them seem to need kallsyms either so
> they could remove those includes as well...  But I agree all that that
> probably doesn't belong as part of this patchset.
> 
> > So NACK for this patch.
> > 
> > To see what needs module.h, before reverting this patch, remove the
> > module.h from kallsyms.h, and also remove the MODULE_NAME_LEN from it as
> > well. Compile all the tracing code, and see what does not build. Those
> > files will be the ones that you need to add back the module.h to.
> > 
> 
> Yeah, doing that I didn't see any build errors, but I'll stick with just
> the explicit KSYM_SYMBOL_LEN-using files for this patchset to keep
> things restricted to the specific case at hand.
> 

Yeah, if they still compile without the module.h either in kallsyms, or
explicitly being called, then it should be OK to remove them. And yes,
in another patch.

-- Steve