2002-01-16 00:05:32

by John Weber

[permalink] [raw]
Subject: 2.5.3-pre1 compile error

gcc -D__KERNEL__ -I/usr/src/linux-2.5.2/include -Wall
-Wstrict-prototypes -Wno-trigraphs -O2 -fomit-frame-pointer
-fno-strict-aliasing -fno-common -pipe -mpreferred-stack-boundary=2
-march=i686 -c -o read_write.o read_write.c
In file included from read_write.c:10:
/usr/src/linux-2.5.2/include/linux/file.h:25: parse error before `atomic_t'
/usr/src/linux-2.5.2/include/linux/file.h:25: warning: no semicolon at
end of struct or union
/usr/src/linux-2.5.2/include/linux/file.h:36: parse error before `}'
/usr/src/linux-2.5.2/include/linux/file.h: In function `fcheck_files':
/usr/src/linux-2.5.2/include/linux/file.h:50: dereferencing pointer to
incomplete type
/usr/src/linux-2.5.2/include/linux/file.h:51: dereferencing pointer to
incomplete type
In file included from read_write.c:13:
/usr/src/linux-2.5.2/include/linux/dnotify.h: At top level:
/usr/src/linux-2.5.2/include/linux/dnotify.h:18: warning: `struct inode'
declared inside parameter list
/usr/src/linux-2.5.2/include/linux/dnotify.h:18: warning: its scope is
only this definition or declaration, which is probably not what you want.
/usr/src/linux-2.5.2/include/linux/dnotify.h:21: warning: `struct inode'
declared inside parameter list
/usr/src/linux-2.5.2/include/linux/dnotify.h: In function
`inode_dir_notify':
/usr/src/linux-2.5.2/include/linux/dnotify.h:23: dereferencing pointer
to incomplete type
/usr/src/linux-2.5.2/include/linux/dnotify.h:24: warning: passing arg 1
of `__inode_dir_notify' from incompatible pointer type
read_write.c: In function `sys_read':
read_write.c:167: warning: passing arg 1 of `inode_dir_notify' from
incompatible pointer type
read_write.c: In function `sys_write':
read_write.c:194: warning: passing arg 1 of `inode_dir_notify' from
incompatible pointer type
read_write.c: In function `do_readv_writev':
read_write.c:299: warning: passing arg 1 of `inode_dir_notify' from
incompatible pointer type
read_write.c: In function `sys_pread':
read_write.c:371: warning: passing arg 1 of `inode_dir_notify' from
incompatible pointer type
read_write.c: In function `sys_pwrite':
read_write.c:403: warning: passing arg 1 of `inode_dir_notify' from
incompatible pointer type
make[2]: *** [read_write.o] Error 1
make[2]: Leaving directory `/usr/src/linux-2.5.2/fs'
make[1]: *** [first_rule] Error 2
make[1]: Leaving directory `/usr/src/linux-2.5.2/fs'
make: *** [_dir_fs] Error 2


2002-01-16 00:21:07

by Benjamin LaHaise

[permalink] [raw]
Subject: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, Jan 15, 2002 at 07:05:10PM -0500, John Weber wrote:
> incompatible pointer type
> read_write.c: In function `sys_pwrite':
> read_write.c:403: warning: passing arg 1 of `inode_dir_notify' from
> incompatible pointer type

Hmm, this should fix that.

-ben

:r ~/patches/v2.5.3-pre1-file_fix.diff
diff -urN v2.5.3-pre1/include/linux/file.h v2.5.3-pre1-fix/include/linux/file.h
--- v2.5.3-pre1/include/linux/file.h Tue Jan 15 19:11:11 2002
+++ v2.5.3-pre1-fix/include/linux/file.h Tue Jan 15 19:20:06 2002
@@ -5,6 +5,9 @@
#ifndef __LINUX_FILE_H
#define __LINUX_FILE_H

+#ifndef __ASM__ATOMIC_H
+#include <asm/atomic.h>
+#endif
#ifndef _LINUX_POSIX_TYPES_H /* __FD_CLR */
#include <linux/posix_types.h>
#endif

2002-01-16 00:30:27

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error


On Tue, 15 Jan 2002, Benjamin LaHaise wrote:
>
> Hmm, this should fix that.

probably will, BUT..

> +#ifndef __ASM__ATOMIC_H
> +#include <asm/atomic.h>
> +#endif

Please do not assume knowdledge of what the different header files use to
define their re-entrancy.

Just do

#include <asm/atomic.h>

and be done with it.

Linus

2002-01-16 00:38:47

by David Weinehall

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, Jan 15, 2002 at 04:29:38PM -0800, Linus Torvalds wrote:
>
> On Tue, 15 Jan 2002, Benjamin LaHaise wrote:
> >
> > Hmm, this should fix that.
>
> probably will, BUT..
>
> > +#ifndef __ASM__ATOMIC_H
> > +#include <asm/atomic.h>
> > +#endif
>
> Please do not assume knowdledge of what the different header files use to
> define their re-entrancy.
>
> Just do
>
> #include <asm/atomic.h>
>
> and be done with it.

The lines below say:

#ifndef _LINUX_POSIX_TYPES_H /* __FD_CLR */
#include <linux/posix_types.h>
#endif


Maybe fix this while at it?!


/David
_ _
// David Weinehall <[email protected]> /> Northern lights wander \\
// Maintainer of the v2.0 kernel // Dance across the winter sky //
\> http://www.acc.umu.se/~tao/ </ Full colour fire </

2002-01-16 00:40:57

by Davide Libenzi

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, 15 Jan 2002, Linus Torvalds wrote:

>
> On Tue, 15 Jan 2002, Benjamin LaHaise wrote:
> >
> > Hmm, this should fix that.
>
> probably will, BUT..
>
> > +#ifndef __ASM__ATOMIC_H
> > +#include <asm/atomic.h>
> > +#endif
>
> Please do not assume knowdledge of what the different header files use to
> define their re-entrancy.
>
> Just do
>
> #include <asm/atomic.h>
>
> and be done with it.

I needed two fixes :

#include <asm/atomic.h> in include/linux/file.h

#include <linux/fs.h> in include/linux/dnotify.h

after that it builds for me ... but it crashes at boot time




- Davide


2002-01-16 00:42:17

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error


On Wed, 16 Jan 2002, David Weinehall wrote:
> >
> > Just do
> >
> > #include <asm/atomic.h>
> >
> > and be done with it.
>
> The lines below say:
>
> #ifndef _LINUX_POSIX_TYPES_H /* __FD_CLR */
> #include <linux/posix_types.h>
> #endif

At least that doesn't depend on all architectures having the same
exclusion-defines, but yeah, it's ugly too.

If this actally makes any noticeable difference to compilation speed I
could live with it. Does it?

Linus

2002-01-16 00:44:27

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error


On Tue, 15 Jan 2002, Davide Libenzi wrote:
>
> after that it builds for me ... but it crashes at boot time

Where?

The main big thing in 2.5.3-pre1 is the IDE driver update, the rest looks
fairly simple. I've tested the new IDE driver on my system (the famous
"Works For Me" test), and it's been tested quite a lot in earlier
incarnations on other platforms, but there might easily be some BIO or
low-level IDE chipset interactions.

If it's not in the IDE driver, I'm at a loss.

Linus

2002-01-16 00:44:17

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, Jan 15, 2002 at 04:29:38PM -0800, Linus Torvalds wrote:
> > +#ifndef __ASM__ATOMIC_H
> > +#include <asm/atomic.h>
> > +#endif
>
> Please do not assume knowdledge of what the different header files use to
> define their re-entrancy.

Well, I actually disagree on this. For large include files (fs.h is the
worst), and complicated arrangement, this technique eliminates spurious
includes and saves a lot on compile time (really!). If your concern is
that the convention is not consistent, I'll gladly patch all of them to
use the same format (ie use an __ prefix and escape / to __ and . to _).

-ben

2002-01-16 00:47:35

by Davide Libenzi

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, 15 Jan 2002, Benjamin LaHaise wrote:

> On Tue, Jan 15, 2002 at 04:46:36PM -0800, Davide Libenzi wrote:
> > after that it builds for me ... but it crashes at boot time
>
> Config? Arch? Compiler?

Let me reboot with the crashing kernel for tracing ...

x86 UP Athlon gcc-3.0.3




- Davide


2002-01-16 00:47:34

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error


On Tue, 15 Jan 2002, Benjamin LaHaise wrote:
>
> Well, I actually disagree on this. For large include files (fs.h is the
> worst), and complicated arrangement, this technique eliminates spurious
> includes and saves a lot on compile time (really!).

Numbers please.

I'd MUCH rather just clean up the include file hierarchy than have these
kinds of non-local knowledge issues.

Linus

2002-01-16 00:56:39

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, Jan 15, 2002 at 04:44:54PM -0800, Linus Torvalds wrote:
> Numbers please.
>
> I'd MUCH rather just clean up the include file hierarchy than have these
> kinds of non-local knowledge issues.

The last time I did it for fs.h et al (this meant pulling the fs.h and
sched.h codependancy apart), it got 2.4 compiles back down to 2.2 compile
times (3m -> 2m45s maybe 2m30s iirc) -- about a 10% drop in compile time.
It's even more noticeable when you're doing a fully blown modular kernel
build as distributions do.

-ben
--
Fish.

2002-01-16 00:56:58

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, Jan 15, 2002 at 04:41:08PM -0800, Linus Torvalds wrote:
> > #ifndef _LINUX_POSIX_TYPES_H /* __FD_CLR */
> > #include <linux/posix_types.h>
> > #endif
> If this actally makes any noticeable difference to compilation speed I
> could live with it. Does it?

I'm sure I read somewhere that gcc is clever enough to know
when it hits a #include, it checks for a symbol equal to a
mangled version of the filename before including it.
(Ie, doing this transparently).

Then again, I may have imagined it all.

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2002-01-16 00:56:57

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

From: Linus Torvalds <[email protected]>
Date: Tue, 15 Jan 2002 16:43:58 -0800 (PST)

If it's not in the IDE driver, I'm at a loss.

That "#if 1" buisness in the new ide-dma.c code looks
really suspicious...

2002-01-16 01:11:47

by Davide Libenzi

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, 15 Jan 2002, Linus Torvalds wrote:

>
> On Tue, 15 Jan 2002, Davide Libenzi wrote:
> >
> > after that it builds for me ... but it crashes at boot time
>
> Where?
>
> The main big thing in 2.5.3-pre1 is the IDE driver update, the rest looks
> fairly simple. I've tested the new IDE driver on my system (the famous
> "Works For Me" test), and it's been tested quite a lot in earlier
> incarnations on other platforms, but there might easily be some BIO or
> low-level IDE chipset interactions.
>
> If it's not in the IDE driver, I'm at a loss.

Doh !

Trace; c0113988 <schedule+118/2b0>
Trace; c01075cd <sys_clone+1d/30>
Trace; c0105020 <init+0/140>
Trace; c0105000 <_stext+0/0>
Trace; c010720d <kernel_thread+1d/30>
Trace; c0105011 <rest_init+11/20>

Let me try something ...




- Davide


2002-01-16 01:17:07

by David Weinehall

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, Jan 15, 2002 at 04:52:23PM -0800, Davide Libenzi wrote:
> On Tue, 15 Jan 2002, Benjamin LaHaise wrote:
>
> > On Tue, Jan 15, 2002 at 04:46:36PM -0800, Davide Libenzi wrote:
> > > after that it builds for me ... but it crashes at boot time
> >
> > Config? Arch? Compiler?
>
> Let me reboot with the crashing kernel for tracing ...
>
> x86 UP Athlon gcc-3.0.3

Let me suggest gcc-2.95.3 or gcc-2.96-whatever


/David Weinehall
_ _
// David Weinehall <[email protected]> /> Northern lights wander \\
// Maintainer of the v2.0 kernel // Dance across the winter sky //
\> http://www.acc.umu.se/~tao/ </ Full colour fire </

2002-01-16 01:17:57

by Davide Libenzi

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, 15 Jan 2002, Davide Libenzi wrote:

> Doh !
>
> Trace; c0113988 <schedule+118/2b0>
> Trace; c01075cd <sys_clone+1d/30>
> Trace; c0105020 <init+0/140>
> Trace; c0105000 <_stext+0/0>
> Trace; c010720d <kernel_thread+1d/30>
> Trace; c0105011 <rest_init+11/20>
>
> Let me try something ...

Ehmmm, let's do like if nothing happened :-)




- Davide


2002-01-16 01:19:34

by Davide Libenzi

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Wed, 16 Jan 2002, David Weinehall wrote:

> On Tue, Jan 15, 2002 at 04:52:23PM -0800, Davide Libenzi wrote:
> > On Tue, 15 Jan 2002, Benjamin LaHaise wrote:
> >
> > > On Tue, Jan 15, 2002 at 04:46:36PM -0800, Davide Libenzi wrote:
> > > > after that it builds for me ... but it crashes at boot time
> > >
> > > Config? Arch? Compiler?
> >
> > Let me reboot with the crashing kernel for tracing ...
> >
> > x86 UP Athlon gcc-3.0.3
>
> Let me suggest gcc-2.95.3 or gcc-2.96-whatever

No you should suggest me to check my code :-)




- Davide


2002-01-16 01:19:33

by David Weinehall

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, Jan 15, 2002 at 07:52:04PM -0500, Benjamin LaHaise wrote:
> On Tue, Jan 15, 2002 at 04:44:54PM -0800, Linus Torvalds wrote:
> > Numbers please.
> >
> > I'd MUCH rather just clean up the include file hierarchy than have these
> > kinds of non-local knowledge issues.
>
> The last time I did it for fs.h et al (this meant pulling the fs.h and
> sched.h codependancy apart), it got 2.4 compiles back down to 2.2 compile
> times (3m -> 2m45s maybe 2m30s iirc) -- about a 10% drop in compile time.
> It's even more noticeable when you're doing a fully blown modular kernel
> build as distributions do.

The difference between 3 minutes and 2.45 isn't really a lot imho. What
is interesting if difference scales; that is, if you compile on a 486
or so, does the 10% drop still hold, or are we still talking about a
difference of 15 seconds. If the latter, I'd go for the cleanup,
but if the former is true, maybe, just maybe, the ugly version might
be the solution.

I'd prefer all includes that can be to be non-conditional, though.


/David
_ _
// David Weinehall <[email protected]> /> Northern lights wander \\
// Maintainer of the v2.0 kernel // Dance across the winter sky //
\> http://www.acc.umu.se/~tao/ </ Full colour fire </

2002-01-16 01:23:18

by Robert Love

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, 2002-01-15 at 19:43, Benjamin LaHaise wrote:
> +#ifndef __ASM__ATOMIC_H
> +#include <asm/atomic.h>
> +#endif

gcc -D__KERNEL__ -I/home/rml/src/linux/linux/include -Wall -Wstrict-prototypes -Wno-trigraphs -O2 -fomit-frame-pointer -fno-strict-aliasing -fno-common -pipe -mpreferred-stack-boundary=2 -march=athlon -c -o brlock.o brlock.c
brlock.c:21: initializer element is not constant
brlock.c:21: (near initialization for `__brlock_array[0][0]')
... for all the [x][y] elements ...
brlock.c:21: initializer element is not constant
brlock.c:21: (near initialization for `__brlock_array[31]')

I get this compile error under 2.5.3-pre1, too. System is SMP. 2.5.2
worked fine. I don't see a thing in the patch that would cause this ...

Robert Love

2002-01-16 01:27:36

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, Jan 15, 2002 at 08:24:22PM -0500, Robert Love wrote:
> I get this compile error under 2.5.3-pre1, too. System is SMP. 2.5.2
> worked fine. I don't see a thing in the patch that would cause this ...

Uh, brlock.c probably should be including threads.h...

--
Fish.

2002-01-16 01:30:58

by Dave Jones

[permalink] [raw]
Subject: Re: 2.5.3-pre1 compile error

Another weirdo. Where did this come from ??

(davej@noodles:linux-2.5.3-pre1)$ rgrep isa_bus_to_virt * | wc -l
1

diff -u --recursive --new-file v2.5.2/linux/drivers/video/vesafb.c linux/drivers/video/vesafb.c
--- v2.5.2/linux/drivers/video/vesafb.c Mon Jan 14 18:25:10 2002
+++ linux/drivers/video/vesafb.c Tue Jan 15 10:56:35 2002
@@ -550,7 +550,7 @@
ypan = pmi_setpal = 0; /* not available or some DOS TSR ... */

if (ypan || pmi_setpal) {
- pmi_base = (unsigned short*)bus_to_virt(((unsigned long)screen_info.vesapm_seg << 4) + screen_info.vesapm_off);
+ pmi_base = (unsigned short*)isa_bus_to_virt(((unsigned long)screen_info.vesapm_seg << 4) + screen_info.vesapm_off);
pmi_start = (void*)((char*)pmi_base + pmi_base[1]);
pmi_pal = (void*)((char*)pmi_base + pmi_base[2]);
printk(KERN_INFO "vesafb: pmi: set display start = %p, set palette = %p\n",pmi_start,pmi_pal);

--
Dave Jones. http://www.codemonkey.org.uk
SuSE Labs.

2002-01-16 01:36:17

by Robert Love

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On Tue, 2002-01-15 at 20:25, Benjamin LaHaise wrote:

> Uh, brlock.c probably should be including threads.h...

lib/brlock.c includes include/linux/sched.h which includes
include/linux/threads.h ...

Robert Love

2002-01-16 01:37:57

by Andre Hedrick

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error


Well if you look at it closely, that function.

"ide_raw_build_sglist()"

is not called except from the user-space diagnostics ioctl

You that is getting called with from a BIO request, there will be major
request completion problems.

That is there to allow a future "Alex Viro" suggestion be considered.


On Tue, 15 Jan 2002, David S. Miller wrote:

> From: Linus Torvalds <[email protected]>
> Date: Tue, 15 Jan 2002 16:43:58 -0800 (PST)
>
> If it's not in the IDE driver, I'm at a loss.
>
> That "#if 1" buisness in the new ide-dma.c code looks
> really suspicious...

Andre Hedrick
Linux Disk Certification Project Linux ATA Development

2002-01-16 01:45:48

by Alan

[permalink] [raw]
Subject: Re: 2.5.3-pre1 compile error

> Another weirdo. Where did this come from ??

This looks dubious but with the right results.

> - pmi_base = (unsigned short*)bus_to_virt(((unsigned long)screen_info.vesapm_seg << 4) + screen_info.vesapm_off);
> + pmi_base = (unsigned short*)isa_bus_to_virt(((unsigned long)screen_info.vesapm_seg << 4) + screen_info.vesapm_off);

The address passed back from the BIOS is a physical address. Not a bus
address, not an ISA address. phys_to_virt I suspect is genuinely the right
thing in this unusual case.

2002-01-16 02:10:48

by dmeyer

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

In article <[email protected]> you write:
> On Tue, Jan 15, 2002 at 04:41:08PM -0800, Linus Torvalds wrote:
> > > #ifndef _LINUX_POSIX_TYPES_H /* __FD_CLR */
> > > #include <linux/posix_types.h>
> > > #endif
> > If this actally makes any noticeable difference to compilation speed I
> > could live with it. Does it?
>
> I'm sure I read somewhere that gcc is clever enough to know
> when it hits a #include, it checks for a symbol equal to a
> mangled version of the filename before including it.
> (Ie, doing this transparently).
>
> Then again, I may have imagined it all.

I'm pretty sure you did, since it's perfectly legal (and occasionally
useful, if gruesome) to do

#define THE_STRUCT struct1
#include "struct_def.h"
#undef THE_STRUCT
#define THE_STRUCT struct2
#include "struct_def.h"

and have both #includes work. You may be thinking of #import, which
is deprecated except for objective-c, IIRC.

In answer to Linus' question...yes, in a large system redundent
include guards can make a real difference, particularly for headers
which get included by other headers regularly. OTOH, my last
experience using them was on an underpowered Solaris box, which (a)
didn't have enough memory for all the developers compiling on it and
(b) was running an old Solaris, so its read caching was pretty lame
anyway. It makes rather a lot of difference if the preprocessor has
to read <linux/posix_types.h> from the disk 20 times or if it can get
it from the file cache 20 times.

--
Dave Meyer
[email protected]

2002-01-16 02:45:33

by David Miller

[permalink] [raw]
Subject: Re: 2.5.3-pre1 compile error

From: Alan Cox <[email protected]>
Date: Wed, 16 Jan 2002 01:57:30 +0000 (GMT)

This looks dubious but with the right results.

> - pmi_base = (unsigned short*)bus_to_virt(((unsigned long)screen_info.vesapm_seg << 4) + screen_info.vesapm_off);
> + pmi_base = (unsigned short*)isa_bus_to_virt(((unsigned long)screen_info.vesapm_seg << 4) + screen_info.vesapm_off);

The address passed back from the BIOS is a physical address. Not a bus
address, not an ISA address. phys_to_virt I suspect is genuinely the right
thing in this unusual case.

This slipped thru by accident, it is part of the "kill bus_to_virt"
stuff I'm working on with Jens. Oops...

2002-01-16 05:00:01

by Russ Allbery

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

dmeyer <[email protected]> writes:
> In article <[email protected]> you write:

>> I'm sure I read somewhere that gcc is clever enough to know when it
>> hits a #include, it checks for a symbol equal to a mangled version of
>> the filename before including it. (Ie, doing this transparently).

>> Then again, I may have imagined it all.

No, you read that gcc notices when the entirety of a source file is
wrapped in an #ifdef guard and won't re-read that file when it's included
again if the symbol is defined.

> In answer to Linus' question...yes, in a large system redundent include
> guards can make a real difference, particularly for headers which get
> included by other headers regularly.

Yes, but you don't need to put them around the #include. Just make sure
there is nothing but comments outside the multiple inclusion guards in the
header files and any competent compiler will do the right thing.

--
Russ Allbery ([email protected]) <http://www.eyrie.org/~eagle/>

2002-01-16 11:27:46

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

Linus Torvalds <[email protected]> writes:
>
> > +#ifndef __ASM__ATOMIC_H
> > +#include <asm/atomic.h>
> > +#endif
>
> Please do not assume knowdledge of what the different header files use to
> define their re-entrancy.
>
> Just do
>
> #include <asm/atomic.h>
>
> and be done with it.

The first check is done automatically by the gcc preprocessor
anyways (it has a special check for the #ifndef BLA_H #define BLA_H #endif
construct for whole files and doesn't even bother to open them again on a
second include). So it's completely unnecessary.

Someone added some of these useless checks to 2.5.3pre1. Here is a patch
to remove them. Please consider applying.

--- linux-2.5.3pre1/include/linux/file.h-o Wed Jan 16 12:24:09 2002
+++ linux-2.5.3pre1/include/linux/file.h Wed Jan 16 12:25:10 2002
@@ -5,12 +5,8 @@
#ifndef __LINUX_FILE_H
#define __LINUX_FILE_H

-#ifndef _LINUX_POSIX_TYPES_H /* __FD_CLR */
#include <linux/posix_types.h>
-#endif
-#ifndef __LINUX_COMPILER_H /* unlikely */
#include <linux/compiler.h>
-#endif

/*
* The default fd array needs to be at least BITS_PER_LONG,
--- linux-2.5.3pre1/include/linux/init_task.h-o Wed Jan 16 12:24:09 2002
+++ linux-2.5.3pre1/include/linux/init_task.h Wed Jan 16 12:25:27 2002
@@ -1,9 +1,7 @@
#ifndef _LINUX__INIT_TASK_H
#define _LINUX__INIT_TASK_H

-#ifndef __LINUX_FILE_H
#include <linux/file.h>
-#endif

#define INIT_FILES \
{ \


-Andi

2002-01-16 11:40:17

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

On 16 Jan 2002, Andi Kleen wrote:

> The first check is done automatically by the gcc preprocessor
> anyways (it has a special check for the #ifndef BLA_H #define BLA_H #endif
> construct for whole files and doesn't even bother to open them again on a
> second include). So it's completely unnecessary.

Ahah! I knew I didn't imagine it. Either that or we've both had the same
hallucination. 8-)

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2002-01-16 18:33:08

by Peter Osterlund

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

Dave Jones <[email protected]> writes:

> On Tue, Jan 15, 2002 at 04:41:08PM -0800, Linus Torvalds wrote:
> > > #ifndef _LINUX_POSIX_TYPES_H /* __FD_CLR */
> > > #include <linux/posix_types.h>
> > > #endif
> > If this actally makes any noticeable difference to compilation speed I
> > could live with it. Does it?
>
> I'm sure I read somewhere that gcc is clever enough to know
> when it hits a #include, it checks for a symbol equal to a
> mangled version of the filename before including it.
> (Ie, doing this transparently).
>
> Then again, I may have imagined it all.

Not exactly, but there is an optimization in cpp that makes it
possible to do the cleanup Linus wants without sacrificing
performance. From the cpp info pages:

The GNU C preprocessor is programmed to notice when a header
file uses this particular construct and handle it efficiently.
If a header file is contained entirely in a `#ifndef'
conditional, modulo whitespace and comments, then it remembers
that fact. If a subsequent `#include' specifies the same
file, and the macro in the `#ifndef' is already defined, then
the directive is skipped without processing the specified file
at all.

I did an strace on cpp to verify that this optimization actually
works.

--
Peter Osterlund - [email protected]
http://w1.894.telia.com/~u89404340

2002-01-17 07:39:53

by Neil Booth

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

Dave Jones wrote:-

> On 16 Jan 2002, Andi Kleen wrote:
>
> > The first check is done automatically by the gcc preprocessor
> > anyways (it has a special check for the #ifndef BLA_H #define BLA_H #endif
> > construct for whole files and doesn't even bother to open them again on a
> > second include). So it's completely unnecessary.
>
> Ahah! I knew I didn't imagine it. Either that or we've both had the same
> hallucination. 8-)

I told you over a beer, so maybe you were hallucinating 8^)

Neil.

2002-01-17 22:16:14

by Christopher Turcksin

[permalink] [raw]
Subject: Re: [PATCH] Re: 2.5.3-pre1 compile error

Linus Torvalds wrote:

> > Well, I actually disagree on this. For large include files (fs.h is the
> > worst), and complicated arrangement, this technique eliminates spurious
> > includes and saves a lot on compile time (really!).
>
> Numbers please.

The GNU C preprocessor remembers if an include file was completely
inside an #ifndef/#endif conditional (modulo whitespace and comments)
and will skip the #include for the same file if the macro in #ifndef is
already defined. I suspect putting guards around the include will not
save any more time.

bfn,
Christopher Turcksin <[email protected]>

IBM Global Services