public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* RFC: cleaning up the in-kernel headers
@ 2006-07-11 16:06 Adrian Bunk
  2006-07-11 16:28 ` David Woodhouse
                   ` (5 more replies)
  0 siblings, 6 replies; 26+ messages in thread
From: Adrian Bunk @ 2006-07-11 16:06 UTC (permalink / raw)
  To: linux-kernel; +Cc: David Woodhouse, torvalds, akpm

I'd like to cleanup the mess of the in-kernel headers, based on the 
following rules:
- every header should #include everything it uses
- remove unneeded #include's from headers

This would also remove all the implicit rules "before #include'ing 
header foo.h, you must #include header bar.h" you usually only see when 
the compilation fails.

There might be exceptions (e.g. for avoiding circular #include's) but 
these would be special cases.

As a side effect, this might also lead to additional cleanups.

This might cause some breakages, but it should usually only be compile 
breakages I'll fix as soon as I see them (or anyone else reports them 
to me).

My plan is to create a git tree where I'll work on this that will be 
included in -mm.

Is this OK for everyone?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 16:06 RFC: cleaning up the in-kernel headers Adrian Bunk
@ 2006-07-11 16:28 ` David Woodhouse
  2006-07-11 17:33   ` Christoph Hellwig
  2006-07-11 22:24   ` Adrian Bunk
  2006-07-11 17:07 ` Dave Jones
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 26+ messages in thread
From: David Woodhouse @ 2006-07-11 16:28 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: linux-kernel, torvalds, akpm

