Remove dupilcated #include in arch/x86/kernel/dumpstack.c.
Signed-off-by: Huang Weiyi <[email protected]>
---
arch/x86/kernel/dumpstack.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 95ea5fa..6d7966d 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -15,7 +15,6 @@
#include <linux/bug.h>
#include <linux/nmi.h>
#include <linux/sysfs.h>
-#include <linux/ftrace.h>
#include <asm/stacktrace.h>
--
1.6.1.3
* Huang Weiyi <[email protected]> wrote:
> Remove dupilcated #include in arch/x86/kernel/dumpstack.c.
>
> Signed-off-by: Huang Weiyi <[email protected]>
> ---
> arch/x86/kernel/dumpstack.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> index 95ea5fa..6d7966d 100644
> --- a/arch/x86/kernel/dumpstack.c
> +++ b/arch/x86/kernel/dumpstack.c
> @@ -15,7 +15,6 @@
> #include <linux/bug.h>
> #include <linux/nmi.h>
> #include <linux/sysfs.h>
> -#include <linux/ftrace.h>
>
> #include <asm/stacktrace.h>
Many of those include lines are probably unnecessary. Instead of
these trivial patches causing churn, would you be interested in
doing a comprehensive search to eliminate all the unused ones?
That would be a real step forward. (and this holds for your
other patches in this series too.)
The include files section of fault.c might be a good template to
use:
#include <linux/magic.h> /* STACK_END_MAGIC */
#include <linux/sched.h> /* test_thread_flag(), ... */
#include <linux/kdebug.h> /* oops_begin/end, ... */
#include <linux/module.h> /* search_exception_table */
#include <linux/bootmem.h> /* max_low_pfn */
#include <linux/kprobes.h> /* __kprobes, ... */
#include <linux/mmiotrace.h> /* kmmio_handler, ... */
#include <linux/perf_counter.h> /* perf_swcounter_*(), ... */
#include <asm/traps.h> /* dotraplinkage, ... */
#include <asm/pgalloc.h> /* pgd_*(), ... */
#include <asm/kmemcheck.h> /* kmemcheck_*(), ... */
I was able to eliminate half of all include file lines there.
See latest -tip for that:
http://people.redhat.com/mingo/tip.git/README
Thanks,
Ingo
On Wed, Apr 08, 2009 at 02:21:21PM +0200, Ingo Molnar wrote:
>
> * Huang Weiyi <[email protected]> wrote:
>
> > Remove dupilcated #include in arch/x86/kernel/dumpstack.c.
> >
> > Signed-off-by: Huang Weiyi <[email protected]>
> > ---
> > arch/x86/kernel/dumpstack.c | 1 -
> > 1 files changed, 0 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> > index 95ea5fa..6d7966d 100644
> > --- a/arch/x86/kernel/dumpstack.c
> > +++ b/arch/x86/kernel/dumpstack.c
> > @@ -15,7 +15,6 @@
> > #include <linux/bug.h>
> > #include <linux/nmi.h>
> > #include <linux/sysfs.h>
> > -#include <linux/ftrace.h>
> >
> > #include <asm/stacktrace.h>
>
> Many of those include lines are probably unnecessary. Instead of
> these trivial patches causing churn, would you be interested in
> doing a comprehensive search to eliminate all the unused ones?
> That would be a real step forward. (and this holds for your
> other patches in this series too.)
>
> The include files section of fault.c might be a good template to
> use:
>
> #include <linux/magic.h> /* STACK_END_MAGIC */
> #include <linux/sched.h> /* test_thread_flag(), ... */
> #include <linux/kdebug.h> /* oops_begin/end, ... */
> #include <linux/module.h> /* search_exception_table */
> #include <linux/bootmem.h> /* max_low_pfn */
> #include <linux/kprobes.h> /* __kprobes, ... */
> #include <linux/mmiotrace.h> /* kmmio_handler, ... */
> #include <linux/perf_counter.h> /* perf_swcounter_*(), ... */
>
> #include <asm/traps.h> /* dotraplinkage, ... */
> #include <asm/pgalloc.h> /* pgd_*(), ... */
> #include <asm/kmemcheck.h> /* kmemcheck_*(), ... */
>
> I was able to eliminate half of all include file lines there.
I assume you are aware that when you minimize the # of include file in
the various .c files, then you implicitly add dependency on the includes
the individual .h files have.
If we for example killed the include of <linux/list.h> from <linux/module.h>
with this approach we would likely see random build failures.
It is good to have it cleaned up but we should keep a critical eye on the
header files we use. Sometimes these are the right target
to clean up but as this is a bit more complicated or at least requires
more build testing to do right.
Sam
* Sam Ravnborg <[email protected]> wrote:
> On Wed, Apr 08, 2009 at 02:21:21PM +0200, Ingo Molnar wrote:
> >
> > * Huang Weiyi <[email protected]> wrote:
> >
> > > Remove dupilcated #include in arch/x86/kernel/dumpstack.c.
> > >
> > > Signed-off-by: Huang Weiyi <[email protected]>
> > > ---
> > > arch/x86/kernel/dumpstack.c | 1 -
> > > 1 files changed, 0 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> > > index 95ea5fa..6d7966d 100644
> > > --- a/arch/x86/kernel/dumpstack.c
> > > +++ b/arch/x86/kernel/dumpstack.c
> > > @@ -15,7 +15,6 @@
> > > #include <linux/bug.h>
> > > #include <linux/nmi.h>
> > > #include <linux/sysfs.h>
> > > -#include <linux/ftrace.h>
> > >
> > > #include <asm/stacktrace.h>
> >
> > Many of those include lines are probably unnecessary. Instead of
> > these trivial patches causing churn, would you be interested in
> > doing a comprehensive search to eliminate all the unused ones?
> > That would be a real step forward. (and this holds for your
> > other patches in this series too.)
> >
> > The include files section of fault.c might be a good template to
> > use:
> >
> > #include <linux/magic.h> /* STACK_END_MAGIC */
> > #include <linux/sched.h> /* test_thread_flag(), ... */
> > #include <linux/kdebug.h> /* oops_begin/end, ... */
> > #include <linux/module.h> /* search_exception_table */
> > #include <linux/bootmem.h> /* max_low_pfn */
> > #include <linux/kprobes.h> /* __kprobes, ... */
> > #include <linux/mmiotrace.h> /* kmmio_handler, ... */
> > #include <linux/perf_counter.h> /* perf_swcounter_*(), ... */
> >
> > #include <asm/traps.h> /* dotraplinkage, ... */
> > #include <asm/pgalloc.h> /* pgd_*(), ... */
> > #include <asm/kmemcheck.h> /* kmemcheck_*(), ... */
> >
> > I was able to eliminate half of all include file lines there.
>
> I assume you are aware that when you minimize the # of include
> file in the various .c files, then you implicitly add dependency
> on the includes the individual .h files have.
Yes. Look at the commit -tip that does the above change (also
attached below):
a2bcd47: x86/mm: further cleanups of fault.c's include file section
that commit uncovered a masked-until-then dependency bug in one of
the x86 include files.
More automation to discover include file dependency bugs would be
nice though, it's hard to get these things right and it all needs a
strong testing infrastructure.
Ingo
----------------->
>From a2bcd4731f77cb77ae4b5e4a3d7f5471cf346c33 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Sun, 29 Mar 2009 23:47:48 +0200
Subject: [PATCH] x86/mm: further cleanups of fault.c's include file section
Impact: cleanup
Eliminate more than 20 unnecessary #include lines in fault.c
Also fix include file dependency bug in asm/traps.h. (this was
masked before, by implicit inclusion)
Signed-off-by: Ingo Molnar <[email protected]>
LKML-Reference: <[email protected]>
Acked-by: H. Peter Anvin <[email protected]>
---
arch/x86/include/asm/traps.h | 1 +
arch/x86/mm/fault.c | 42 +++++++++---------------------------------
2 files changed, 10 insertions(+), 33 deletions(-)
diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index 0d53425..37fb07a 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -2,6 +2,7 @@
#define _ASM_X86_TRAPS_H
#include <asm/debugreg.h>
+#include <asm/siginfo.h> /* TRAP_TRACE, ... */
#ifdef CONFIG_X86_32
#define dotraplinkage
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index a03b727..24a36a6 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -3,40 +3,16 @@
* Copyright (C) 2001, 2002 Andi Kleen, SuSE Labs.
* Copyright (C) 2008-2009, Red Hat Inc., Ingo Molnar
*/
-#include <linux/interrupt.h>
-#include <linux/mmiotrace.h>
-#include <linux/bootmem.h>
-#include <linux/compiler.h>
-#include <linux/highmem.h>
-#include <linux/kprobes.h>
-#include <linux/uaccess.h>
-#include <linux/vmalloc.h>
-#include <linux/vt_kern.h>
-#include <linux/signal.h>
-#include <linux/kernel.h>
-#include <linux/ptrace.h>
-#include <linux/string.h>
-#include <linux/module.h>
-#include <linux/kdebug.h>
-#include <linux/errno.h>
-#include <linux/magic.h>
-#include <linux/sched.h>
-#include <linux/types.h>
-#include <linux/init.h>
-#include <linux/mman.h>
-#include <linux/tty.h>
-#include <linux/smp.h>
-#include <linux/mm.h>
-
-#include <asm-generic/sections.h>
-
-#include <asm/tlbflush.h>
-#include <asm/pgalloc.h>
-#include <asm/segment.h>
-#include <asm/system.h>
-#include <asm/proto.h>
-#include <asm/traps.h>
-#include <asm/desc.h>
+#include <linux/magic.h> /* STACK_END_MAGIC */
+#include <linux/sched.h> /* test_thread_flag(), ... */
+#include <linux/kdebug.h> /* oops_begin/end, ... */
+#include <linux/module.h> /* search_exception_table */
+#include <linux/bootmem.h> /* max_low_pfn */
+#include <linux/kprobes.h> /* __kprobes, ... */
+#include <linux/mmiotrace.h> /* kmmio_handler, ... */
+
+#include <asm/traps.h> /* dotraplinkage, ... */
+#include <asm/pgalloc.h> /* pgd_*(), ... */
/*
* Page fault error code bits:
> >
> > I assume you are aware that when you minimize the # of include
> > file in the various .c files, then you implicitly add dependency
> > on the includes the individual .h files have.
>
> Yes. Look at the commit -tip that does the above change (also
> attached below):
>
> a2bcd47: x86/mm: further cleanups of fault.c's include file section
>
> that commit uncovered a masked-until-then dependency bug in one of
> the x86 include files.
We have plenty of header files that needs other header files to be
included in order to build.
I tried to do a simple build check with a i386 defconfig.
It revealed that the appended list of header files did not build.
What I did was to create a simple:
module.h.c file containing:
#include <linux/module.h>
And then I used kbuild to build the file,
adding "obj-y += module.h.o" to Kbuild.
I did so for all header files in include/linux/
and result was that 172 header files failed to build.
Some of these was expected since they are not supposed to be
included direct. But the rest should build have
build with no errors?
I can automate the above included some way to say which header files
we should NOT build.
But eliminating the includes that are not needed is a bigger challenge
and also the biggest win :-(
Is it worthwhile to make the header files buildable alone?
Sam
include/linux/ac97_codec.h
include/linux/adfs_fs.h
include/linux/adfs_fs_i.h
include/linux/adfs_fs_sb.h
include/linux/arcdevice.h
include/linux/atalk.h
include/linux/ata_platform.h
include/linux/ath9k_platform.h
include/linux/atmel_pwm.h
include/linux/atm_zatm.h
include/linux/bit_spinlock.h
include/linux/bsg.h
include/linux/cgroup_subsys.h
include/linux/chio.h
include/linux/coda_fs_i.h
include/linux/coda_psdev.h
include/linux/compiler-gcc3.h
include/linux/compiler-gcc4.h
include/linux/compiler-gcc.h
include/linux/compiler-intel.h
include/linux/console_struct.h
include/linux/cramfs_fs_sb.h
include/linux/cryptohash.h
include/linux/cyclades.h
include/linux/cycx_drv.h
include/linux/cycx_x25.h
include/linux/dca.h
include/linux/dio.h
include/linux/dirent.h
include/linux/dlm_netlink.h
include/linux/dlm_plock.h
include/linux/dm9000.h
include/linux/eeprom_93cx6.h
include/linux/efs_vh.h
include/linux/elfcore-compat.h
include/linux/eventpoll.h
include/linux/ext2_fs.h
include/linux/ext3_fs.h
include/linux/ext3_fs_i.h
include/linux/ext3_fs_sb.h
include/linux/f75375s.h
include/linux/fault-inject.h
include/linux/fiemap.h
include/linux/filter.h
include/linux/flat.h
include/linux/fscache-cache.h
include/linux/fs_struct.h
include/linux/fs_uart_pd.h
include/linux/genalloc.h
include/linux/hayesesp.h
include/linux/hid-debug.h
include/linux/hiddev.h
include/linux/hpet.h
include/linux/hp_sdc.h
include/linux/htirq.h
include/linux/hwmon-sysfs.h
include/linux/hwmon-vid.h
include/linux/i2c-ocores.h
include/linux/i2c-pnx.h
include/linux/ibmtr.h
include/linux/if_ec.h
include/linux/if_ppp.h
include/linux/if_tunnel.h
include/linux/init_ohci1394_dma.h
include/linux/ioc3.h
include/linux/iommu.h
include/linux/iommu-helper.h
include/linux/ip6_tunnel.h
include/linux/ipv6_route.h
include/linux/irda.h
include/linux/irq_cpustat.h
include/linux/iscsi_ibft.h
include/linux/isdn_ppp.h
include/linux/istallion.h
include/linux/ixjuser.h
include/linux/jhash.h
include/linux/kdev_t.h
include/linux/kernelcapi.h
include/linux/kmalloc_sizes.h
include/linux/kvm_host.h
include/linux/kvm_para.h
include/linux/leds-bd2802.h
include/linux/leds_pwm.h
include/linux/libps2.h
include/linux/license.h
include/linux/llc.h
include/linux/lmb.h
include/linux/lzo.h
include/linux/maple.h
include/linux/mbcache.h
include/linux/mbus.h
include/linux/mc6821.h
include/linux/mm_inline.h
include/linux/mpage.h
include/linux/mroute6.h
include/linux/mv643xx_eth.h
include/linux/ncp_fs_i.h
include/linux/netrom.h
include/linux/nfs4_mount.h
include/linux/nfs_fs_sb.h
include/linux/nfs.h
include/linux/nfs_page.h
include/linux/nfs_xdr.h
include/linux/nilfs2_fs.h
include/linux/nls.h
include/linux/n_r3964.h
include/linux/nsc_gpio.h
include/linux/nubus.h
include/linux/of_device.h
include/linux/of.h
include/linux/of_platform.h
include/linux/of_spi.h
include/linux/page_cgroup.h
include/linux/pagevec.h
include/linux/parport_pc.h
include/linux/parser.h
include/linux/patchkey.h
include/linux/pci-acpi.h
include/linux/pcieport_if.h
include/linux/pci_hotplug.h
include/linux/pda_power.h
include/linux/phonet.h
include/linux/phy_fixed.h
include/linux/pipe_fs_i.h
include/linux/pm_wakeup.h
include/linux/prio_tree.h
include/linux/qnx4_fs.h
include/linux/ramfs.h
include/linux/rcuclassic.h
include/linux/rcupreempt.h
include/linux/reiserfs_fs_i.h
include/linux/reiserfs_fs_sb.h
include/linux/reiserfs_xattr.h
include/linux/romfs_fs.h
include/linux/rose.h
include/linux/rwsem-spinlock.h
include/linux/scc.h
include/linux/scx200_gpio.h
include/linux/sdla.h
include/linux/selection.h
include/linux/selinux.h
include/linux/serial167.h
include/linux/serialP.h
include/linux/sh_intc.h
include/linux/signalfd.h
include/linux/slab_def.h
include/linux/slob_def.h
include/linux/slub_def.h
include/linux/smb_fs_sb.h
include/linux/smb_mount.h
include/linux/spinlock_api_smp.h
include/linux/spinlock_api_up.h
include/linux/spinlock_types_up.h
include/linux/spinlock_up.h
include/linux/splice.h
include/linux/srcu.h
include/linux/stallion.h
include/linux/svga.h
include/linux/swapops.h
include/linux/sysv_fs.h
include/linux/task_io_accounting.h
include/linux/tick.h
include/linux/tty_driver.h
include/linux/tty_flip.h
include/linux/un.h
include/linux/usb_usual.h
include/linux/videotext.h
include/linux/virtio_net.h
include/linux/vmstat.h
include/linux/vt_buffer.h
include/linux/x25.h
include/linux/zorro.h
* Sam Ravnborg <[email protected]> wrote:
> > >
> > > I assume you are aware that when you minimize the # of include
> > > file in the various .c files, then you implicitly add dependency
> > > on the includes the individual .h files have.
> >
> > Yes. Look at the commit -tip that does the above change (also
> > attached below):
> >
> > a2bcd47: x86/mm: further cleanups of fault.c's include file section
> >
> > that commit uncovered a masked-until-then dependency bug in one of
> > the x86 include files.
>
> We have plenty of header files that needs other header files to be
> included in order to build.
>
> I tried to do a simple build check with a i386 defconfig.
> It revealed that the appended list of header files did not build.
>
> What I did was to create a simple:
>
> module.h.c file containing:
> #include <linux/module.h>
>
> And then I used kbuild to build the file,
> adding "obj-y += module.h.o" to Kbuild.
>
> I did so for all header files in include/linux/
> and result was that 172 header files failed to build.
>
> Some of these was expected since they are not supposed to be
> included direct. But the rest should build have
> build with no errors?
>
> I can automate the above included some way to say which header files
> we should NOT build.
> But eliminating the includes that are not needed is a bigger challenge
> and also the biggest win :-(
> Is it worthwhile to make the header files buildable alone?
I think it is definitely worthwile, for a couple of reasons:
- a data type definition that does not even build if included in a
.c file is broken, almost by definition. If it happens to build
in its current include file environment that's just a hidden bug,
nothing more.
- having such a check would simplify header dependency hell removal
efforts _immensely_. I could remove a suspect looking #include
line from a header and build with your check - that would be a
pretty good mechanism to drive header file simplification
efforts. (Human review goes only so far and it has failed to
reach its goal in the past 15 years, as it only addressed the
most obvious excesses.)
If we are too broken right now then maybe make it only available to
someone explicitly interested in it, via make
CONFIG_DEBUG_HEADER_DEPENDENCIES=y or so?
Ingo