kallsyms header cleanup
Subsystems (5)
cover kallsyms header cleanup
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(-)
Signature · 1 line
1.9.3
PATCH 2/4 kallsyms: Add module.h include
KSYM_SYMBOL_LEN uses MODULE_NAME_LEN defined in module.h, so include module.h so users don't have to.
--- include/linux/kallsyms.h | 1 + 1 file changed, 1 insertion(+)
include/linux/kallsyms.h +1 -0
@@ -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) + \
Signature · 1 line
1.9.3
PATCH 1/4 tracing: Remove kallsyms.h include from linux/ftrace.h
In reply to Tom Zanussikallsyms.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.
--- 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(-)
include/linux/ftrace.h +0 -1
@@ -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>
kernel/trace/trace_kprobe.c +1 -0
@@ -18,6 +18,7 @@*/ #include <linux/module.h> +#include <linux/kallsyms.h> #include <linux/uaccess.h> #include "trace_probe.h"
kernel/trace/trace_output.c +1 -0
@@ -6,6 +6,7 @@*/ #include <linux/module.h> +#include <linux/kallsyms.h> #include <linux/mutex.h> #include <linux/ftrace.h>
kernel/trace/trace_syscalls.c +1 -0
@@ -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>
Signature · 1 line
1.9.3
PATCH 3/4 tracing: Remove redundant module.h includes
In reply to Tom Zanussikallsyms.h now includes module.h, so remove module.h includes that were apparently there only to satisfy kallsyms use of MODULE_NAME_LEN.
--- 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(-)
kernel/trace/trace.c +0 -1
@@ -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>
kernel/trace/trace_branch.c +0 -1
@@ -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>
kernel/trace/trace_export.c +0 -1
@@ -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"
kernel/trace/trace_irqsoff.c +0 -1
@@ -11,7 +11,6 @@*/ #include <linux/kallsyms.h> #include <linux/uaccess.h> -#include <linux/module.h> #include <linux/ftrace.h> #include "trace.h"
kernel/trace/trace_output.c +0 -1
@@ -5,7 +5,6 @@* */ -#include <linux/module.h> #include <linux/kallsyms.h> #include <linux/mutex.h> #include <linux/ftrace.h>
kernel/trace/trace_sched_switch.c +0 -1
@@ -4,7 +4,6 @@* Copyright (C) 2007 Steven Rostedt <[email protected]> * */ -#include <linux/module.h> #include <linux/kallsyms.h> #include <linux/uaccess.h> #include <linux/ftrace.h>
kernel/trace/trace_sched_wakeup.c +0 -1
@@ -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>
kernel/trace/trace_stack.c +0 -1
@@ -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>
kernel/trace/trace_syscalls.c +0 -1
@@ -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>
Signature · 1 line
1.9.3
PATCH 4/4 lib: Remove redundant module.h includes
In reply to Tom Zanussikallsyms.h now includes module.h, so remove module.h includes that were apparently there only to satisfy kallsyms use of MODULE_NAME_LEN.
--- lib/vsprintf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/vsprintf.c +1 -1
@@ -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>
Signature · 1 line
1.9.3
Steven Rostedt
maintainerRe: [PATCH 3/4] tracing: Remove redundant module.h includes
In reply to Tom ZanussiQuoted 2 lines · On Fri, 10 Apr 2015 11:05:02 -0500 Tom Zanussi <[email protected]>…
kallsyms.hnow includesmodule.h, so removemodule.hincludes that were apparently there only to satisfy kallsyms use ofMODULE_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
Steven Rostedt
maintainerRe: [PATCH 4/4] lib: Remove redundant module.h includes
In reply to Tom ZanussiQuoted 13 lines · On Fri, 10 Apr 2015 11:05:03 -0500 Tom Zanussi <[email protected]>…
kallsyms.hnow includesmodule.h, so removemodule.hincludes that were apparently there only to satisfy kallsyms use ofMODULE_NAME_LEN.Signed-off-by Tom Zanussi <[email protected]>--- lib/vsprintf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)lib/vsprintf.c +0
-0
lib/vsprintf.c line 20
Quoted 5 lines
lib/vsprintf.c +1
-1@@ -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
Quoted 3 lines
lib/vsprintf.c +0
-0#include <linux/types.h> #include <linux/string.h> #include <linux/ctype.h>
Re: [PATCH 3/4] tracing: Remove redundant module.h includes
In reply to Steven RostedtQuoted 6 lines · Steven Rostedt
Quoted 2 lines · On Fri, 10 Apr 2015 11:05:02 -0500 Tom Zanussi <[email protected]>…
kallsyms.hnow includesmodule.h, so removemodule.hincludes that were apparently there only to satisfy kallsyms use ofMODULE_NAME_LEN.
Quoted 8 lines
Only remove it for those that need it for
MODULE_NAME_LENbecause it was used inKSYM_SYMBOL_LEN. But not the rest. Lots of these files use module code directly. Now you made it requirekallsyms.hto get to that code. If for some reason we no longer needkallsyms.h, or thatkallsyms.hno longer needsmodule.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.
Quoted 7 lines
So NACK for this patch.
To see what needs
module.h, before reverting this patch, remove themodule.hfromkallsyms.h, and also remove theMODULE_NAME_LENfrom 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 themodule.hto.
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
Quoted 2 lines
-- Steve
Re: [PATCH 4/4] lib: Remove redundant module.h includes
In reply to Steven RostedtQuoted 18 lines · Steven Rostedt
Quoted 15 lines · On Fri, 10 Apr 2015 11:05:03 -0500 Tom Zanussi <[email protected]>…
kallsyms.hnow includesmodule.h, so removemodule.hincludes that were apparently there only to satisfy kallsyms use ofMODULE_NAME_LEN.Signed-off-by Tom Zanussi <[email protected]>--- lib/vsprintf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)lib/vsprintf.c +0
-0@@ -17,7 +17,7 @@*/
Quoted 5 lines
lib/vsprintf.c line 20
Quoted 3 lines
lib/vsprintf.c +1
-1#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
Quoted 7 lines
-- Steve
Quoted 3 lines
lib/vsprintf.c +0
-0#include <linux/types.h> #include <linux/string.h> #include <linux/ctype.h>
Steven Rostedt
maintainerRe: [PATCH 3/4] tracing: Remove redundant module.h includes
In reply to Tom ZanussiQuoted 13 lines · On Fri, 10 Apr 2015 13:25:04 -0500 Tom Zanussi <[email protected]>…
Quoted 12 lines · Steven Rostedt
Quoted 2 lines · On Fri, 10 Apr 2015 11:05:02 -0500 Tom Zanussi <[email protected]>…
kallsyms.hnow includesmodule.h, so removemodule.hincludes that were apparently there only to satisfy kallsyms use ofMODULE_NAME_LEN.Only remove it for those that need it for
MODULE_NAME_LENbecause it was used inKSYM_SYMBOL_LEN. But not the rest. Lots of these files use module code directly. Now you made it requirekallsyms.hto get to that code. If for some reason we no longer needkallsyms.h, or thatkallsyms.hno longer needsmodule.h, these files will not build.
Quoted 8 lines
Quoted 2 lines
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_LENaretrace_outputandtrace_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.
Quoted 13 lines
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.
Quoted 7 lines
So NACK for this patch.
To see what needs
module.h, before reverting this patch, remove themodule.hfromkallsyms.h, and also remove theMODULE_NAME_LENfrom 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 themodule.hto.
Quoted 4 lines
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