On Tue, 2006-07-11 at 18:06 +0200, Adrian Bunk wrote:
> I'd like to cleanup the mess of the in-kernel headers, based on the 
> following rules:
> - every header should #include everything it uses
> - remove unneeded #include's from headers
> 
> This would also remove all the implicit rules "before #include'ing 
> header foo.h, you must #include header bar.h" you usually only see
> when the compilation fails.
> 
> There might be exceptions (e.g. for avoiding circular #include's) but 
> these would be special cases.

Seems eminently sensible. Please make sure you don't introduce
regressions in the output of 'make headers_install' by unconditionally
including files which don't exist in the export -- if something is only
_used_ within #ifdef __KERNEL__ then it should only be #included within
#ifdef __KERNEL__ too.

It would be nice in the general case if we could actually _compile_ each
header file, standalone. There may be some cases where that doesn't
work, but it's a useful goal in most cases, for bother exported headers
_and_ the in-kernel version. For the former case it would be nice to add
it to 'make headers_check' once it's realistic to do so.

I still think it would be quite nice if we could _eliminate_ some of
those ifdefs, to be honest -- but I'm not overly bothered about that
now, because 'make headers_install' works well enough.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 16:06 RFC: cleaning up the in-kernel headers Adrian Bunk
  2006-07-11 16:28 ` David Woodhouse
@ 2006-07-11 17:07 ` Dave Jones
  2006-07-11 17:15   ` Joshua Hudson
  2006-07-11 22:27   ` Adrian Bunk
  2006-07-11 19:05 ` Russell King
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 26+ messages in thread
From: Dave Jones @ 2006-07-11 17:07 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: linux-kernel, David Woodhouse, torvalds, akpm

On Tue, Jul 11, 2006 at 06:06:39PM +0200, Adrian Bunk wrote:
 > I'd like to cleanup the mess of the in-kernel headers, based on the 
 > following rules:
 > - every header should #include everything it uses
 > - remove unneeded #include's from headers
 > 
 > This would also remove all the implicit rules "before #include'ing 
 > header foo.h, you must #include header bar.h" you usually only see when 
 > the compilation fails.

You may want to add as a secondary goal, splitting up some of the
huge 3-headed monster include files like sched.h
(It's better than it used to be, but it still sucks, and that thing
#include's the world).  Worst, iirc module.h pulls it in, which means
everything built as a module is pulling in hundreds of includes
even though most of the time, it'll never use anything from the
indirect ones.

ghviz & hviz from http://www.kernel.org/pub/linux/kernel/people/acme/
are invaluable for eyeballing include dependancies btw.
They need graphviz installed, and run like so..

ghviz include/linux/sched.h 10

to produce a pretty graph.

 > There might be exceptions (e.g. for avoiding circular #include's) but 
 > these would be special cases.

In many cases, adding forward references is a lot cleaner than
adding dozens of indirect include dependancies.

		Dave

-- 
http://www.codemonkey.org.uk

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 17:07 ` Dave Jones
@ 2006-07-11 17:15   ` Joshua Hudson
  2006-07-11 18:19     ` Dave Jones
  2006-07-11 22:27   ` Adrian Bunk
  1 sibling, 1 reply; 26+ messages in thread
From: Joshua Hudson @ 2006-07-11 17:15 UTC (permalink / raw)
  To: Dave Jones, Adrian Bunk, linux-kernel, David Woodhouse, torvalds,
	akpm

On 7/11/06, Dave Jones <davej@redhat.com> wrote:
> On Tue, Jul 11, 2006 at 06:06:39PM +0200, Adrian Bunk wrote:
>  > I'd like to cleanup the mess of the in-kernel headers, based on the
>  > following rules:
>  > - every header should #include everything it uses
>  > - remove unneeded #include's from headers
>  >
>  > This would also remove all the implicit rules "before #include'ing
>  > header foo.h, you must #include header bar.h" you usually only see when
>  > the compilation fails.
>
> You may want to add as a secondary goal, splitting up some of the
> huge 3-headed monster include files like sched.h
> (It's better than it used to be, but it still sucks, and that thing
> #include's the world).  Worst, iirc module.h pulls it in, which means
> everything built as a module is pulling in hundreds of includes
> even though most of the time, it'll never use anything from the
> indirect ones.
If you pull this off, you can shave a lot off compile time as almost
every component #includes module.h

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 16:28 ` David Woodhouse
@ 2006-07-11 17:33   ` Christoph Hellwig
  2006-07-11 19:34     ` Sam Ravnborg
  2006-07-11 21:04     ` David Woodhouse
  2006-07-11 22:24   ` Adrian Bunk
  1 sibling, 2 replies; 26+ messages in thread
From: Christoph Hellwig @ 2006-07-11 17:33 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Adrian Bunk, linux-kernel, torvalds, akpm

On Tue, Jul 11, 2006 at 05:28:43PM +0100, David Woodhouse wrote:
> It would be nice in the general case if we could actually _compile_ each
> header file, standalone. There may be some cases where that doesn't
> work, but it's a useful goal in most cases, for bother exported headers
> _and_ the in-kernel version. For the former case it would be nice to add
> it to 'make headers_check' once it's realistic to do so.

That would be extremly valueable.  Maybe one of the kbuild gurus could
cook up a make checkheaders rule that does this?


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 17:15   ` Joshua Hudson
@ 2006-07-11 18:19     ` Dave Jones
  0 siblings, 0 replies; 26+ messages in thread
From: Dave Jones @ 2006-07-11 18:19 UTC (permalink / raw)
  To: Joshua Hudson; +Cc: Adrian Bunk, linux-kernel, David Woodhouse, torvalds, akpm

On Tue, Jul 11, 2006 at 10:15:59AM -0700, Joshua Hudson wrote:
 > On 7/11/06, Dave Jones <davej@redhat.com> wrote:
 > >On Tue, Jul 11, 2006 at 06:06:39PM +0200, Adrian Bunk wrote:
 > > > I'd like to cleanup the mess of the in-kernel headers, based on the
 > > > following rules:
 > > > - every header should #include everything it uses
 > > > - remove unneeded #include's from headers
 > > >
 > > > This would also remove all the implicit rules "before #include'ing
 > > > header foo.h, you must #include header bar.h" you usually only see when
 > > > the compilation fails.
 > >
 > >You may want to add as a secondary goal, splitting up some of the
 > >huge 3-headed monster include files like sched.h
 > >(It's better than it used to be, but it still sucks, and that thing
 > >#include's the world).  Worst, iirc module.h pulls it in, which means
 > >everything built as a module is pulling in hundreds of includes
 > >even though most of the time, it'll never use anything from the
 > >indirect ones.
 > If you pull this off, you can shave a lot off compile time as almost
 > every component #includes module.h

I did it on at least two occasions in the past, and someone else picked
it up and retried submitting it a third time, but it never got merged.
The problem with changes of this scale is they tend to be wide-reaching,
and impact areas which other people are patching, so it's something
that needs to go in as soon as the patch is done, or it becomes stale
very quickly.

It did make a noticable difference to compiles, though I forget the
exact numbers.   Should be in the list archives somewhere.
I have vague recollection that it shaved off around 20-30s, though
my memory is fuzzy.

		Dave

-- 
http://www.codemonkey.org.uk

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 16:06 RFC: cleaning up the in-kernel headers Adrian Bunk
  2006-07-11 16:28 ` David Woodhouse
  2006-07-11 17:07 ` Dave Jones
@ 2006-07-11 19:05 ` Russell King
  2006-07-11 22:19   ` Adrian Bunk
  2006-07-11 20:26 ` [PATCH] Fix broken kernel headers preventing ARM build Russell King
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 26+ messages in thread
From: Russell King @ 2006-07-11 19:05 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: linux-kernel, David Woodhouse, torvalds, akpm

On Tue, Jul 11, 2006 at 06:06:39PM +0200, Adrian Bunk wrote:
> My plan is to create a git tree where I'll work on this that will be 
> included in -mm.
> 
> Is this OK for everyone?

Sure, provided it's also tested on non-x86 architectures as well.

At the moment, someone did a "clean up" of linux/tty.h includes
which has broken the 99% of the ARM defconfigs, and despite this
patch being in -mm and allegedly fixed by akpm for ARM, the result
is still massive breakage.

Hence, -mm is probably far too different from mainline for include
cleanups to be properly tested before they're sent to Linus - iow
they need an additional level of testing against Linus' tree _prior_
to being submitted to Linus.

Apart from that, I've no problem with anyone who wants to clean up
the include mess.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 17:33   ` Christoph Hellwig
@ 2006-07-11 19:34     ` Sam Ravnborg
  2006-07-11 19:41       ` Sam Ravnborg
  2006-07-11 21:04     ` David Woodhouse
  1 sibling, 1 reply; 26+ messages in thread
From: Sam Ravnborg @ 2006-07-11 19:34 UTC (permalink / raw)
  To: Christoph Hellwig, David Woodhouse, Adrian Bunk, linux-kernel,
	torvalds, akpm

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 721 bytes --]

On Tue, Jul 11, 2006 at 06:33:01PM +0100, Christoph Hellwig wrote:
> On Tue, Jul 11, 2006 at 05:28:43PM +0100, David Woodhouse wrote:
> > It would be nice in the general case if we could actually _compile_ each
> > header file, standalone. There may be some cases where that doesn't
> > work, but it's a useful goal in most cases, for bother exported headers
> > _and_ the in-kernel version. For the former case it would be nice to add
> > it to 'make headers_check' once it's realistic to do so.
> 
> That would be extremly valueable.  Maybe one of the kbuild gurus could
> cook up a make checkheaders rule that does this?
JÃrn Engel IIRC created a perl scrip that did this a year or two ago.
Try googling a bit.

	Sam

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 19:34     ` Sam Ravnborg
@ 2006-07-11 19:41       ` Sam Ravnborg
  2006-07-11 20:41         ` Randy.Dunlap
  0 siblings, 1 reply; 26+ messages in thread
From: Sam Ravnborg @ 2006-07-11 19:41 UTC (permalink / raw)
  To: Christoph Hellwig, David Woodhouse, Adrian Bunk, linux-kernel,
	torvalds, akpm

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 135 bytes --]

> JÃrn Engel IIRC created a perl scrip that did this a year or two ago.
> Try googling a bit.
http://lkml.org/lkml/2003/10/1/74

	Sam

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH] Fix broken kernel headers preventing ARM build
  2006-07-11 16:06 RFC: cleaning up the in-kernel headers Adrian Bunk
                   ` (2 preceding siblings ...)
  2006-07-11 19:05 ` Russell King
@ 2006-07-11 20:26 ` Russell King
  2006-07-13 19:05 ` RFC: cleaning up the in-kernel headers Christoph Lameter
  2006-07-14  0:11 ` David Woodhouse
  5 siblings, 0 replies; 26+ messages in thread
From: Russell King @ 2006-07-11 20:26 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, David Woodhouse, Adrian Bunk

And this can be the first of the series.  Note that this is NOT a
cleanup, but a build fix, so _is_ immediate -rc1 material.
-----

As a result of 894673ee6122a3ce1958e1fe096901ba5356a96b, the ARM
architecture is more or less unbuildable - only one defconfig appears
to build, with all others erroring out with:

  CC      arch/arm/kernel/setup.o
In file included from /home/rmk/git/linux-2.6-rmk/arch/arm/kernel/setup.c:22:
/home/rmk/git/linux-2.6-rmk/include/linux/root_dev.h:7: warning: implicit declaration of function `MKDEV'
/home/rmk/git/linux-2.6-rmk/include/linux/root_dev.h:7: error: enumerator value for `Root_NFS' not integer constant
/home/rmk/git/linux-2.6-rmk/include/linux/root_dev.h:8: error: enumerator value for `Root_RAM0' not integer constant
/home/rmk/git/linux-2.6-rmk/include/linux/root_dev.h:9: error: enumerator value for `Root_RAM1' not integer constant
/home/rmk/git/linux-2.6-rmk/include/linux/root_dev.h:10: error: enumerator value for `Root_FD0' not integer constant
/home/rmk/git/linux-2.6-rmk/include/linux/root_dev.h:11: error: enumerator value for `Root_HDA1' not integer constant
/home/rmk/git/linux-2.6-rmk/include/linux/root_dev.h:12: error: enumerator value for `Root_HDA2' not integer constant
/home/rmk/git/linux-2.6-rmk/include/linux/root_dev.h:13: error: enumerator value for `Root_SDA1' not integer constant
/home/rmk/git/linux-2.6-rmk/include/linux/root_dev.h:14: error: enumerator value for `Root_SDA2' not integer constant
/home/rmk/git/linux-2.6-rmk/include/linux/root_dev.h:15: error: enumerator value for `Root_HDC1' not integer constant
/home/rmk/git/linux-2.6-rmk/include/linux/root_dev.h:16: error: enumerator value for `Root_SR0' not integer constant
make[2]: *** [arch/arm/kernel/setup.o] Error 1
make[1]: *** [arch/arm/kernel] Error 2
make: *** [_all] Error 2

Essentially, root_dev.h uses MKDEV and dev_t, but does not include any
headers which provide either of these definitions.  The reason it worked
previously is that linux/tty.h just happened to include the required
headers for linux/root_dev.h.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

---

 include/linux/root_dev.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/root_dev.h b/include/linux/root_dev.h
index ea4bc9d..ed241aa 100644
--- a/include/linux/root_dev.h
+++ b/include/linux/root_dev.h
@@ -2,6 +2,8 @@ #ifndef _ROOT_DEV_H_
 #define _ROOT_DEV_H_
 
 #include <linux/major.h>
+#include <linux/types.h>
+#include <linux/kdev_t.h>
 
 enum {
 	Root_NFS = MKDEV(UNNAMED_MAJOR, 255),


-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 19:41       ` Sam Ravnborg
@ 2006-07-11 20:41         ` Randy.Dunlap
  2006-07-11 21:37           ` Jörn Engel
  0 siblings, 1 reply; 26+ messages in thread
From: Randy.Dunlap @ 2006-07-11 20:41 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: hch, dwmw2, bunk, linux-kernel, torvalds, akpm

On Tue, 11 Jul 2006 21:41:07 +0200 Sam Ravnborg wrote:

> > JÃrn Engel IIRC created a perl scrip that did this a year or two ago.
> > Try googling a bit.
> http://lkml.org/lkml/2003/10/1/74

That is version 2 of the script.  There are also versions 3 & 4.

http://marc.theaimsgroup.com/?l=linux-kernel&w=2&r=1&s=check+headers+for+complete+includes&q=t

---
~Randy

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 17:33   ` Christoph Hellwig
  2006-07-11 19:34     ` Sam Ravnborg
@ 2006-07-11 21:04     ` David Woodhouse
  1 sibling, 0 replies; 26+ messages in thread
From: David Woodhouse @ 2006-07-11 21:04 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Adrian Bunk, linux-kernel, torvalds, akpm

On Tue, 2006-07-11 at 18:33 +0100, Christoph Hellwig wrote:
> On Tue, Jul 11, 2006 at 05:28:43PM +0100, David Woodhouse wrote:
> > It would be nice in the general case if we could actually _compile_ each
> > header file, standalone. There may be some cases where that doesn't
> > work, but it's a useful goal in most cases, for bother exported headers
> > _and_ the in-kernel version. For the former case it would be nice to add
> > it to 'make headers_check' once it's realistic to do so.
> 
> That would be extremly valueable.  Maybe one of the kbuild gurus could
> cook up a make checkheaders rule that does this? 

We already have it for _exported_ headers -- run 'make headers_check'.
It doesn't do much yet though.

-- 
dwmw2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 20:41         ` Randy.Dunlap
@ 2006-07-11 21:37           ` Jörn Engel
  2006-07-11 22:01             ` H. Peter Anvin
  2006-07-11 22:40             ` Adrian Bunk
  0 siblings, 2 replies; 26+ messages in thread
From: Jörn Engel @ 2006-07-11 21:37 UTC (permalink / raw)
  To: Randy.Dunlap; +Cc: Sam Ravnborg, hch, dwmw2, bunk, linux-kernel, torvalds, akpm

On Tue, 11 July 2006 13:41:06 -0700, Randy.Dunlap wrote:
> On Tue, 11 Jul 2006 21:41:07 +0200 Sam Ravnborg wrote:
> 
> > > JÃrn Engel IIRC created a perl scrip that did this a year or two ago.
> > > Try googling a bit.
> > http://lkml.org/lkml/2003/10/1/74
> 
> That is version 2 of the script.  There are also versions 3 & 4.
> 
> http://marc.theaimsgroup.com/?l=linux-kernel&w=2&r=1&s=check+headers+for+complete+includes&q=t

Boy, it took me a while to remember what I did back then.  In
principle, the script just compiles trivial c files with a single
#include <linux/foo.h>
inside.

Not too bad in principle, but there were two problems I couldn't
solve:
1. One of the goals should be to make a compile faster, not slower.
Adding further includes hardly helps.
2. It is practically impossible to test every possible combination of
#ifdefs in the various headers pulled in.

Problem 1 would be fairly simple, as the script could be changed
slightly to prune single #include lines from headers and see if they
still compile.  But then there is problem 2.

One possibility to tackly problem 2 would be to create a set of config
files to use for automatic testing.  Instead of compiling once, a
header must get compiled with every config in the set.  Hopefully this
set would not grow too big.

And in the end, there is just so much you can automate.  Humans have
to think about the changes before sending patches.

Jörn

-- 
Optimizations always bust things, because all optimizations are, in
the long haul, a form of cheating, and cheaters eventually get caught.
-- Larry Wall

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 21:37           ` Jörn Engel
@ 2006-07-11 22:01             ` H. Peter Anvin
  2006-07-11 22:20               ` Jörn Engel
  2006-07-11 22:39               ` Linus Torvalds
  2006-07-11 22:40             ` Adrian Bunk
  1 sibling, 2 replies; 26+ messages in thread
From: H. Peter Anvin @ 2006-07-11 22:01 UTC (permalink / raw)
  To: Jörn Engel
  Cc: Randy.Dunlap, Sam Ravnborg, hch, dwmw2, bunk, linux-kernel,
	torvalds, akpm

Jörn Engel wrote:
> On Tue, 11 July 2006 13:41:06 -0700, Randy.Dunlap wrote:
>> On Tue, 11 Jul 2006 21:41:07 +0200 Sam Ravnborg wrote:
>>
>>>> JÃrn Engel IIRC created a perl scrip that did this a year or two ago.
>>>> Try googling a bit.
>>> http://lkml.org/lkml/2003/10/1/74
>> That is version 2 of the script.  There are also versions 3 & 4.
>>
>> http://marc.theaimsgroup.com/?l=linux-kernel&w=2&r=1&s=check+headers+for+complete+includes&q=t
> 
> Boy, it took me a while to remember what I did back then.  In
> principle, the script just compiles trivial c files with a single
> #include <linux/foo.h>
> inside.
> 
> Not too bad in principle, but there were two problems I couldn't
> solve:
> 1. One of the goals should be to make a compile faster, not slower.
> Adding further includes hardly helps.
> 2. It is practically impossible to test every possible combination of
> #ifdefs in the various headers pulled in.
> 

#1 I doubt the time taken to look at include files that are #ifndef'd in 
their entirety is significant (I think there is special code in gcc to 
handle this case fast.)

#2 is actually a non-issue.  If each file is usable standalone (and have 
a multiple inclusion guard), then the include order shouldn't matter. 
Not that one can't create contrived cases where it would matter, but one 
can't solve every problem...

	-hpa

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 19:05 ` Russell King
@ 2006-07-11 22:19   ` Adrian Bunk
  0 siblings, 0 replies; 26+ messages in thread
From: Adrian Bunk @ 2006-07-11 22:19 UTC (permalink / raw)
  To: linux-kernel, David Woodhouse, torvalds, akpm

On Tue, Jul 11, 2006 at 08:05:44PM +0100, Russell King wrote:
> On Tue, Jul 11, 2006 at 06:06:39PM +0200, Adrian Bunk wrote:
> > My plan is to create a git tree where I'll work on this that will be 
> > included in -mm.
> > 
> > Is this OK for everyone?
> 
> Sure, provided it's also tested on non-x86 architectures as well.
>...

Sure.

There might still be compile errors in some configurations, but I'll 
test this stuff on several architectures.

> Russell King

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 22:01             ` H. Peter Anvin
@ 2006-07-11 22:20               ` Jörn Engel
  2006-07-11 22:39               ` Linus Torvalds
  1 sibling, 0 replies; 26+ messages in thread
From: Jörn Engel @ 2006-07-11 22:20 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Randy.Dunlap, Sam Ravnborg, hch, dwmw2, bunk, linux-kernel,
	torvalds, akpm

On Tue, 11 July 2006 15:01:21 -0700, H. Peter Anvin wrote:
> Jörn Engel wrote:
> >
> >Not too bad in principle, but there were two problems I couldn't
> >solve:
> >1. One of the goals should be to make a compile faster, not slower.
> >Adding further includes hardly helps.
> >2. It is practically impossible to test every possible combination of
> >#ifdefs in the various headers pulled in.
> >
> 
> #1 I doubt the time taken to look at include files that are #ifndef'd in 
> their entirety is significant (I think there is special code in gcc to 
> handle this case fast.)

Quoting Dave Jones from a few mails down the thread:
It did make a noticable difference to compiles, though I forget the
exact numbers.   Should be in the list archives somewhere.
I have vague recollection that it shaved off around 20-30s, though
my memory is fuzzy.

> #2 is actually a non-issue.  If each file is usable standalone (and have 
> a multiple inclusion guard), then the include order shouldn't matter. 
> Not that one can't create contrived cases where it would matter, but one 
> can't solve every problem...

#2 was not about include order, but about things like CONFIG_SMP,
different arches, etc.  As Russell King showed, breaking Arm is easier
than many people think.

Jörn

-- 
Don't patch bad code, rewrite it.
-- Kernigham and Pike, according to Rusty

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 16:28 ` David Woodhouse
  2006-07-11 17:33   ` Christoph Hellwig
@ 2006-07-11 22:24   ` Adrian Bunk
  1 sibling, 0 replies; 26+ messages in thread
From: Adrian Bunk @ 2006-07-11 22:24 UTC (permalink / raw)
  To: David Woodhouse; +Cc: linux-kernel, torvalds, akpm

On Tue, Jul 11, 2006 at 05:28:43PM +0100, David Woodhouse wrote:
> On Tue, 2006-07-11 at 18:06 +0200, Adrian Bunk wrote:
> > I'd like to cleanup the mess of the in-kernel headers, based on the 
> > following rules:
> > - every header should #include everything it uses
> > - remove unneeded #include's from headers
> > 
> > This would also remove all the implicit rules "before #include'ing 
> > header foo.h, you must #include header bar.h" you usually only see
> > when the compilation fails.
> > 
> > There might be exceptions (e.g. for avoiding circular #include's) but 
> > these would be special cases.
> 
> Seems eminently sensible. Please make sure you don't introduce
> regressions in the output of 'make headers_install' by unconditionally
> including files which don't exist in the export -- if something is only
> _used_ within #ifdef __KERNEL__ then it should only be #included within
> #ifdef __KERNEL__ too.

Sure, I have the userspace headers in mind, too.

> It would be nice in the general case if we could actually _compile_ each
> header file, standalone. There may be some cases where that doesn't
> work, but it's a useful goal in most cases, for bother exported headers
> _and_ the in-kernel version. For the former case it would be nice to add
> it to 'make headers_check' once it's realistic to do so.

That's what I meant with "every header should #include everything it 
uses".

Unfortunately, compiling alone is not enough due to:
- different config options affecting a header
- code only used in a #define

I got a problem caused by the combination of both just a few days ago...

>...
> dwmw2

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 17:07 ` Dave Jones
  2006-07-11 17:15   ` Joshua Hudson
@ 2006-07-11 22:27   ` Adrian Bunk
  1 sibling, 0 replies; 26+ messages in thread
From: Adrian Bunk @ 2006-07-11 22:27 UTC (permalink / raw)
  To: Dave Jones, linux-kernel, David Woodhouse, torvalds, akpm

On Tue, Jul 11, 2006 at 01:07:25PM -0400, Dave Jones wrote:
>...
> ghviz & hviz from http://www.kernel.org/pub/linux/kernel/people/acme/
> are invaluable for eyeballing include dependancies btw.
> They need graphviz installed, and run like so..
> 
> ghviz include/linux/sched.h 10
> 
> to produce a pretty graph.

I'll need something like this. It's not exactly what I have in mind but 
a good starting point.

>...
> 		Dave

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 22:01             ` H. Peter Anvin
  2006-07-11 22:20               ` Jörn Engel
@ 2006-07-11 22:39               ` Linus Torvalds
  1 sibling, 0 replies; 26+ messages in thread
From: Linus Torvalds @ 2006-07-11 22:39 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Jörn Engel, Randy.Dunlap, Sam Ravnborg, hch, dwmw2, bunk,
	linux-kernel, akpm



On Tue, 11 Jul 2006, H. Peter Anvin wrote:
> 
> #1 I doubt the time taken to look at include files that are #ifndef'd in their
> entirety is significant (I think there is special code in gcc to handle this
> case fast.)

Correct. Don't worry about multiple includes.

HOWEVER. If you can avoid the include entirely, that can be an absolutely 
huge timesaver. Much of the compile time is from just parsing the things 
the first time around (rather than parsing it over and over), and if you 
don't strictly need a header, avoiding parsing it in the first place is a 
big big issue.

For example, if a header only needs a structure to be declared because it 
uses a pointer to it, then rather than including the header file that 
declares that structure fully, just doing a

	struct task_struct;

to say that "there is such a thing as a 'struct task_struct'" can be a 
huge time-saver.

Of course, anything that actually needs to look past the pointer will need 
to get the full declaration..

		Linus

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 21:37           ` Jörn Engel
  2006-07-11 22:01             ` H. Peter Anvin
@ 2006-07-11 22:40             ` Adrian Bunk
  1 sibling, 0 replies; 26+ messages in thread
From: Adrian Bunk @ 2006-07-11 22:40 UTC (permalink / raw)
  To: Jörn Engel
  Cc: Randy.Dunlap, Sam Ravnborg, hch, dwmw2, linux-kernel, torvalds,
	akpm

On Tue, Jul 11, 2006 at 11:37:33PM +0200, Jörn Engel wrote:
> On Tue, 11 July 2006 13:41:06 -0700, Randy.Dunlap wrote:
> > On Tue, 11 Jul 2006 21:41:07 +0200 Sam Ravnborg wrote:
> > 
> > > > JÃrn Engel IIRC created a perl scrip that did this a year or two ago.
> > > > Try googling a bit.
> > > http://lkml.org/lkml/2003/10/1/74
> > 
> > That is version 2 of the script.  There are also versions 3 & 4.
> > 
> > http://marc.theaimsgroup.com/?l=linux-kernel&w=2&r=1&s=check+headers+for+complete+includes&q=t
> 
> Boy, it took me a while to remember what I did back then.  In
> principle, the script just compiles trivial c files with a single
> #include <linux/foo.h>
> inside.

Sure, but it seems quite useful for this task.

> Not too bad in principle, but there were two problems I couldn't
> solve:
> 1. One of the goals should be to make a compile faster, not slower.
> Adding further includes hardly helps.

For me, this is not a goal.

It might perhaps be a side effect, but I care about correctness, not 
about compile time.

> 2. It is practically impossible to test every possible combination of
> #ifdefs in the various headers pulled in.

It's not perfect, but it finds a subset of the problems.

There also other possible problems like stuff only used in a #define.

The perfect is the enemy of the good.

> Problem 1 would be fairly simple, as the script could be changed
> slightly to prune single #include lines from headers and see if they
> still compile.  But then there is problem 2.
> 
> One possibility to tackly problem 2 would be to create a set of config
> files to use for automatic testing.  Instead of compiling once, a
> header must get compiled with every config in the set.  Hopefully this
> set would not grow too big.
> 
> And in the end, there is just so much you can automate.  Humans have
> to think about the changes before sending patches.

i don't think the header cleanup could be completely automated.

But if automated tools help that's a good thing.

> Jörn

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 16:06 RFC: cleaning up the in-kernel headers Adrian Bunk
                   ` (3 preceding siblings ...)
  2006-07-11 20:26 ` [PATCH] Fix broken kernel headers preventing ARM build Russell King
@ 2006-07-13 19:05 ` Christoph Lameter
  2006-07-15  4:18   ` Steven Rostedt
  2006-07-14  0:11 ` David Woodhouse
  5 siblings, 1 reply; 26+ messages in thread
From: Christoph Lameter @ 2006-07-13 19:05 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: linux-kernel, David Woodhouse, torvalds, akpm

On Tue, 11 Jul 2006, Adrian Bunk wrote:

> This would also remove all the implicit rules "before #include'ing 
> header foo.h, you must #include header bar.h" you usually only see when 
> the compilation fails.
> 
> There might be exceptions (e.g. for avoiding circular #include's) but 
> these would be special cases.

Great! Yes please. There is also some weirdness in #include in the 
middle of another .h file (see mm.h). It would be great if you could find 
a solution.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-11 16:06 RFC: cleaning up the in-kernel headers Adrian Bunk
                   ` (4 preceding siblings ...)
  2006-07-13 19:05 ` RFC: cleaning up the in-kernel headers Christoph Lameter
@ 2006-07-14  0:11 ` David Woodhouse
  5 siblings, 0 replies; 26+ messages in thread
From: David Woodhouse @ 2006-07-14  0:11 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: linux-kernel, torvalds, akpm

On Tue, 2006-07-11 at 18:06 +0200, Adrian Bunk wrote:
> I'd like to cleanup the mess of the in-kernel headers, based on the 
> following rules:
> - every header should #include everything it uses
> - remove unneeded #include's from headers 

If you also fancy removing gratuitous instances of #ifdef __KERNEL__,
that might be useful...

$ for a in `grep -rl __KERNEL__ include` ; do DIR=`dirname $a` ;
NAME=`basename $a` ; grep -q $NAME $DIR/Kbuild 2>/dev/null || grep -q
$NAME include/asm-generic/Kbuild.asm || echo $DIR/$NAME ; done | wc -l
481


-- 
dwmw2


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-13 19:05 ` RFC: cleaning up the in-kernel headers Christoph Lameter
@ 2006-07-15  4:18   ` Steven Rostedt
  2006-07-15  4:59     ` Christoph Lameter
  0 siblings, 1 reply; 26+ messages in thread
From: Steven Rostedt @ 2006-07-15  4:18 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Adrian Bunk, linux-kernel, David Woodhouse, torvalds, akpm

On Thu, 2006-07-13 at 12:05 -0700, Christoph Lameter wrote:
> On Tue, 11 Jul 2006, Adrian Bunk wrote:
> 
> > This would also remove all the implicit rules "before #include'ing 
> > header foo.h, you must #include header bar.h" you usually only see when 
> > the compilation fails.
> > 
> > There might be exceptions (e.g. for avoiding circular #include's) but 
> > these would be special cases.
> 
> Great! Yes please. There is also some weirdness in #include in the 
> middle of another .h file (see mm.h). It would be great if you could find 
> a solution.

Are you talking about the page-flags.h or the vmstat.h?

The page-flags.h has a FIXME by it to remove it, but under Adrian's
rules, it seems that it should still belong.  The rule is that if foo.h
needs bar.h to compile, then foo.h should have bar.h in it. And just
seeing what happens if we remove page-flags.h from mm.h, we get a
compile error in mm.h. Which means that that page-flags.h belongs in
mm.h since it wont compile without it.

Now for the vmstat.h, I just tried removing that, and it seems that this
is a candidate to be removed from mm.h since mm.h compiles fine without
it. But vmstat.h doesn't compile without mm.h.  So it seems that we
should add mm.h to vmstat.h, remove vmstat.h from mm.h and for those .c
files that break, just add vmstat.h to them.

-- Steve



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-15  4:18   ` Steven Rostedt
@ 2006-07-15  4:59     ` Christoph Lameter
  2006-07-17  0:53       ` Steven Rostedt
  0 siblings, 1 reply; 26+ messages in thread
From: Christoph Lameter @ 2006-07-15  4:59 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Adrian Bunk, linux-kernel, David Woodhouse, torvalds, akpm

On Sat, 15 Jul 2006, Steven Rostedt wrote:

> Are you talking about the page-flags.h or the vmstat.h?

Both are troublesome. I forked off vmstat.hy from page-flags.h
and all my attempts to straigthen things out results in other problems
so I finally gave up and just did the same for vmstat.h.

> The page-flags.h has a FIXME by it to remove it, but under Adrian's
> rules, it seems that it should still belong.  The rule is that if foo.h
> needs bar.h to compile, then foo.h should have bar.h in it. And just
> seeing what happens if we remove page-flags.h from mm.h, we get a
> compile error in mm.h. Which means that that page-flags.h belongs in
> mm.h since it wont compile without it.

page-flags.h also seems to be included from the arches 
for various purposes. 

grep page-flags include/asm-*/*

include/asm-ia64/cacheflush.h:#include <linux/page-flags.h>
include/asm-ia64/pgalloc.h:#include <linux/page-flags.h>
include/asm-ia64/uaccess.h:#include <linux/page-flags.h>

grep page-flags arch/*/*/*
arch/s390/appldata/appldata_base.c:#include <linux/page-flags.h>

mm.h gets too big. It would be best if we had some smaller granularity and
page-flags.h should be pretty much stand on its own.


> Now for the vmstat.h, I just tried removing that, and it seems that this
> is a candidate to be removed from mm.h since mm.h compiles fine without
> it. But vmstat.h doesn't compile without mm.h.  So it seems that we
> should add mm.h to vmstat.h, remove vmstat.h from mm.h and for those .c
> files that break, just add vmstat.h to them.

Great if you can detangle that.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-15  4:59     ` Christoph Lameter
@ 2006-07-17  0:53       ` Steven Rostedt
  2006-07-20 10:56         ` Adrian Bunk
  0 siblings, 1 reply; 26+ messages in thread
From: Steven Rostedt @ 2006-07-17  0:53 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Adrian Bunk, linux-kernel, David Woodhouse, torvalds, akpm

On Fri, 2006-07-14 at 21:59 -0700, Christoph Lameter wrote:

> > Now for the vmstat.h, I just tried removing that, and it seems that this
> > is a candidate to be removed from mm.h since mm.h compiles fine without
> > it. But vmstat.h doesn't compile without mm.h.  So it seems that we
> > should add mm.h to vmstat.h, remove vmstat.h from mm.h and for those .c
> > files that break, just add vmstat.h to them.
> 
> Great if you can detangle that.

Are you supporting the effort if I send in patches that removes the
vmstat.h and then goes and tries to find all the places that fail to
compile because of the removal and adds vmstat.h directly, that the
patches would get accepted?

It would probably need to go into -mm for a bit just to find those
places I missed.

This wouldn't be a problem to do and can be accomplished rather quickly,
but I wont waste any time on it if it is doomed at the start.

-- Steve



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: RFC: cleaning up the in-kernel headers
  2006-07-17  0:53       ` Steven Rostedt
@ 2006-07-20 10:56         ` Adrian Bunk
  0 siblings, 0 replies; 26+ messages in thread
From: Adrian Bunk @ 2006-07-20 10:56 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Christoph Lameter, linux-kernel, David Woodhouse, torvalds, akpm

On Sun, Jul 16, 2006 at 08:53:50PM -0400, Steven Rostedt wrote:
> On Fri, 2006-07-14 at 21:59 -0700, Christoph Lameter wrote:
> 
> > > Now for the vmstat.h, I just tried removing that, and it seems that this
> > > is a candidate to be removed from mm.h since mm.h compiles fine without
> > > it. But vmstat.h doesn't compile without mm.h.  So it seems that we
> > > should add mm.h to vmstat.h, remove vmstat.h from mm.h and for those .c
> > > files that break, just add vmstat.h to them.
> > 
> > Great if you can detangle that.
> 
> Are you supporting the effort if I send in patches that removes the
> vmstat.h and then goes and tries to find all the places that fail to
> compile because of the removal and adds vmstat.h directly, that the
> patches would get accepted?
> 
> It would probably need to go into -mm for a bit just to find those
> places I missed.
> 
> This wouldn't be a problem to do and can be accomplished rather quickly,
> but I wont waste any time on it if it is doomed at the start.

It sounds good.

I've created a git tree [1] including all my cleanups work, and I'd also 
include your patch.

> -- Steve

cu
Adrian

[1] git://git.kernel.org/pub/scm/linux/kernel/git/bunk/hdrcleanup.git

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2006-07-20 10:56 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-07-11 16:06 RFC: cleaning up the in-kernel headers Adrian Bunk
2006-07-11 16:28 ` David Woodhouse
2006-07-11 17:33   ` Christoph Hellwig
2006-07-11 19:34     ` Sam Ravnborg
2006-07-11 19:41       ` Sam Ravnborg
2006-07-11 20:41         ` Randy.Dunlap
2006-07-11 21:37           ` Jörn Engel
2006-07-11 22:01             ` H. Peter Anvin
2006-07-11 22:20               ` Jörn Engel
2006-07-11 22:39               ` Linus Torvalds
2006-07-11 22:40             ` Adrian Bunk
2006-07-11 21:04     ` David Woodhouse
2006-07-11 22:24   ` Adrian Bunk
2006-07-11 17:07 ` Dave Jones
2006-07-11 17:15   ` Joshua Hudson
2006-07-11 18:19     ` Dave Jones
2006-07-11 22:27   ` Adrian Bunk
2006-07-11 19:05 ` Russell King
2006-07-11 22:19   ` Adrian Bunk
2006-07-11 20:26 ` [PATCH] Fix broken kernel headers preventing ARM build Russell King
2006-07-13 19:05 ` RFC: cleaning up the in-kernel headers Christoph Lameter
2006-07-15  4:18   ` Steven Rostedt
2006-07-15  4:59     ` Christoph Lameter
2006-07-17  0:53       ` Steven Rostedt
2006-07-20 10:56         ` Adrian Bunk
2006-07-14  0:11 ` David Woodhouse

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox