2008-06-08 09:47:04

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH] Speed up "make headers_*"

This is just a heads up patch if anyone is interested.
I finally took the time needed to optimize the
make headers_* targets.

On my box it now takes less than 10 seconds to run
the full install + check cycle.
And it generates roughtly one screen full of output.

Compare that to ~31 seconds and output filling up
my scroll back buffer.

This fixes a long standing complaint from Linus that this is
too simple to warrant such long execution time - yet
there are room for improvements.

Note that this patchset removes support for ALTARCH - only remaining
user is sparc and patches are pending to remove ALTARCH usage form sparc too.

I will do a proper split-up of the patch later and add it
to kbuild-next.git.

Comments (especially to the perl scripts) are welcome.

Sam

b/Makefile | 41 ++++---
b/scripts/Makefile.headersinst | 218 ++++++++++++-----------------------------
b/scripts/headers_check.pl | 48 +++++++++
b/scripts/headers_install.pl | 39 +++++++
scripts/hdrcheck.sh | 10 -
5 files changed, 176 insertions(+), 180 deletions(-)

diff --git a/Makefile b/Makefile
index 8db70fe..8f8fd6f 100644
--- a/Makefile
+++ b/Makefile
@@ -996,36 +996,45 @@ depend dep:

# ---------------------------------------------------------------------------
# Kernel headers
-INSTALL_HDR_PATH=$(objtree)/usr
-export INSTALL_HDR_PATH

-HDRFILTER=generic i386 x86_64
-HDRARCHES=$(filter-out $(HDRFILTER),$(patsubst $(srctree)/include/asm-%/Kbuild,%,$(wildcard $(srctree)/include/asm-*/Kbuild)))
+#Default location for installed headers
+export INSTALL_HDR_PATH = $(objtree)/usr

-PHONY += headers_install_all
-headers_install_all: include/linux/version.h scripts_basic FORCE
+hdr-filter := generic um ppc
+hdr-archs := $(filter-out $(hdr-filter), \
+ $(patsubst $(srctree)/include/asm-%/Kbuild,%, \
+ $(wildcard $(srctree)/include/asm-*/Kbuild)))
+hdr-inst := -rR -f $(srctree)/scripts/Makefile.headersinst obj
+
+PHONY += __headers
+__headers: include/linux/version.h scripts_basic FORCE
$(Q)$(MAKE) $(build)=scripts scripts/unifdef
- $(Q)for arch in $(HDRARCHES); do \
- $(MAKE) ARCH=$$arch -f $(srctree)/scripts/Makefile.headersinst obj=include BIASMDIR=-bi-$$arch ;\
+
+PHONY += headers_install_all
+headers_install_all: __headers
+ $(Q)set -e; for arch in $(hdr-archs); do \
+ $(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include \
+ BIASMDIR=-bi-$$arch ;\
done

PHONY += headers_install
-headers_install: include/linux/version.h scripts_basic FORCE
- @if [ ! -r $(srctree)/include/asm-$(SRCARCH)/Kbuild ]; then \
+headers_install: __headers
+ $(Q)if [ ! -r $(srctree)/include/asm-$(SRCARCH)/Kbuild ]; then \
echo '*** Error: Headers not exportable for this architecture ($(SRCARCH))'; \
- exit 1 ; fi
- $(Q)$(MAKE) $(build)=scripts scripts/unifdef
- $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.headersinst ARCH=$(SRCARCH) obj=include
+ exit 1 ; \
+ fi
+ $(Q)$(MAKE) $(hdr-inst)=include ARCH=$(SRCARCH)

PHONY += headers_check_all
headers_check_all: headers_install_all
- $(Q)for arch in $(HDRARCHES); do \
- $(MAKE) ARCH=$$arch -f $(srctree)/scripts/Makefile.headersinst obj=include BIASMDIR=-bi-$$arch HDRCHECK=1 ;\
+ $(Q)set -e; for arch in $(hdr-archs); do \
+ $(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include \
+ BIASMDIR=-bi-$$arch HDRCHECK=1 ;\
done

PHONY += headers_check
headers_check: headers_install
- $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.headersinst ARCH=$(SRCARCH) obj=include HDRCHECK=1
+ $(Q)$(MAKE) $(hdr-inst)=include ARCH=$(SRCARCH) HDRCHECK=1

# ---------------------------------------------------------------------------
# Modules
diff --git a/scripts/Makefile.headersinst b/scripts/Makefile.headersinst
index 53dae3e..80df6f3 100644
--- a/scripts/Makefile.headersinst
+++ b/scripts/Makefile.headersinst
@@ -7,188 +7,98 @@
#
# ==========================================================================

-UNIFDEF := scripts/unifdef -U__KERNEL__
-
-# Eliminate the contents of (and inclusions of) compiler.h
-HDRSED := sed -e "s/ inline / __inline__ /g" \
- -e "s/[[:space:]]__user[[:space:]]\{1,\}/ /g" \
- -e "s/(__user[[:space:]]\{1,\}/ (/g" \
- -e "s/[[:space:]]__force[[:space:]]\{1,\}/ /g" \
- -e "s/(__force[[:space:]]\{1,\}/ (/g" \
- -e "s/[[:space:]]__iomem[[:space:]]\{1,\}/ /g" \
- -e "s/(__iomem[[:space:]]\{1,\}/ (/g" \
- -e "s/[[:space:]]__attribute_const__[[:space:]]\{1,\}/\ /g" \
- -e "s/[[:space:]]__attribute_const__$$//" \
- -e "/^\#include <linux\/compiler.h>/d"
-
_dst := $(if $(dst),$(dst),$(obj))

-ifeq (,$(patsubst include/asm/%,,$(obj)/))
-# For producing the generated stuff in include/asm for biarch builds, include
-# both sets of Kbuild files; we'll generate anything which is mentioned in
-# _either_ arch, and recurse into subdirectories which are mentioned in either
-# arch. Since some directories may exist in one but not the other, we must
-# use $(wildcard...).
-GENASM := 1
-archasm := $(subst include/asm,asm-$(ARCH),$(obj))
-altarchasm := $(subst include/asm,asm-$(ALTARCH),$(obj))
-KBUILDFILES := $(wildcard $(srctree)/include/$(archasm)/Kbuild $(srctree)/include/$(altarchasm)/Kbuild)
-else
-KBUILDFILES := $(srctree)/$(obj)/Kbuild
-endif
+kbuild-file := $(srctree)/$(obj)/Kbuild
+include $(kbuild-file)

-include $(KBUILDFILES)
+include scripts/Kbuild.include

-include scripts/Kbuild.include
-
-# If this is include/asm-$(ARCH) and there's no $(ALTARCH), then
-# override $(_dst) so that we install to include/asm directly.
+# If this is include/asm-$(ARCH) then override $(_dst) so that
+# we install to include/asm directly.
# Unless $(BIASMDIR) is set, in which case we're probably doing
# a 'headers_install_all' build and we should keep the -$(ARCH)
# in the directory name.
-ifeq ($(obj)$(ALTARCH),include/asm-$(ARCH)$(BIASMDIR))
+ifeq ($(obj),include/asm-$(ARCH)$(BIASMDIR))
_dst := include/asm
endif

-header-y := $(sort $(header-y))
-unifdef-y := $(sort $(unifdef-y))
-subdir-y := $(patsubst %/,%,$(filter %/, $(header-y)))
-header-y := $(filter-out %/, $(header-y))
-header-y := $(filter-out $(unifdef-y),$(header-y))
+install := $(INSTALL_HDR_PATH)/$(_dst)

-# stamp files for header checks
-check-y := $(patsubst %,.check.%,$(header-y) $(unifdef-y) $(objhdr-y))
+header-y := $(sort $(header-y) $(unifdef-y))
+subdirs := $(patsubst %/,%,$(filter %/, $(header-y)))
+header-y := $(filter-out %/, $(header-y))

-# Work out what needs to be removed
-oldheaders := $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,%,$(wildcard $(INSTALL_HDR_PATH)/$(_dst)/*.h))
-unwanted := $(filter-out $(header-y) $(unifdef-y) $(objhdr-y),$(oldheaders))
+# files used to track state of install/check
+install-file := $(install)/.install
+check-file := $(install)/.check
+check-dep := $(install)/.check.d

-oldcheckstamps := $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,%,$(wildcard $(INSTALL_HDR_PATH)/$(_dst)/.check.*.h))
-unwanted += $(filter-out $(check-y),$(oldcheckstamps))
+# all headers files for this dir
+all-files := $(header-y) $(objhdr-y)

-# Prefix them all with full paths to $(INSTALL_HDR_PATH)
-header-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(header-y))
-unifdef-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(unifdef-y))
-objhdr-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(objhdr-y))
-check-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(check-y))
+# Work out what needs to be removed
+oldheaders := $(patsubst $(install)/%,%,$(wildcard $(install)/*.h))
+unwanted := $(filter-out $(all-files),$(oldheaders))

+# Prefix all with full paths to $(INSTALL_HDR_PATH)
+header-y := $(addprefix $(install)/, $(header-y))
+objhdr-y := $(addprefix $(install)/, $(objhdr-y))
+unwanted := $(addprefix $(install)/, $(unwanted))

-ifdef ALTARCH
-ifeq ($(obj),include/asm-$(ARCH))
-altarch-y := altarch-dir
-endif
-endif
+printdir = $(patsubst $(INSTALL_HDR_PATH)/%/,%,$(dir $@))

-# Make the definitions visible for recursive make invocations
-export ALTARCH
-export ARCHDEF
-export ALTARCHDEF
-
-quiet_cmd_o_hdr_install = INSTALL $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_o_hdr_install = cp $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,$(objtree)/$(obj)/%,$@) \
- $(INSTALL_HDR_PATH)/$(_dst)
-
-quiet_cmd_headers_install = INSTALL $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_headers_install = $(HDRSED) $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,$(srctree)/$(obj)/%,$@) \
- > $@
-
-quiet_cmd_unifdef = UNIFDEF $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_unifdef = $(UNIFDEF) $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,$(srctree)/$(obj)/%,$@) \
- | $(HDRSED) > $@ || :
-
-quiet_cmd_check = CHECK $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/.check.%,$(_dst)/%,$@)
- cmd_check = $(CONFIG_SHELL) $(srctree)/scripts/hdrcheck.sh \
- $(INSTALL_HDR_PATH)/include $(subst /.check.,/,$@) $@
-
-quiet_cmd_remove = REMOVE $(_dst)/$@
- cmd_remove = rm -f $(INSTALL_HDR_PATH)/$(_dst)/$@
-
-quiet_cmd_mkdir = MKDIR $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_mkdir = mkdir -p $@
-
-quiet_cmd_gen = GEN $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_gen = \
-FNAME=$(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,%,$@); \
-STUBDEF=__ASM_STUB_`echo $$FNAME | tr a-z.- A-Z__`; \
-(echo "/* File autogenerated by 'make headers_install' */" ; \
-echo "\#ifndef $$STUBDEF" ; \
-echo "\#define $$STUBDEF" ; \
-echo "\# if $(ARCHDEF)" ; \
-if [ -r $(subst /$(_dst)/,/include/$(archasm)/,$@) ]; then \
- echo "\# include <$(archasm)/$$FNAME>" ; \
-else \
- echo "\# error $(archasm)/$$FNAME does not exist in" \
- "the $(ARCH) architecture" ; \
-fi ; \
-echo "\# elif $(ALTARCHDEF)" ; \
-if [ -r $(subst /$(_dst)/,/include/$(altarchasm)/,$@) ]; then \
- echo "\# include <$(altarchasm)/$$FNAME>" ; \
-else \
- echo "\# error $(altarchasm)/$$FNAME does not exist in" \
- "the $(ALTARCH) architecture" ; \
-fi ; \
-echo "\# else" ; \
-echo "\# warning This machine appears to be" \
- "neither $(ARCH) nor $(ALTARCH)." ; \
-echo "\# endif" ; \
-echo "\#endif /* $$STUBDEF */" ; \
-) > $@
-
-.PHONY: __headersinst __headerscheck
+quiet_cmd_install = INSTALL $(printdir) ($(words $(all-files)) files)
+ cmd_install = $(PERL) $(srctree)/scripts/headers_install.pl \
+ $(obj) $(install) $(all-files)

-ifdef HDRCHECK
-__headerscheck: $(subdir-y) $(check-y)
- @true
+quiet_cmd_remove = REMOVE $(patsubst $(INSTALL_HDR_PATH)/%,%,$(unwanted))
+ cmd_remove = rm -f $(unwanted)
+
+quiet_cmd_check = CHECK $(printdir) ($(words $(all-files)) files)
+ cmd_check = $(PERL) $(srctree)/scripts/headers_check.pl \
+ $(INSTALL_HDR_PATH)/include \
+ $(addprefix $(install)/, $(all-files))

-$(check-y) : $(INSTALL_HDR_PATH)/$(_dst)/.check.%.h : $(INSTALL_HDR_PATH)/$(_dst)/%.h
- $(call cmd,check)
+PHONY += __headersinst __headerscheck

-# Other dependencies for $(check-y)
-include /dev/null $(wildcard $(check-y))
+ifdef HDRCHECK
+__headerscheck: $(subdirs) $(check-file)
+ @:

-# ... but leave $(check-y) as .PHONY for now until those deps are actually correct.
-.PHONY: $(check-y)
+targets += $(check-file)
+$(check-file): $(addprefix $(srctree)/$(obj)/,$(all-files)) FORCE
+ $(call if_changed,check)
+ $(Q)touch $@

else
# Rules for installing headers
-__headersinst: $(subdir-y) $(header-y) $(unifdef-y) $(altarch-y) $(objhdr-y)
- @true
-
-$(objhdr-y) $(subdir-y) $(header-y) $(unifdef-y): | $(INSTALL_HDR_PATH)/$(_dst) $(unwanted)
-
-$(INSTALL_HDR_PATH)/$(_dst):
- $(call cmd,mkdir)
+__headersinst: $(subdirs) $(install-file)
+ @:

-.PHONY: $(unwanted)
-$(unwanted):
- $(call cmd,remove)
+targets += $(install-file)
+$(install-file): $(addprefix $(srctree)/$(obj)/,$(all-files)) FORCE
+ $(if $(unwanted),$(call cmd,remove),)
+ $(if $(wildcard $(dir $@)),,$(shell mkdir -p $(dir $@)))
+ $(call if_changed,install)
+ $(Q)touch $@

-ifdef GENASM
-$(objhdr-y) $(header-y) $(unifdef-y): $(KBUILDFILES)
- $(call cmd,gen)
+endif

-else
-$(objhdr-y) : $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(objtree)/$(obj)/%.h $(KBUILDFILES)
- $(call cmd,o_hdr_install)
+# Recursion
+hdr-inst := -rR -f $(srctree)/scripts/Makefile.headersinst obj
+.PHONY: $(subdirs)
+$(subdirs):
+ $(Q)$(MAKE) $(hdr-inst)=$(obj)/$@ dst=$(_dst)/$@

-$(header-y) : $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(srctree)/$(obj)/%.h $(KBUILDFILES)
- $(call cmd,headers_install)
+targets := $(wildcard $(sort $(targets)))
+cmd_files := $(wildcard \
+ $(foreach f,$(targets),$(dir $(f)).$(notdir $(f)).cmd))

-$(unifdef-y) : $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(srctree)/$(obj)/%.h $(KBUILDFILES)
- $(call cmd,unifdef)
-endif
+ifneq ($(cmd_files),)
+ include $(cmd_files)
endif

-hdrinst := -rR -f $(srctree)/scripts/Makefile.headersinst obj
-
-.PHONY: altarch-dir
-# All the files in the normal arch dir must be created first, since we test
-# for their existence.
-altarch-dir: $(subdir-y) $(header-y) $(unifdef-y) $(objhdr-y)
- $(Q)$(MAKE) $(hdrinst)=include/asm-$(ALTARCH) dst=include/asm-$(ALTARCH)
- $(Q)$(MAKE) $(hdrinst)=include/asm dst=include/asm$(BIASMDIR)
-
-# Recursion
-.PHONY: $(subdir-y)
-$(subdir-y):
- $(Q)$(MAKE) $(hdrinst)=$(obj)/$@ dst=$(_dst)/$@ rel=../$(rel)
+.PHONY: $(PHONY)
+PHONY += FORCE
+FORCE: ;
diff --git a/scripts/hdrcheck.sh b/scripts/hdrcheck.sh
deleted file mode 100755
index 3159858..0000000
--- a/scripts/hdrcheck.sh
+++ /dev/null
@@ -1,10 +0,0 @@
-#!/bin/sh
-
-for FILE in `grep '^[ \t]*#[ \t]*include[ \t]*<' $2 | cut -f2 -d\< | cut -f1 -d\> | egrep ^linux\|^asm` ; do
- if [ ! -r $1/$FILE ]; then
- echo $2 requires $FILE, which does not exist in exported headers
- exit 1
- fi
-done
-# FIXME: List dependencies into $3
-touch $3
diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl
new file mode 100644
index 0000000..dfb2ec7
--- /dev/null
+++ b/scripts/headers_check.pl
@@ -0,0 +1,48 @@
+#!/usr/bin/perl
+#
+# headers_check.pl execute a number of trivial consistency checks
+#
+# Usage: headers_check.pl dir [files...]
+# dir..: dir to look for included files
+# files: list of files to check
+#
+# The script reads the supplied files line by line and:
+#
+# 1) for each include statement it checks if the
+# included file actually exists.
+# Only include files located in asm* and linux* are checked.
+# The rest are assumed to be system include files.
+#
+# 2) TODO
+
+my ($dir, @files) = @ARGV;
+
+my $ret = 0;
+my $lineno = 0;
+my $filename;
+
+foreach $file (@files) {
+ $filename = $file;
+ open(FILE, "< $filename") or die "$filename: $!\n";
+ $lineno = 0;
+ while ($line = <FILE>) {
+ $lineno++;
+ &check_include();
+ }
+ close(FILE);
+}
+exit($ret);
+
+my $includere = qr/^\s*#\s*include\s+<(.*)>/;
+sub check_include()
+{
+ if ($line =~ m/^\s*#\s*include\s+<(.*)>/) {
+ my $inc = $1;
+ if ($inc =~ m/^asm/ || $inc =~ m/^linux/) {
+ if (!(stat($dir . "/" . $inc))) {
+ printf STDERR "$filename:$lineno: included file '$inc' is not exported\n";
+ $ret = 1;
+ }
+ }
+ }
+}
diff --git a/scripts/headers_install.pl b/scripts/headers_install.pl
new file mode 100644
index 0000000..8aa66af
--- /dev/null
+++ b/scripts/headers_install.pl
@@ -0,0 +1,39 @@
+#!/usr/bin/perl
+#
+# headers_install prepare the listed header files for use in
+# user space and copy the files to their destination.
+#
+# Usage: headers_install.pl opendir installdir [files...]
+# opendir: dir to open files
+# install: dir to install the files
+# files..: list of files to check
+#
+# Step in preparation for users space:
+# 1) Drop all use of compiler.h definitions
+# 2) Drop include of compiler.h
+# 3) Drop all sections defined out by __KERNEL__
+
+my ($opendir, $installdir, @files) = @ARGV;
+
+my $ret = 0;
+
+foreach $file (@files) {
+ open(INFILE, "< $opendir/$file") or die "$openddir/$file: $!\n";
+ open(OUTFILE, "> $installdir/$file.tmp") or
+ die "$installdir/$file.tmp: $!\n";
+ while ($line = <INFILE>) {
+ $line =~ s/([\s(])__user\s/\1/g;
+ $line =~ s/([\s(])__force\s/\1/g;
+ $line =~ s/([\s(])__iomem\s/\1/g;
+ $line =~ s/\s__attribute_const__\s/ /g;
+ $line =~ s/\s__attribute_const__$//g;
+ $line =~ s/^#include <linux\/compiler.h>//;
+ printf OUTFILE $line;
+ }
+ close(OUTFILE);
+ close(INFILE);
+ system "scripts/unifdef -U__KERNEL__ $installdir/$file.tmp > $installdir/$file"
+}
+
+exit($ret);
+


2008-06-08 10:12:48

by Vegard Nossum

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, Jun 8, 2008 at 11:47 AM, Sam Ravnborg <[email protected]> wrote:
> This is just a heads up patch if anyone is interested.
> I finally took the time needed to optimize the
> make headers_* targets.
>
> On my box it now takes less than 10 seconds to run
> the full install + check cycle.
> And it generates roughtly one screen full of output.
>
> Compare that to ~31 seconds and output filling up
> my scroll back buffer.

Nice :-)

> Comments (especially to the perl scripts) are welcome.

Will do! Some of my comments are a bit on the pedantic side, so you
choose yourself which ones you want to heed!

> diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl
> new file mode 100644
> index 0000000..dfb2ec7
> --- /dev/null
> +++ b/scripts/headers_check.pl
> @@ -0,0 +1,48 @@
> +#!/usr/bin/perl
> +#
> +# headers_check.pl execute a number of trivial consistency checks
> +#
> +# Usage: headers_check.pl dir [files...]
> +# dir..: dir to look for included files
> +# files: list of files to check
> +#
> +# The script reads the supplied files line by line and:
> +#
> +# 1) for each include statement it checks if the
> +# included file actually exists.
> +# Only include files located in asm* and linux* are checked.
> +# The rest are assumed to be system include files.
> +#
> +# 2) TODO

'use strict' and 'use warnings' is recommended. You have "my" on the
variables anyway (which is a good thing!).

> +
> +my ($dir, @files) = @ARGV;
> +
> +my $ret = 0;
> +my $lineno = 0;
> +my $filename;
> +
> +foreach $file (@files) {
> + $filename = $file;
> + open(FILE, "< $filename") or die "$filename: $!\n";

I personally prefer to put '<', $filename here instead of mashing them
together as one string. I don't know if it really matters.

If you want to compile with strict/warnings here, you should replace
FILE with "my $fh" here

> + $lineno = 0;
> + while ($line = <FILE>) {

...and use <$fh> here.

> + $lineno++;
> + &check_include();

The & should be gone (see perldoc -q 'calling a function')

> + }
> + close(FILE);
> +}
> +exit($ret);

The parentheses are not needed for most of the built-in functions.

> +
> +my $includere = qr/^\s*#\s*include\s+<(.*)>/;

What seems to be the purpose of this, also what's up with the funny name? :-D

> +sub check_include()

The parentheses should be gone. This has something to do with
prototypes and should normally not be used.

> +{
> + if ($line =~ m/^\s*#\s*include\s+<(.*)>/) {
> + my $inc = $1;

This could also be written as if (my($inc) = $line =~ ...), and you
bypass the use of $1, which is often error-prone... but it's correct
in this case and is simply a matter of taste.

> + if ($inc =~ m/^asm/ || $inc =~ m/^linux/) {
> + if (!(stat($dir . "/" . $inc))) {

May use unless() instead of if(!()) here. But that's just perl pedantry.

> + printf STDERR "$filename:$lineno: included file '$inc' is not exported\n";
> + $ret = 1;
> + }
> + }
> + }
> +}

More or less the same comments would apply to the next script as well.

> diff --git a/scripts/headers_install.pl b/scripts/headers_install.pl
> new file mode 100644
> index 0000000..8aa66af
> --- /dev/null
> +++ b/scripts/headers_install.pl
...
> + $line =~ s/([\s(])__user\s/\1/g;
...

Use of $1 instead of \1 is recommended, see perldoc perlre ("Warning
on \1 vs $1").


Vegard

--
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
-- E. W. Dijkstra, EWD1036

2008-06-08 10:40:51

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, Jun 08, 2008 at 12:12:35PM +0200, Vegard Nossum wrote:
> On Sun, Jun 8, 2008 at 11:47 AM, Sam Ravnborg <[email protected]> wrote:
> > This is just a heads up patch if anyone is interested.
> > I finally took the time needed to optimize the
> > make headers_* targets.
> >
> > On my box it now takes less than 10 seconds to run
> > the full install + check cycle.
> > And it generates roughtly one screen full of output.
> >
> > Compare that to ~31 seconds and output filling up
> > my scroll back buffer.
>
> Nice :-)
>
> > Comments (especially to the perl scripts) are welcome.
>
> Will do! Some of my comments are a bit on the pedantic side, so you
> choose yourself which ones you want to heed!

Thnaks!

headers_install.pl looks like this now.
I am not happy about the way I call unifdef - can it be
done better?
No error handling and I like to avid the extra tmp file.

Sam

#!/usr/bin/perl
#
# headers_install prepare the listed header files for use in
# user space and copy the files to their destination.
#
# Usage: headers_install.pl odir installdir [files...]
# odir: dir to open files
# install: dir to install the files
# files: list of files to check
#
# Step in preparation for users space:
# 1) Drop all use of compiler.h definitions
# 2) Drop include of compiler.h
# 3) Drop all sections defined out by __KERNEL__

use strict;
use warnings;

my ($odir, $installdir, @files) = @ARGV;

my $ret = 0;

foreach my $file (@files) {
open(my $infile, '<', "$odir/$file") or die "$odir/$file: $!\n";
open(my $outfile, '>', "$installdir/$file.tmp") or
die "$installdir/$file.tmp: $!\n";
while (my $line = <$infile>) {
$line =~ s/([\s(])__user\s/$1/g;
$line =~ s/([\s(])__force\s/$1/g;
$line =~ s/([\s(])__iomem\s/$1/g;
$line =~ s/\s__attribute_const__\s/ /g;
$line =~ s/\s__attribute_const__$//g;
$line =~ s/^#include <linux\/compiler.h>//;
printf $outfile "%s", $line;
}
close($outfile);
close($outfile);
system "scripts/unifdef -U__KERNEL__ $installdir/$file.tmp > $installdir/$file"
}

exit $ret;

2008-06-08 10:49:18

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, 2008-06-08 at 12:41 +0200, Sam Ravnborg wrote:
> headers_install.pl looks like this now.
> I am not happy about the way I call unifdef - can it be
> done better?

Possibly. unifdef only actually handles "#ifdef __KERNEL__" and
"#ifndef __KERNEL__", doesn't it? It shouldn't be too hard to recreate
at least that much functionality in perl, surely?

Bonus points for making it handle more interesting constructs like
"#if defined (KERNEL) || defined (FOO)", and for warning/erroring
whenever any ifdefs on CONFIG_xxx would be visible in userspace.
But those can come later; we don't have those yet anyway.

--
dwmw2

2008-06-08 11:05:33

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, Jun 08, 2008 at 12:41:22PM +0200, Sam Ravnborg wrote:
>On Sun, Jun 08, 2008 at 12:12:35PM +0200, Vegard Nossum wrote:
>> On Sun, Jun 8, 2008 at 11:47 AM, Sam Ravnborg <[email protected]> wrote:
>> > This is just a heads up patch if anyone is interested.
>> > I finally took the time needed to optimize the
>> > make headers_* targets.
>> >
>> > On my box it now takes less than 10 seconds to run
>> > the full install + check cycle.
>> > And it generates roughtly one screen full of output.
>> >
>> > Compare that to ~31 seconds and output filling up
>> > my scroll back buffer.
>>
>> Nice :-)
>>
>> > Comments (especially to the perl scripts) are welcome.
>>
>> Will do! Some of my comments are a bit on the pedantic side, so you
>> choose yourself which ones you want to heed!
>
>Thnaks!
>
>headers_install.pl looks like this now.
>I am not happy about the way I call unifdef - can it be
>done better?
>No error handling and I like to avid the extra tmp file.


I think you can open a pipe in Perl, e.g. open FILE, "|scripts/unifdef";.


>
> Sam
>
>#!/usr/bin/perl
>#
># headers_install prepare the listed header files for use in
># user space and copy the files to their destination.
>#
># Usage: headers_install.pl odir installdir [files...]
># odir: dir to open files
># install: dir to install the files
># files: list of files to check
>#
># Step in preparation for users space:
># 1) Drop all use of compiler.h definitions
># 2) Drop include of compiler.h
># 3) Drop all sections defined out by __KERNEL__
>
>use strict;
>use warnings;
>
>my ($odir, $installdir, @files) = @ARGV;
>
>my $ret = 0;


This is only used by last exit, thus can be removed.

>
>foreach my $file (@files) {
> open(my $infile, '<', "$odir/$file") or die "$odir/$file: $!\n";
> open(my $outfile, '>', "$installdir/$file.tmp") or
> die "$installdir/$file.tmp: $!\n";
> while (my $line = <$infile>) {
> $line =~ s/([\s(])__user\s/$1/g;
> $line =~ s/([\s(])__force\s/$1/g;
> $line =~ s/([\s(])__iomem\s/$1/g;
> $line =~ s/\s__attribute_const__\s/ /g;
> $line =~ s/\s__attribute_const__$//g;
> $line =~ s/^#include <linux\/compiler.h>//;
> printf $outfile "%s", $line;
> }
> close($outfile);
> close($outfile);


'close' doesn't need parenthesises neither.

> system "scripts/unifdef -U__KERNEL__ $installdir/$file.tmp > $installdir/$file"


Will scripts/unifdef clean the tmp file? If not, you should do it, right?


>}
>
>exit $ret;
>


--
Hi, I'm a .signature virus, please copy/paste me to help me spread
all over the world.

2008-06-08 11:06:53

by Vegard Nossum

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, Jun 8, 2008 at 12:41 PM, Sam Ravnborg <[email protected]> wrote:
> headers_install.pl looks like this now.
> I am not happy about the way I call unifdef - can it be
> done better?
> No error handling and I like to avid the extra tmp file.
>
> Sam
>
> #!/usr/bin/perl
> #
> # headers_install prepare the listed header files for use in
> # user space and copy the files to their destination.
> #
> # Usage: headers_install.pl odir installdir [files...]
> # odir: dir to open files
> # install: dir to install the files
> # files: list of files to check
> #
> # Step in preparation for users space:
> # 1) Drop all use of compiler.h definitions
> # 2) Drop include of compiler.h
> # 3) Drop all sections defined out by __KERNEL__
>
> use strict;
> use warnings;
>
> my ($odir, $installdir, @files) = @ARGV;
>
> my $ret = 0;
>
> foreach my $file (@files) {
> open(my $infile, '<', "$odir/$file") or die "$odir/$file: $!\n";
> open(my $outfile, '>', "$installdir/$file.tmp") or
> die "$installdir/$file.tmp: $!\n";
> while (my $line = <$infile>) {
> $line =~ s/([\s(])__user\s/$1/g;
> $line =~ s/([\s(])__force\s/$1/g;
> $line =~ s/([\s(])__iomem\s/$1/g;
> $line =~ s/\s__attribute_const__\s/ /g;
> $line =~ s/\s__attribute_const__$//g;
> $line =~ s/^#include <linux\/compiler.h>//;
> printf $outfile "%s", $line;
> }
> close($outfile);
> close($outfile);

Btw, this should probably be $infile if you decide to keep this version.

> system "scripts/unifdef -U__KERNEL__ $installdir/$file.tmp > $installdir/$file"
> }

Yeah, it should be possible, but I fear that it involves the use of a
bidirectional pipe. You want to pipe some data into the program and
some data out of it. See perldoc perlipc ("Bidirectional Communication
with Another Process").

In short, I think you'd need this:

use FileHandle;
use IPC::Open2;

my($unifdef_in, $unifdef_out);
open2($unifdef_in, $unifdef_out, 'scripts/unifdef', '-U__KERNEL__') ...;

open(my $infile, '<', "$odir/$ofile") || die ...;
while (my $line = <$infile>) {
print $unifdef_in $line;
}
close $infile;
close $unifdef_in; # Send EOF to unifdef, so that it sends EOF to us.

open(my $outfile ...;
while (my $line = <$unifdef_out>) {
print $outfile $line;
}
close $outfile;
close $unifdef_out;

But as you see this is rather lengthy. I also don't know if it's
correct (IOW, completely untested), so you may have to fiddle a bit to
get it working.

Good luck.


Vegard

--
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
-- E. W. Dijkstra, EWD1036

2008-06-08 11:16:34

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, Jun 08, 2008 at 11:49:01AM +0100, David Woodhouse wrote:
> On Sun, 2008-06-08 at 12:41 +0200, Sam Ravnborg wrote:
> > headers_install.pl looks like this now.
> > I am not happy about the way I call unifdef - can it be
> > done better?
>
> Possibly. unifdef only actually handles "#ifdef __KERNEL__" and
> "#ifndef __KERNEL__", doesn't it? It shouldn't be too hard to recreate
> at least that much functionality in perl, surely?

Correct. A quick grep shows that we have these different
uses of __KERNEL__:

#if defined(__ARM_EABI__) && !defined(__KERNEL__)
#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
#if defined(CONFIG_VIDEO_V4L1_COMPAT) || !defined (__KERNEL__)
#if defined(__GNUC__) && !defined(__STRICT_ANSI__) || defined(__KERNEL__)
#if defined(__KERNEL__)
#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
#if defined(__KERNEL__) && defined(CONFIG_PPC32)
#if defined(__KERNEL__) && defined(CONFIG_SMP) && !defined(__ASSEMBLY__)
#if !defined(__KERNEL__) || defined(CONFIG_X86)
#if defined(__KERNEL__) || defined(__DEFINE_BSD_TERMIOS)
#if !defined(__KERNEL__) && !defined(DIV_ROUND_UP)
#if defined(__KERNEL__) || !defined(__GLIBC__) || (__GLIBC__ < 2)
#if (!defined(__KERNEL__) && !defined(KERNEL) && !defined(INKERNEL) && !defined(_KERNEL)) || defined(USE_SEQ_MACROS)
#if defined(__KERNEL__) || defined(__linux__)
# if (defined(__KERNEL__) || !defined(RELOC_DEBUG)) \
#if defined(__KERNEL__) || defined(__USE_ALL)
#if defined(__KERNEL__) || defined(__WANT_POSIX1B_SIGNALS__)
#if defined(__KERNEL__) && defined(__x86_64__)
#if !defined(__STRICT_ANSI__) || defined(__KERNEL__)
#ifdef __KERNEL__
#ifndef __KERNEL__

#else and #endif filtered away.

A script needs to take into account other preprocessor
uses too due to their nested nature.
But doable I'm sure.

And I rather have 100 lines perl than use the unifdef utility
because we then have it collected in one place and can do even
stricter validation.


>
> Bonus points for making it handle more interesting constructs like
> "#if defined (KERNEL) || defined (FOO)", and for warning/erroring
> whenever any ifdefs on CONFIG_xxx would be visible in userspace.
> But those can come later; we don't have those yet anyway.

Agree.

Sam

2008-06-08 11:18:00

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

Hi WANG.

> >No error handling and I like to avid the extra tmp file.
>
>
> I think you can open a pipe in Perl, e.g. open FILE, "|scripts/unifdef";.
>
>
> >
> > Sam
> >
> >#!/usr/bin/perl
> >#
> ># headers_install prepare the listed header files for use in
> ># user space and copy the files to their destination.
> >#
> ># Usage: headers_install.pl odir installdir [files...]
> ># odir: dir to open files
> ># install: dir to install the files
> ># files: list of files to check
> >#
> ># Step in preparation for users space:
> ># 1) Drop all use of compiler.h definitions
> ># 2) Drop include of compiler.h
> ># 3) Drop all sections defined out by __KERNEL__
> >
> >use strict;
> >use warnings;
> >
> >my ($odir, $installdir, @files) = @ARGV;
> >
> >my $ret = 0;
>
>
> This is only used by last exit, thus can be removed.
>
> >
> >foreach my $file (@files) {
> > open(my $infile, '<', "$odir/$file") or die "$odir/$file: $!\n";
> > open(my $outfile, '>', "$installdir/$file.tmp") or
> > die "$installdir/$file.tmp: $!\n";
> > while (my $line = <$infile>) {
> > $line =~ s/([\s(])__user\s/$1/g;
> > $line =~ s/([\s(])__force\s/$1/g;
> > $line =~ s/([\s(])__iomem\s/$1/g;
> > $line =~ s/\s__attribute_const__\s/ /g;
> > $line =~ s/\s__attribute_const__$//g;
> > $line =~ s/^#include <linux\/compiler.h>//;
> > printf $outfile "%s", $line;
> > }
> > close($outfile);
> > close($outfile);
>
>
> 'close' doesn't need parenthesises neither.
>
> > system "scripts/unifdef -U__KERNEL__ $installdir/$file.tmp > $installdir/$file"
>
>
> Will scripts/unifdef clean the tmp file? If not, you should do it, right?

Will fix - it was left as I hope to fix the unifdef thing
and then it may error out too.

Sam

2008-06-08 11:20:21

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, 2008-06-08 at 11:47 +0200, Sam Ravnborg wrote:
> + $(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include \
> + BIASMDIR=-bi-$$arch ;\

We still use $(BIASMDIR) as a trigger to install into asm-$(ARCH)/
instead of just to asm/, for 'headers_install_all'. But it's a fairly
bad name now, since it doesn't do what it used to do.

--
dwmw2

2008-06-08 11:20:39

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, Jun 08, 2008 at 01:06:43PM +0200, Vegard Nossum wrote:
> On Sun, Jun 8, 2008 at 12:41 PM, Sam Ravnborg <[email protected]> wrote:
> > headers_install.pl looks like this now.
> > I am not happy about the way I call unifdef - can it be
> > done better?
> > No error handling and I like to avid the extra tmp file.
> >
> > Sam
> >
> > #!/usr/bin/perl
> > #
> > # headers_install prepare the listed header files for use in
> > # user space and copy the files to their destination.
> > #
> > # Usage: headers_install.pl odir installdir [files...]
> > # odir: dir to open files
> > # install: dir to install the files
> > # files: list of files to check
> > #
> > # Step in preparation for users space:
> > # 1) Drop all use of compiler.h definitions
> > # 2) Drop include of compiler.h
> > # 3) Drop all sections defined out by __KERNEL__
> >
> > use strict;
> > use warnings;
> >
> > my ($odir, $installdir, @files) = @ARGV;
> >
> > my $ret = 0;
> >
> > foreach my $file (@files) {
> > open(my $infile, '<', "$odir/$file") or die "$odir/$file: $!\n";
> > open(my $outfile, '>', "$installdir/$file.tmp") or
> > die "$installdir/$file.tmp: $!\n";
> > while (my $line = <$infile>) {
> > $line =~ s/([\s(])__user\s/$1/g;
> > $line =~ s/([\s(])__force\s/$1/g;
> > $line =~ s/([\s(])__iomem\s/$1/g;
> > $line =~ s/\s__attribute_const__\s/ /g;
> > $line =~ s/\s__attribute_const__$//g;
> > $line =~ s/^#include <linux\/compiler.h>//;
> > printf $outfile "%s", $line;
> > }
> > close($outfile);
> > close($outfile);
>
> Btw, this should probably be $infile if you decide to keep this version.

Ups - thanks.

>
> > system "scripts/unifdef -U__KERNEL__ $installdir/$file.tmp > $installdir/$file"
> > }
>
> Yeah, it should be possible, but I fear that it involves the use of a
> bidirectional pipe. You want to pipe some data into the program and
> some data out of it. See perldoc perlipc ("Bidirectional Communication
> with Another Process").
>
> In short, I think you'd need this:
>
> use FileHandle;
> use IPC::Open2;
>
> my($unifdef_in, $unifdef_out);
> open2($unifdef_in, $unifdef_out, 'scripts/unifdef', '-U__KERNEL__') ...;
>
> open(my $infile, '<', "$odir/$ofile") || die ...;
> while (my $line = <$infile>) {
> print $unifdef_in $line;
> }
> close $infile;
> close $unifdef_in; # Send EOF to unifdef, so that it sends EOF to us.
>
> open(my $outfile ...;
> while (my $line = <$unifdef_out>) {
> print $outfile $line;
> }
> close $outfile;
> close $unifdef_out;
>
> But as you see this is rather lengthy. I also don't know if it's
> correct (IOW, completely untested), so you may have to fiddle a bit to
> get it working.

I hope someone will cook up the limted unifdef functionality in perl,
then we do not have to care - hint ;-)

Otherwise I will play with your suggestion.

Thanks,
Sam

2008-06-08 11:29:54

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, Jun 08, 2008 at 12:20:04PM +0100, David Woodhouse wrote:
> On Sun, 2008-06-08 at 11:47 +0200, Sam Ravnborg wrote:
> > + $(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include \
> > + BIASMDIR=-bi-$$arch ;\
>
> We still use $(BIASMDIR) as a trigger to install into asm-$(ARCH)/
> instead of just to asm/, for 'headers_install_all'. But it's a fairly
> bad name now, since it doesn't do what it used to do.

Thanks.

I will name it ASMDIR and set it to 'y' if we shall install to asm/

Sam

2008-06-08 11:47:17

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, 2008-06-08 at 13:30 +0200, Sam Ravnborg wrote:
> I will name it ASMDIR and set it to 'y' if we shall install to asm/

That works if we can't just make the top-level Makefile set $(dst) for
itself. Which perhaps we could, if we stopped it running the whole
recursive installation on include/ for each arch, and instead made it
run only on include/asm-$(ARCH) for each arch.

And do a separate run for the other directories, of course -- probably
with $(ARCH) and $(SRCARCH) set to empty or something.

That also might help shave a little more time off by not repeating the
export 20 times for every non-asm subdirectory. (Not that we actually
_do_ repeat it, but even the make invocation and the dependency checks
take time).

--
dwmw2

2008-06-08 12:14:38

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, 2008-06-08 at 12:47 +0100, David Woodhouse wrote:
> On Sun, 2008-06-08 at 13:30 +0200, Sam Ravnborg wrote:
> > I will name it ASMDIR and set it to 'y' if we shall install to asm/
>
> That works if we can't just make the top-level Makefile set $(dst) for
> itself. Which perhaps we could, if we stopped it running the whole
> recursive installation on include/ for each arch, and instead made it
> run only on include/asm-$(ARCH) for each arch.
>
> And do a separate run for the other directories, of course -- probably
> with $(ARCH) and $(SRCARCH) set to empty or something.
>
> That also might help shave a little more time off by not repeating the
> export 20 times for every non-asm subdirectory. (Not that we actually
> _do_ repeat it, but even the make invocation and the dependency checks
> take time).

Something like this takes the time for 'headers_install_all' from 30s to
20s on my machine, and from 9s to 1.5s when it doesn't actually have to
do anything. I have yet to actually remove $(BIARCH) and just override
$(dst), though.

diff --git a/Makefile b/Makefile
index eff8fee..4eae126 100644
--- a/Makefile
+++ b/Makefile
@@ -1017,6 +1017,14 @@ headers_install_all: __headers
BIASMDIR=-bi-$$arch ;\
done

+PHONY += headers_install_all_faster
+headers_install_all_faster: __headers
+ $(Q)$(MAKE) ARCH= SRCARCH= $(hdr-inst)=include
+ $(Q)set -e; for arch in $(hdr-archs); do \
+ $(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include/asm-$$arch \
+ BIASMDIR=-bi-$$arch ;\
+ done
+
PHONY += headers_install
headers_install: __headers
$(Q)if [ ! -r $(srctree)/include/asm-$(SRCARCH)/Kbuild ]; then \
diff --git a/include/Kbuild b/include/Kbuild
index b522887..9393f7e 100644
--- a/include/Kbuild
+++ b/include/Kbuild
@@ -5,4 +5,6 @@ header-y += mtd/
header-y += rdma/
header-y += video/

+ifneq ($(ARCH),)
header-y += asm-$(ARCH)/
+endif

--
dwmw2

2008-06-08 12:23:38

by Oleg Verych

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

> Correct. A quick grep shows that we have these different
> uses of __KERNEL__:
>
> #if defined(__ARM_EABI__) && !defined(__KERNEL__)
> #if !defined(CONFIG_M68K) || !defined(__KERNEL__)
> #if defined(CONFIG_VIDEO_V4L1_COMPAT) || !defined (__KERNEL__)
> #if defined(__GNUC__) && !defined(__STRICT_ANSI__) || defined(__KERNEL__)
> #if defined(__KERNEL__)

> #ifndef __KERNEL__
>
> #else and #endif filtered away.
>
> A script needs to take into account other preprocessor
> uses too due to their nested nature.
> But doable I'm sure.
>
> And I rather have 100 lines perl than use the unifdef utility
> because we then have it collected in one place and can do even
> stricter validation.

I've suggested some time ago to adopt a coding style for simple

#if defined(__KERNEL__)
#ifndef __KERNEL__

this will be much clearner as for developers to know the scope by
using simple /* __KERNEL__ */ /* !__KERNEL__ */, and `sed`
script for that would be from 1 to 4 lines.

--
sed 'sed && sh + olecom = love' << ''
-o--=O`C
#oo'L O
<___=E M

2008-06-08 12:30:14

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, 2008-06-08 at 13:14 +0100, David Woodhouse wrote:
> I have yet to actually remove $(BIARCH) and just override $(dst),
> though.

Done that now, although headers_check_all is still not fixed to cope
with it...

diff --git a/Makefile b/Makefile
index eff8fee..1376c33 100644
--- a/Makefile
+++ b/Makefile
@@ -1012,9 +1012,9 @@ __headers: include/linux/version.h scripts_basic FORCE

PHONY += headers_install_all
headers_install_all: __headers
+ $(Q)$(MAKE) ARCH= SRCARCH= $(hdr-inst)=include
$(Q)set -e; for arch in $(hdr-archs); do \
- $(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include \
- BIASMDIR=-bi-$$arch ;\
+ $(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include/asm-$$arch; \
done

PHONY += headers_install
@@ -1029,7 +1029,7 @@ PHONY += headers_check_all
headers_check_all: headers_install_all
$(Q)set -e; for arch in $(hdr-archs); do \
$(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include \
- BIASMDIR=-bi-$$arch HDRCHECK=1 ;\
+ HDRCHECK=1 ;\
done

PHONY += headers_check
diff --git a/include/Kbuild b/include/Kbuild
index b522887..9393f7e 100644
--- a/include/Kbuild
+++ b/include/Kbuild
@@ -5,4 +5,6 @@ header-y += mtd/
header-y += rdma/
header-y += video/

+ifneq ($(ARCH),)
header-y += asm-$(ARCH)/
+endif
diff --git a/scripts/Makefile.headersinst b/scripts/Makefile.headersinst
index 80df6f3..e96d18e 100644
--- a/scripts/Makefile.headersinst
+++ b/scripts/Makefile.headersinst
@@ -14,15 +14,6 @@ include $(kbuild-file)

include scripts/Kbuild.include

-# If this is include/asm-$(ARCH) then override $(_dst) so that
-# we install to include/asm directly.
-# Unless $(BIASMDIR) is set, in which case we're probably doing
-# a 'headers_install_all' build and we should keep the -$(ARCH)
-# in the directory name.
-ifeq ($(obj),include/asm-$(ARCH)$(BIASMDIR))
- _dst := include/asm
-endif
-
install := $(INSTALL_HDR_PATH)/$(_dst)

header-y := $(sort $(header-y) $(unifdef-y))
@@ -88,8 +79,12 @@ endif
# Recursion
hdr-inst := -rR -f $(srctree)/scripts/Makefile.headersinst obj
.PHONY: $(subdirs)
+
+# If the next directory is include/asm-$(ARCH) then override
+# $(_dst) so that we install to include/asm directly.
$(subdirs):
- $(Q)$(MAKE) $(hdr-inst)=$(obj)/$@ dst=$(_dst)/$@
+ $(Q)$(MAKE) $(hdr-inst)=$(obj)/$@ \
+ dst=$(patsubst include/asm-$(ARCH),include/asm,$(_dst)/$@)

targets := $(wildcard $(sort $(targets)))
cmd_files := $(wildcard \

--
dwmw2

2008-06-08 13:17:19

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, 2008-06-08 at 13:17 +0200, Sam Ravnborg wrote:
> #if !defined(CONFIG_M68K) || !defined(__KERNEL__)

That's just scary, and broken for m68k where in userspace neither
CONFIG_M68K nor __KERNEL__ will be defined, so the unwanted ac_ahz
member will actually show up and break the binary compatibility.

Assuming we _don't_ want the ac_ahz member to be included on m68k, this
should fix it (is __mc68000__ the right thing to use?)...

diff --git a/include/linux/acct.h b/include/linux/acct.h
index e8cae54..228473b 100644
--- a/include/linux/acct.h
+++ b/include/linux/acct.h
@@ -58,8 +58,7 @@ struct acct
comp_t ac_minflt; /* Minor Pagefaults */
comp_t ac_majflt; /* Major Pagefaults */
comp_t ac_swaps; /* Number of Swaps */
-/* m68k had no padding here. */
-#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
+#ifndef __mc68000__ /* m68k had no padding here. */
__u16 ac_ahz; /* AHZ */
#endif
__u32 ac_exitcode; /* Exitcode */




--
dwmw2

2008-06-08 17:08:06

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

> Assuming we _don't_ want the ac_ahz member to be included on m68k, this
> should fix it (is __mc68000__ the right thing to use?)...

I think __m68k__ is preferred, it's harder to confuse for one of the
tuning symbols (that is, __mc68040__ and friends).

It should function identically though, both __m68k__ and __mc68000__
(and __mc68000 and mc68000) are defined under exactly the same
conditions.


Segher

2008-06-08 17:34:46

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

Segher Boessenkool <[email protected]> writes:

>> Assuming we _don't_ want the ac_ahz member to be included on m68k, this
>> should fix it (is __mc68000__ the right thing to use?)...
>
> I think __m68k__ is preferred, it's harder to confuse for one of the
> tuning symbols (that is, __mc68040__ and friends).

__m68k__ wasn't pre-defined by gcc before 3.4.

Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstra?e 5, 90409 N?rnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2008-06-08 19:47:06

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On 8 jun 2008, at 19:34, Andreas Schwab wrote:
>>> Assuming we _don't_ want the ac_ahz member to be included on m68k,
>>> this
>>> should fix it (is __mc68000__ the right thing to use?)...
>>
>> I think __m68k__ is preferred, it's harder to confuse for one of the
>> tuning symbols (that is, __mc68040__ and friends).
>
> __m68k__ wasn't pre-defined by gcc before 3.4.

Darn, I knew I should have checked the history. __mc68000__ is
the best thing to use, then, some people still use 3.3 (don't ask
me why).


Segher

2008-06-08 20:06:55

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, Jun 08, 2008 at 11:47:30AM +0200, Sam Ravnborg wrote:
> This is just a heads up patch if anyone is interested.
> I finally took the time needed to optimize the
> make headers_* targets.

Based on inputs from various people I have now updated the patch.
A proper serie with a few preparational patches will be posted
and they are aiming for kbuild-next.

The patchset:

kbuild: refactor headers_* targets in Makefile
kbuild: always unifdef files in headers_install*
kbuild: drop support of ALTARCH for headers_*
kbuild: code refactoring in Makefile.headerinst
kbuild: error out early in make headers_install
kbuild: optimize headers_* targets

The meat of the patch is in the last patch which contains all
the real improvements.

Sam

2008-06-08 20:07:27

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH 4/6] kbuild: code refactoring in Makefile.headerinst

No functional changes just improved readability

Signed-off-by: Sam Ravnborg <[email protected]>
---
scripts/Makefile.headersinst | 64 +++++++++++++++++++++++-------------------
1 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/scripts/Makefile.headersinst b/scripts/Makefile.headersinst
index 1fb8c00..599adc6 100644
--- a/scripts/Makefile.headersinst
+++ b/scripts/Makefile.headersinst
@@ -10,7 +10,7 @@
UNIFDEF := scripts/unifdef -U__KERNEL__

# Eliminate the contents of (and inclusions of) compiler.h
-HDRSED := sed -e "s/ inline / __inline__ /g" \
+HDRSED := sed -e "s/ inline / __inline__ /g" \
-e "s/[[:space:]]__user[[:space:]]\{1,\}/ /g" \
-e "s/(__user[[:space:]]\{1,\}/ (/g" \
-e "s/[[:space:]]__force[[:space:]]\{1,\}/ /g" \
@@ -37,6 +37,8 @@ ifeq ($(obj),include/asm-$(ARCH)$(BIASMDIR))
_dst := include/asm
endif

+install := $(INSTALL_HDR_PATH)/$(_dst)
+
header-y := $(sort $(header-y) $(unifdef-y))
subdir-y := $(patsubst %/,%,$(filter %/, $(header-y)))
header-y := $(filter-out %/, $(header-y))
@@ -45,34 +47,34 @@ header-y := $(filter-out %/, $(header-y))
check-y := $(patsubst %,.check.%,$(header-y) $(objhdr-y))

# Work out what needs to be removed
-oldheaders := $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,%,$(wildcard $(INSTALL_HDR_PATH)/$(_dst)/*.h))
-unwanted := $(filter-out $(header-y) $(objhdr-y),$(oldheaders))
+oldheaders := $(patsubst $(install)/%,%,$(wildcard $(install)/*.h))
+unwanted := $(filter-out $(header-y) $(objhdr-y),$(oldheaders))

-oldcheckstamps := $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,%,$(wildcard $(INSTALL_HDR_PATH)/$(_dst)/.check.*.h))
-unwanted += $(filter-out $(check-y),$(oldcheckstamps))
+oldcheckstamps := $(patsubst $(install)/%,%,$(wildcard $(install)/.check.*.h))
+unwanted += $(filter-out $(check-y),$(oldcheckstamps))

# Prefix them all with full paths to $(INSTALL_HDR_PATH)
-header-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(header-y))
-objhdr-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(objhdr-y))
-check-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(check-y))
+header-y := $(patsubst %,$(install)/%,$(header-y))
+objhdr-y := $(patsubst %,$(install)/%,$(objhdr-y))
+check-y := $(patsubst %,$(install)/%,$(check-y))

-quiet_cmd_o_hdr_install = INSTALL $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_o_hdr_install = cp $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,$(objtree)/$(obj)/%,$@) \
- $(INSTALL_HDR_PATH)/$(_dst)
+quiet_cmd_o_hdr_install = INSTALL $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
+ cmd_o_hdr_install = cp $(patsubst $(install)/%,$(objtree)/$(obj)/%,$@) \
+ $(install)

-quiet_cmd_unifdef = UNIFDEF $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_unifdef = $(UNIFDEF) $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,$(srctree)/$(obj)/%,$@) \
- | $(HDRSED) > $@ || :
+quiet_cmd_unifdef = UNIFDEF $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
+ cmd_unifdef = $(UNIFDEF) $(patsubst $(install)/%,$(srctree)/$(obj)/%,$@)\
+ | $(HDRSED) > $@ || :

-quiet_cmd_check = CHECK $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/.check.%,$(_dst)/%,$@)
- cmd_check = $(CONFIG_SHELL) $(srctree)/scripts/hdrcheck.sh \
- $(INSTALL_HDR_PATH)/include $(subst /.check.,/,$@) $@
+quiet_cmd_check = CHECK $(patsubst $(install)/.check.%,$(_dst)/%,$@)
+ cmd_check = $(CONFIG_SHELL) $(srctree)/scripts/hdrcheck.sh \
+ $(INSTALL_HDR_PATH)/include $(subst /.check.,/,$@) $@

-quiet_cmd_remove = REMOVE $(_dst)/$@
- cmd_remove = rm -f $(INSTALL_HDR_PATH)/$(_dst)/$@
+quiet_cmd_remove = REMOVE $(_dst)/$@
+ cmd_remove = rm -f $(install)/$@

-quiet_cmd_mkdir = MKDIR $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_mkdir = mkdir -p $@
+quiet_cmd_mkdir = MKDIR $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
+ cmd_mkdir = mkdir -p $@

.PHONY: __headersinst __headerscheck

@@ -80,13 +82,14 @@ ifdef HDRCHECK
__headerscheck: $(subdir-y) $(check-y)
@true

-$(check-y) : $(INSTALL_HDR_PATH)/$(_dst)/.check.%.h : $(INSTALL_HDR_PATH)/$(_dst)/%.h
+$(check-y) : $(install)/.check.%.h : $(install)/%.h
$(call cmd,check)

# Other dependencies for $(check-y)
include /dev/null $(wildcard $(check-y))

-# ... but leave $(check-y) as .PHONY for now until those deps are actually correct.
+# but leave $(check-y) as .PHONY for now until those
+# deps are actually correct.
.PHONY: $(check-y)

else
@@ -94,26 +97,29 @@ else
__headersinst: $(subdir-y) $(header-y) $(objhdr-y)
@true

-$(objhdr-y) $(subdir-y) $(header-y): | $(INSTALL_HDR_PATH)/$(_dst) $(unwanted)
+$(objhdr-y) $(subdir-y) $(header-y): | $(install) $(unwanted)

-$(INSTALL_HDR_PATH)/$(_dst):
+$(install):
$(call cmd,mkdir)

+# Rules for removing unwanted header files
.PHONY: $(unwanted)
$(unwanted):
$(call cmd,remove)

-$(objhdr-y): $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(objtree)/$(obj)/%.h $(kbuild-file)
+# Install generated files
+$(objhdr-y): $(install)/%.h: $(objtree)/$(obj)/%.h $(kbuild-file)
$(call cmd,o_hdr_install)

-$(header-y): $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(srctree)/$(obj)/%.h $(kbuild-file)
+# Unifdef header files and install them
+$(header-y): $(install)/%.h: $(srctree)/$(obj)/%.h $(kbuild-file)
$(call cmd,unifdef)

endif

-hdrinst := -rR -f $(srctree)/scripts/Makefile.headersinst obj
+hdr-inst := -rR -f $(srctree)/scripts/Makefile.headersinst obj

# Recursion
.PHONY: $(subdir-y)
$(subdir-y):
- $(Q)$(MAKE) $(hdrinst)=$(obj)/$@ dst=$(_dst)/$@ rel=../$(rel)
+ $(Q)$(MAKE) $(hdr-inst)=$(obj)/$@ dst=$(_dst)/$@
--
1.5.4.1.143.ge7e51

2008-06-08 20:07:52

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH 3/6] kbuild: drop support of ALTARCH for headers_*

ALTARCH is no longer used by any arch(*) so drop
support for this from Makefile.headerinst

Dropping ALTARCH support simplifies Makefile.headerinst

(*) sparc64 uses it but work is ongoing to drop it
and no furter usage is planned.

Signed-off-by: Sam Ravnborg <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: David Miller <[email protected]>
---
scripts/Makefile.headersinst | 84 ++++-------------------------------------
1 files changed, 9 insertions(+), 75 deletions(-)

diff --git a/scripts/Makefile.headersinst b/scripts/Makefile.headersinst
index 22b17af..1fb8c00 100644
--- a/scripts/Makefile.headersinst
+++ b/scripts/Makefile.headersinst
@@ -23,30 +23,17 @@ HDRSED := sed -e "s/ inline / __inline__ /g" \

_dst := $(if $(dst),$(dst),$(obj))

-ifeq (,$(patsubst include/asm/%,,$(obj)/))
-# For producing the generated stuff in include/asm for biarch builds, include
-# both sets of Kbuild files; we'll generate anything which is mentioned in
-# _either_ arch, and recurse into subdirectories which are mentioned in either
-# arch. Since some directories may exist in one but not the other, we must
-# use $(wildcard...).
-GENASM := 1
-archasm := $(subst include/asm,asm-$(ARCH),$(obj))
-altarchasm := $(subst include/asm,asm-$(ALTARCH),$(obj))
-KBUILDFILES := $(wildcard $(srctree)/include/$(archasm)/Kbuild $(srctree)/include/$(altarchasm)/Kbuild)
-else
-KBUILDFILES := $(srctree)/$(obj)/Kbuild
-endif
+kbuild-file := $(srctree)/$(obj)/Kbuild
+include $(kbuild-file)

-include $(KBUILDFILES)
+include scripts/Kbuild.include

-include scripts/Kbuild.include
-
-# If this is include/asm-$(ARCH) and there's no $(ALTARCH), then
-# override $(_dst) so that we install to include/asm directly.
+# If this is include/asm-$(ARCH) then override $(_dst) so that
+# we install to include/asm directly.
# Unless $(BIASMDIR) is set, in which case we're probably doing
# a 'headers_install_all' build and we should keep the -$(ARCH)
# in the directory name.
-ifeq ($(obj)$(ALTARCH),include/asm-$(ARCH)$(BIASMDIR))
+ifeq ($(obj),include/asm-$(ARCH)$(BIASMDIR))
_dst := include/asm
endif

@@ -69,18 +56,6 @@ header-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(header-y))
objhdr-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(objhdr-y))
check-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(check-y))

-
-ifdef ALTARCH
-ifeq ($(obj),include/asm-$(ARCH))
-altarch-y := altarch-dir
-endif
-endif
-
-# Make the definitions visible for recursive make invocations
-export ALTARCH
-export ARCHDEF
-export ALTARCHDEF
-
quiet_cmd_o_hdr_install = INSTALL $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
cmd_o_hdr_install = cp $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,$(objtree)/$(obj)/%,$@) \
$(INSTALL_HDR_PATH)/$(_dst)
@@ -99,34 +74,6 @@ quiet_cmd_remove = REMOVE $(_dst)/$@
quiet_cmd_mkdir = MKDIR $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
cmd_mkdir = mkdir -p $@

-quiet_cmd_gen = GEN $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_gen = \
-FNAME=$(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,%,$@); \
-STUBDEF=__ASM_STUB_`echo $$FNAME | tr a-z.- A-Z__`; \
-(echo "/* File autogenerated by 'make headers_install' */" ; \
-echo "\#ifndef $$STUBDEF" ; \
-echo "\#define $$STUBDEF" ; \
-echo "\# if $(ARCHDEF)" ; \
-if [ -r $(subst /$(_dst)/,/include/$(archasm)/,$@) ]; then \
- echo "\# include <$(archasm)/$$FNAME>" ; \
-else \
- echo "\# error $(archasm)/$$FNAME does not exist in" \
- "the $(ARCH) architecture" ; \
-fi ; \
-echo "\# elif $(ALTARCHDEF)" ; \
-if [ -r $(subst /$(_dst)/,/include/$(altarchasm)/,$@) ]; then \
- echo "\# include <$(altarchasm)/$$FNAME>" ; \
-else \
- echo "\# error $(altarchasm)/$$FNAME does not exist in" \
- "the $(ALTARCH) architecture" ; \
-fi ; \
-echo "\# else" ; \
-echo "\# warning This machine appears to be" \
- "neither $(ARCH) nor $(ALTARCH)." ; \
-echo "\# endif" ; \
-echo "\#endif /* $$STUBDEF */" ; \
-) > $@
-
.PHONY: __headersinst __headerscheck

ifdef HDRCHECK
@@ -144,7 +91,7 @@ include /dev/null $(wildcard $(check-y))

else
# Rules for installing headers
-__headersinst: $(subdir-y) $(header-y) $(altarch-y) $(objhdr-y)
+__headersinst: $(subdir-y) $(header-y) $(objhdr-y)
@true

$(objhdr-y) $(subdir-y) $(header-y): | $(INSTALL_HDR_PATH)/$(_dst) $(unwanted)
@@ -156,29 +103,16 @@ $(INSTALL_HDR_PATH)/$(_dst):
$(unwanted):
$(call cmd,remove)

-ifdef GENASM
-$(objhdr-y) $(header-y): $(KBUILDFILES)
- $(call cmd,gen)
-
-else
-$(objhdr-y): $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(objtree)/$(obj)/%.h $(KBUILDFILES)
+$(objhdr-y): $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(objtree)/$(obj)/%.h $(kbuild-file)
$(call cmd,o_hdr_install)

-$(header-y): $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(srctree)/$(obj)/%.h $(KBUILDFILES)
+$(header-y): $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(srctree)/$(obj)/%.h $(kbuild-file)
$(call cmd,unifdef)

endif
-endif

hdrinst := -rR -f $(srctree)/scripts/Makefile.headersinst obj

-.PHONY: altarch-dir
-# All the files in the normal arch dir must be created first, since we test
-# for their existence.
-altarch-dir: $(subdir-y) $(header-y) $(objhdr-y)
- $(Q)$(MAKE) $(hdrinst)=include/asm-$(ALTARCH) dst=include/asm-$(ALTARCH)
- $(Q)$(MAKE) $(hdrinst)=include/asm dst=include/asm$(BIASMDIR)
-
# Recursion
.PHONY: $(subdir-y)
$(subdir-y):
--
1.5.4.1.143.ge7e51

2008-06-08 20:08:17

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH 5/6] kbuild: error out early in make headers_install

Fix the a.out.h case by setting SRCARCH and error
out early in case of an error.
The a.out.h case failed with the *_all targets.

Signed-off-by: Sam Ravnborg <[email protected]>
---
Makefile | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 2630f5f..8f8fd6f 100644
--- a/Makefile
+++ b/Makefile
@@ -1012,8 +1012,9 @@ __headers: include/linux/version.h scripts_basic FORCE

PHONY += headers_install_all
headers_install_all: __headers
- $(Q)for arch in $(hdr-archs); do \
- $(MAKE) ARCH=$$arch $(hdr-inst)=include BIASMDIR=-bi-$$arch ;\
+ $(Q)set -e; for arch in $(hdr-archs); do \
+ $(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include \
+ BIASMDIR=-bi-$$arch ;\
done

PHONY += headers_install
@@ -1026,8 +1027,9 @@ headers_install: __headers

PHONY += headers_check_all
headers_check_all: headers_install_all
- $(Q)for arch in $(hdr-archs); do \
- $(MAKE) ARCH=$$arch $(hdr-inst)=include BIASMDIR=-bi-$$arch HDRCHECK=1 ;\
+ $(Q)set -e; for arch in $(hdr-archs); do \
+ $(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include \
+ BIASMDIR=-bi-$$arch HDRCHECK=1 ;\
done

PHONY += headers_check
--
1.5.4.1.143.ge7e51

2008-06-08 20:08:31

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH 1/6] kbuild: refactor headers_* targets in Makefile

o Use lower case for local variables
o Add a helper target for common targets
o Use $(hdr-inst)= ... to make Make invocations simpler
o Add -rR to make invocations

In total this adds more lines than it removes but the
benefit is better readability

Signed-off-by: Sam Ravnborg <[email protected]>
Cc: David Woodhouse <[email protected]>
---
Makefile | 39 +++++++++++++++++++++++----------------
1 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/Makefile b/Makefile
index 8db70fe..2630f5f 100644
--- a/Makefile
+++ b/Makefile
@@ -996,36 +996,43 @@ depend dep:

# ---------------------------------------------------------------------------
# Kernel headers
-INSTALL_HDR_PATH=$(objtree)/usr
-export INSTALL_HDR_PATH

-HDRFILTER=generic i386 x86_64
-HDRARCHES=$(filter-out $(HDRFILTER),$(patsubst $(srctree)/include/asm-%/Kbuild,%,$(wildcard $(srctree)/include/asm-*/Kbuild)))
+#Default location for installed headers
+export INSTALL_HDR_PATH = $(objtree)/usr

-PHONY += headers_install_all
-headers_install_all: include/linux/version.h scripts_basic FORCE
+hdr-filter := generic um ppc
+hdr-archs := $(filter-out $(hdr-filter), \
+ $(patsubst $(srctree)/include/asm-%/Kbuild,%, \
+ $(wildcard $(srctree)/include/asm-*/Kbuild)))
+hdr-inst := -rR -f $(srctree)/scripts/Makefile.headersinst obj
+
+PHONY += __headers
+__headers: include/linux/version.h scripts_basic FORCE
$(Q)$(MAKE) $(build)=scripts scripts/unifdef
- $(Q)for arch in $(HDRARCHES); do \
- $(MAKE) ARCH=$$arch -f $(srctree)/scripts/Makefile.headersinst obj=include BIASMDIR=-bi-$$arch ;\
+
+PHONY += headers_install_all
+headers_install_all: __headers
+ $(Q)for arch in $(hdr-archs); do \
+ $(MAKE) ARCH=$$arch $(hdr-inst)=include BIASMDIR=-bi-$$arch ;\
done

PHONY += headers_install
-headers_install: include/linux/version.h scripts_basic FORCE
- @if [ ! -r $(srctree)/include/asm-$(SRCARCH)/Kbuild ]; then \
+headers_install: __headers
+ $(Q)if [ ! -r $(srctree)/include/asm-$(SRCARCH)/Kbuild ]; then \
echo '*** Error: Headers not exportable for this architecture ($(SRCARCH))'; \
- exit 1 ; fi
- $(Q)$(MAKE) $(build)=scripts scripts/unifdef
- $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.headersinst ARCH=$(SRCARCH) obj=include
+ exit 1 ; \
+ fi
+ $(Q)$(MAKE) $(hdr-inst)=include ARCH=$(SRCARCH)

PHONY += headers_check_all
headers_check_all: headers_install_all
- $(Q)for arch in $(HDRARCHES); do \
- $(MAKE) ARCH=$$arch -f $(srctree)/scripts/Makefile.headersinst obj=include BIASMDIR=-bi-$$arch HDRCHECK=1 ;\
+ $(Q)for arch in $(hdr-archs); do \
+ $(MAKE) ARCH=$$arch $(hdr-inst)=include BIASMDIR=-bi-$$arch HDRCHECK=1 ;\
done

PHONY += headers_check
headers_check: headers_install
- $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.headersinst ARCH=$(SRCARCH) obj=include HDRCHECK=1
+ $(Q)$(MAKE) $(hdr-inst)=include ARCH=$(SRCARCH) HDRCHECK=1

# ---------------------------------------------------------------------------
# Modules
--
1.5.4.1.143.ge7e51

2008-06-08 20:08:46

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH 6/6] kbuild: optimize headers_* targets

This patch bring the time to execute make headers_* down
to an accetable level with acceptable amount of output lines.

On my box we went from ~30 seconds down to ~10 seconds
for a "make headers_check" with all headers being installed.
The majority of the time is now spent in unifdef.

The patch introduces two perl scripts.
One is used for the install phase, and the other is used for the
check phase. They both receive a list of files to operate on
and process one directory at a time.

The headers_install.pl script will hopefully one day include
the functionality of unifdef utility to speed up the process.

And the headers_check.pl will most likely be extended to
cover additional type of checks.

Vegard Nossum <[email protected]> gave a lot
of feedback to the perl scripts.
David Woodhouse <[email protected]> contributed
with additional inputs that simplified the Makefile.

Signed-off-by: Sam Ravnborg <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Vegard Nossum <[email protected]
---
Makefile | 28 +++++----
include/Kbuild | 4 +-
scripts/Makefile.headersinst | 140 ++++++++++++++++-------------------------
scripts/hdrcheck.sh | 10 ---
scripts/headers_check.pl | 56 +++++++++++++++++
scripts/headers_install.pl | 43 +++++++++++++
6 files changed, 171 insertions(+), 110 deletions(-)
delete mode 100755 scripts/hdrcheck.sh
create mode 100644 scripts/headers_check.pl
create mode 100644 scripts/headers_install.pl

diff --git a/Makefile b/Makefile
index 8f8fd6f..088751a 100644
--- a/Makefile
+++ b/Makefile
@@ -1000,7 +1000,7 @@ depend dep:
#Default location for installed headers
export INSTALL_HDR_PATH = $(objtree)/usr

-hdr-filter := generic um ppc
+hdr-filter := generic um ppc sparc64
hdr-archs := $(filter-out $(hdr-filter), \
$(patsubst $(srctree)/include/asm-%/Kbuild,%, \
$(wildcard $(srctree)/include/asm-*/Kbuild)))
@@ -1012,29 +1012,31 @@ __headers: include/linux/version.h scripts_basic FORCE

PHONY += headers_install_all
headers_install_all: __headers
+ $(Q)$(MAKE) $(hdr-inst)=include
$(Q)set -e; for arch in $(hdr-archs); do \
- $(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include \
- BIASMDIR=-bi-$$arch ;\
+ $(MAKE) $(hdr-inst)=include/asm-$$arch \
+ SRCARCH=$$arch dst=include/asm-$$arch; \
done

PHONY += headers_install
headers_install: __headers
- $(Q)if [ ! -r $(srctree)/include/asm-$(SRCARCH)/Kbuild ]; then \
- echo '*** Error: Headers not exportable for this architecture ($(SRCARCH))'; \
- exit 1 ; \
- fi
- $(Q)$(MAKE) $(hdr-inst)=include ARCH=$(SRCARCH)
+ $(if $(wildcard $(srctree)/include/asm-$(SRCARCH)/Kbuild),, \
+ $(error Headers not exportable for this architecture ($(SRCARCH))))
+ $(Q)$(MAKE) $(hdr-inst)=include
+ $(Q)$(MAKE) $(hdr-inst)=include/asm-$(SRCARCH) dst=include/asm

PHONY += headers_check_all
-headers_check_all: headers_install_all
- $(Q)set -e; for arch in $(hdr-archs); do \
- $(MAKE) ARCH=$$arch SRCARCH=$$arch $(hdr-inst)=include \
- BIASMDIR=-bi-$$arch HDRCHECK=1 ;\
+headers_check_all: headers_install headers_install_all
+ $(Q)$(MAKE) $(hdr-inst)=include HDRCHECK=1
+ $(Q)for arch in $(hdr-archs); do \
+ $(MAKE) SRCARCH=$$arch $(hdr-inst)=include/asm-$$arch HDRCHECK=1 ;\
done

PHONY += headers_check
headers_check: headers_install
- $(Q)$(MAKE) $(hdr-inst)=include ARCH=$(SRCARCH) HDRCHECK=1
+ $(Q)$(MAKE) $(hdr-inst)=include HDRCHECK=1
+ $(Q)$(MAKE) $(hdr-inst)=include/asm-$(SRCARCH) \
+ dst=include/asm HDRCHECK=1

# ---------------------------------------------------------------------------
# Modules
diff --git a/include/Kbuild b/include/Kbuild
index b522887..6ae595c 100644
--- a/include/Kbuild
+++ b/include/Kbuild
@@ -1,8 +1,8 @@
+# Top-level Makefile calls into asm-$(ARCH)
+
header-y += asm-generic/
header-y += linux/
header-y += sound/
header-y += mtd/
header-y += rdma/
header-y += video/
-
-header-y += asm-$(ARCH)/
diff --git a/scripts/Makefile.headersinst b/scripts/Makefile.headersinst
index 599adc6..aa1965d 100644
--- a/scripts/Makefile.headersinst
+++ b/scripts/Makefile.headersinst
@@ -7,20 +7,6 @@
#
# ==========================================================================

-UNIFDEF := scripts/unifdef -U__KERNEL__
-
-# Eliminate the contents of (and inclusions of) compiler.h
-HDRSED := sed -e "s/ inline / __inline__ /g" \
- -e "s/[[:space:]]__user[[:space:]]\{1,\}/ /g" \
- -e "s/(__user[[:space:]]\{1,\}/ (/g" \
- -e "s/[[:space:]]__force[[:space:]]\{1,\}/ /g" \
- -e "s/(__force[[:space:]]\{1,\}/ (/g" \
- -e "s/[[:space:]]__iomem[[:space:]]\{1,\}/ /g" \
- -e "s/(__iomem[[:space:]]\{1,\}/ (/g" \
- -e "s/[[:space:]]__attribute_const__[[:space:]]\{1,\}/\ /g" \
- -e "s/[[:space:]]__attribute_const__$$//" \
- -e "/^\#include <linux\/compiler.h>/d"
-
_dst := $(if $(dst),$(dst),$(obj))

kbuild-file := $(srctree)/$(obj)/Kbuild
@@ -28,98 +14,82 @@ include $(kbuild-file)

include scripts/Kbuild.include

-# If this is include/asm-$(ARCH) then override $(_dst) so that
-# we install to include/asm directly.
-# Unless $(BIASMDIR) is set, in which case we're probably doing
-# a 'headers_install_all' build and we should keep the -$(ARCH)
-# in the directory name.
-ifeq ($(obj),include/asm-$(ARCH)$(BIASMDIR))
- _dst := include/asm
-endif
+install := $(INSTALL_HDR_PATH)/$(_dst)

-install := $(INSTALL_HDR_PATH)/$(_dst)
+header-y := $(sort $(header-y) $(unifdef-y))
+subdirs := $(patsubst %/,%,$(filter %/, $(header-y)))
+header-y := $(filter-out %/, $(header-y))

-header-y := $(sort $(header-y) $(unifdef-y))
-subdir-y := $(patsubst %/,%,$(filter %/, $(header-y)))
-header-y := $(filter-out %/, $(header-y))
+# files used to track state of install/check
+install-file := $(install)/.install
+check-file := $(install)/.check

-# stamp files for header checks
-check-y := $(patsubst %,.check.%,$(header-y) $(objhdr-y))
+# all headers files for this dir
+all-files := $(header-y) $(objhdr-y)

# Work out what needs to be removed
-oldheaders := $(patsubst $(install)/%,%,$(wildcard $(install)/*.h))
-unwanted := $(filter-out $(header-y) $(objhdr-y),$(oldheaders))
-
-oldcheckstamps := $(patsubst $(install)/%,%,$(wildcard $(install)/.check.*.h))
-unwanted += $(filter-out $(check-y),$(oldcheckstamps))
-
-# Prefix them all with full paths to $(INSTALL_HDR_PATH)
-header-y := $(patsubst %,$(install)/%,$(header-y))
-objhdr-y := $(patsubst %,$(install)/%,$(objhdr-y))
-check-y := $(patsubst %,$(install)/%,$(check-y))
+oldheaders := $(patsubst $(install)/%,%,$(wildcard $(install)/*.h))
+unwanted := $(filter-out $(all-files),$(oldheaders))

-quiet_cmd_o_hdr_install = INSTALL $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_o_hdr_install = cp $(patsubst $(install)/%,$(objtree)/$(obj)/%,$@) \
- $(install)
+# Prefix all with full paths to $(INSTALL_HDR_PATH)
+header-y := $(addprefix $(install)/, $(header-y))
+objhdr-y := $(addprefix $(install)/, $(objhdr-y))
+unwanted := $(addprefix $(install)/, $(unwanted))

-quiet_cmd_unifdef = UNIFDEF $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_unifdef = $(UNIFDEF) $(patsubst $(install)/%,$(srctree)/$(obj)/%,$@)\
- | $(HDRSED) > $@ || :
+printdir = $(patsubst $(INSTALL_HDR_PATH)/%/,%,$(dir $@))

-quiet_cmd_check = CHECK $(patsubst $(install)/.check.%,$(_dst)/%,$@)
- cmd_check = $(CONFIG_SHELL) $(srctree)/scripts/hdrcheck.sh \
- $(INSTALL_HDR_PATH)/include $(subst /.check.,/,$@) $@
+quiet_cmd_install = INSTALL $(printdir) ($(words $(all-files)) files)
+ cmd_install = $(PERL) $(srctree)/scripts/headers_install.pl \
+ $(obj) $(install) $(all-files)

-quiet_cmd_remove = REMOVE $(_dst)/$@
- cmd_remove = rm -f $(install)/$@
+quiet_cmd_remove = REMOVE $(patsubst $(INSTALL_HDR_PATH)/%,%,$(unwanted))
+ cmd_remove = rm -f $(unwanted)

-quiet_cmd_mkdir = MKDIR $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_mkdir = mkdir -p $@
+quiet_cmd_check = CHECK $(printdir) ($(words $(all-files)) files)
+ cmd_check = $(PERL) $(srctree)/scripts/headers_check.pl \
+ $(INSTALL_HDR_PATH)/include \
+ $(SRCARCH) \
+ $(addprefix $(install)/, $(all-files))

-.PHONY: __headersinst __headerscheck
+PHONY += __headersinst __headerscheck

ifdef HDRCHECK
-__headerscheck: $(subdir-y) $(check-y)
- @true
+__headerscheck: $(subdirs) $(check-file)
+ @:

-$(check-y) : $(install)/.check.%.h : $(install)/%.h
- $(call cmd,check)
-
-# Other dependencies for $(check-y)
-include /dev/null $(wildcard $(check-y))
-
-# but leave $(check-y) as .PHONY for now until those
-# deps are actually correct.
-.PHONY: $(check-y)
+targets += $(check-file)
+$(check-file): $(addprefix $(srctree)/$(obj)/,$(all-files)) FORCE
+ $(call if_changed,check)
+ $(Q)touch $@

else
# Rules for installing headers
-__headersinst: $(subdir-y) $(header-y) $(objhdr-y)
- @true
+__headersinst: $(subdirs) $(install-file)
+ @:

-$(objhdr-y) $(subdir-y) $(header-y): | $(install) $(unwanted)
+targets += $(install-file)
+$(install-file): $(addprefix $(srctree)/$(obj)/,$(all-files)) FORCE
+ $(if $(unwanted),$(call cmd,remove),)
+ $(if $(wildcard $(dir $@)),,$(shell mkdir -p $(dir $@)))
+ $(call if_changed,install)
+ $(Q)touch $@

-$(install):
- $(call cmd,mkdir)
-
-# Rules for removing unwanted header files
-.PHONY: $(unwanted)
-$(unwanted):
- $(call cmd,remove)
+endif

-# Install generated files
-$(objhdr-y): $(install)/%.h: $(objtree)/$(obj)/%.h $(kbuild-file)
- $(call cmd,o_hdr_install)
+# Recursion
+hdr-inst := -rR -f $(srctree)/scripts/Makefile.headersinst obj
+.PHONY: $(subdirs)
+$(subdirs):
+ $(Q)$(MAKE) $(hdr-inst)=$(obj)/$@ dst=$(_dst)/$@

-# Unifdef header files and install them
-$(header-y): $(install)/%.h: $(srctree)/$(obj)/%.h $(kbuild-file)
- $(call cmd,unifdef)
+targets := $(wildcard $(sort $(targets)))
+cmd_files := $(wildcard \
+ $(foreach f,$(targets),$(dir $(f)).$(notdir $(f)).cmd))

+ifneq ($(cmd_files),)
+ include $(cmd_files)
endif

-hdr-inst := -rR -f $(srctree)/scripts/Makefile.headersinst obj
-
-# Recursion
-.PHONY: $(subdir-y)
-$(subdir-y):
- $(Q)$(MAKE) $(hdr-inst)=$(obj)/$@ dst=$(_dst)/$@
+.PHONY: $(PHONY)
+PHONY += FORCE
+FORCE: ;
diff --git a/scripts/hdrcheck.sh b/scripts/hdrcheck.sh
deleted file mode 100755
index 3159858..0000000
--- a/scripts/hdrcheck.sh
+++ /dev/null
@@ -1,10 +0,0 @@
-#!/bin/sh
-
-for FILE in `grep '^[ \t]*#[ \t]*include[ \t]*<' $2 | cut -f2 -d\< | cut -f1 -d\> | egrep ^linux\|^asm` ; do
- if [ ! -r $1/$FILE ]; then
- echo $2 requires $FILE, which does not exist in exported headers
- exit 1
- fi
-done
-# FIXME: List dependencies into $3
-touch $3
diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl
new file mode 100644
index 0000000..15d53a6
--- /dev/null
+++ b/scripts/headers_check.pl
@@ -0,0 +1,56 @@
+#!/usr/bin/perl
+#
+# headers_check.pl execute a number of trivial consistency checks
+#
+# Usage: headers_check.pl dir [files...]
+# dir: dir to look for included files
+# arch: architecture
+# files: list of files to check
+#
+# The script reads the supplied files line by line and:
+#
+# 1) for each include statement it checks if the
+# included file actually exists.
+# Only include files located in asm* and linux* are checked.
+# The rest are assumed to be system include files.
+#
+# 2) TODO: check for leaked CONFIG_ symbols
+
+use strict;
+use warnings;
+
+my ($dir, $arch, @files) = @ARGV;
+
+my $ret = 0;
+my $line;
+my $lineno = 0;
+my $filename;
+
+foreach my $file (@files) {
+ $filename = $file;
+ open(my $fh, '<', "$filename") or die "$filename: $!\n";
+ $lineno = 0;
+ while ($line = <$fh>) {
+ $lineno++;
+ check_include();
+ }
+ close $fh;
+}
+exit $ret;
+
+sub check_include
+{
+ if ($line =~ m/^\s*#\s*include\s+<((asm|linux).*)>/) {
+ my $inc = $1;
+ my $found;
+ $found = stat($dir . "/" . $inc);
+ if (!$found) {
+ $inc =~ s#asm/#asm-$arch/#;
+ $found = stat($dir . "/" . $inc);
+ }
+ if (!$found) {
+ printf STDERR "$filename:$lineno: included file '$inc' is not exported\n";
+ $ret = 1;
+ }
+ }
+}
diff --git a/scripts/headers_install.pl b/scripts/headers_install.pl
new file mode 100644
index 0000000..283c055
--- /dev/null
+++ b/scripts/headers_install.pl
@@ -0,0 +1,43 @@
+#!/usr/bin/perl
+#
+# headers_install prepare the listed header files for use in
+# user space and copy the files to their destination.
+#
+# Usage: headers_install.pl odir installdir [files...]
+# odir: dir to open files
+# install: dir to install the files
+# files: list of files to check
+#
+# Step in preparation for users space:
+# 1) Drop all use of compiler.h definitions
+# 2) Drop include of compiler.h
+# 3) Drop all sections defined out by __KERNEL__ (using unifdef)
+
+use strict;
+use warnings;
+
+my ($odir, $installdir, @files) = @ARGV;
+
+my $ret = 0;
+my $unifdef = "scripts/unifdef -U__KERNEL__";
+
+foreach my $file (@files) {
+ my $tmpfile = "$installdir/$file.tmp";
+ open(my $infile, '<', "$odir/$file") or die "$odir/$file: $!\n";
+ open(my $outfile, '>', "$tmpfile") or die "$tmpfile: $!\n";
+ while (my $line = <$infile>) {
+ $line =~ s/([\s(])__user\s/$1/g;
+ $line =~ s/([\s(])__force\s/$1/g;
+ $line =~ s/([\s(])__iomem\s/$1/g;
+ $line =~ s/\s__attribute_const__\s/ /g;
+ $line =~ s/\s__attribute_const__$//g;
+ $line =~ s/^#include <linux\/compiler.h>//;
+ printf $outfile "%s", $line;
+ }
+ close $outfile;
+ close $infile;
+ $ret = system $unifdef . " $tmpfile > $installdir/$file";
+ unlink $tmpfile;
+}
+
+exit $ret;
--
1.5.4.1.143.ge7e51

2008-06-08 20:09:06

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH 2/6] kbuild: always unifdef files in headers_install*

unifdef utility is fast enough to warrant that we always
run the scripts through unifdef.

This patch runs all headers listed with header-y and unifdef-y
through unifdef.
Next step is to drop unifdef-y in all Kbuild files and
that can now be done in smaller steps.

Signed-off-by: Sam Ravnborg <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Adrian Bunk <[email protected]>
---
scripts/Makefile.headersinst | 29 ++++++++++-------------------
1 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/scripts/Makefile.headersinst b/scripts/Makefile.headersinst
index 53dae3e..22b17af 100644
--- a/scripts/Makefile.headersinst
+++ b/scripts/Makefile.headersinst
@@ -50,25 +50,22 @@ ifeq ($(obj)$(ALTARCH),include/asm-$(ARCH)$(BIASMDIR))
_dst := include/asm
endif

-header-y := $(sort $(header-y))
-unifdef-y := $(sort $(unifdef-y))
+header-y := $(sort $(header-y) $(unifdef-y))
subdir-y := $(patsubst %/,%,$(filter %/, $(header-y)))
header-y := $(filter-out %/, $(header-y))
-header-y := $(filter-out $(unifdef-y),$(header-y))

# stamp files for header checks
-check-y := $(patsubst %,.check.%,$(header-y) $(unifdef-y) $(objhdr-y))
+check-y := $(patsubst %,.check.%,$(header-y) $(objhdr-y))

# Work out what needs to be removed
oldheaders := $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,%,$(wildcard $(INSTALL_HDR_PATH)/$(_dst)/*.h))
-unwanted := $(filter-out $(header-y) $(unifdef-y) $(objhdr-y),$(oldheaders))
+unwanted := $(filter-out $(header-y) $(objhdr-y),$(oldheaders))

oldcheckstamps := $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,%,$(wildcard $(INSTALL_HDR_PATH)/$(_dst)/.check.*.h))
unwanted += $(filter-out $(check-y),$(oldcheckstamps))

# Prefix them all with full paths to $(INSTALL_HDR_PATH)
header-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(header-y))
-unifdef-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(unifdef-y))
objhdr-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(objhdr-y))
check-y := $(patsubst %,$(INSTALL_HDR_PATH)/$(_dst)/%,$(check-y))

@@ -88,10 +85,6 @@ quiet_cmd_o_hdr_install = INSTALL $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
cmd_o_hdr_install = cp $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,$(objtree)/$(obj)/%,$@) \
$(INSTALL_HDR_PATH)/$(_dst)

-quiet_cmd_headers_install = INSTALL $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
- cmd_headers_install = $(HDRSED) $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,$(srctree)/$(obj)/%,$@) \
- > $@
-
quiet_cmd_unifdef = UNIFDEF $(patsubst $(INSTALL_HDR_PATH)/%,%,$@)
cmd_unifdef = $(UNIFDEF) $(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,$(srctree)/$(obj)/%,$@) \
| $(HDRSED) > $@ || :
@@ -151,10 +144,10 @@ include /dev/null $(wildcard $(check-y))

else
# Rules for installing headers
-__headersinst: $(subdir-y) $(header-y) $(unifdef-y) $(altarch-y) $(objhdr-y)
+__headersinst: $(subdir-y) $(header-y) $(altarch-y) $(objhdr-y)
@true

-$(objhdr-y) $(subdir-y) $(header-y) $(unifdef-y): | $(INSTALL_HDR_PATH)/$(_dst) $(unwanted)
+$(objhdr-y) $(subdir-y) $(header-y): | $(INSTALL_HDR_PATH)/$(_dst) $(unwanted)

$(INSTALL_HDR_PATH)/$(_dst):
$(call cmd,mkdir)
@@ -164,18 +157,16 @@ $(unwanted):
$(call cmd,remove)

ifdef GENASM
-$(objhdr-y) $(header-y) $(unifdef-y): $(KBUILDFILES)
+$(objhdr-y) $(header-y): $(KBUILDFILES)
$(call cmd,gen)

else
-$(objhdr-y) : $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(objtree)/$(obj)/%.h $(KBUILDFILES)
+$(objhdr-y): $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(objtree)/$(obj)/%.h $(KBUILDFILES)
$(call cmd,o_hdr_install)

-$(header-y) : $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(srctree)/$(obj)/%.h $(KBUILDFILES)
- $(call cmd,headers_install)
-
-$(unifdef-y) : $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(srctree)/$(obj)/%.h $(KBUILDFILES)
+$(header-y): $(INSTALL_HDR_PATH)/$(_dst)/%.h: $(srctree)/$(obj)/%.h $(KBUILDFILES)
$(call cmd,unifdef)
+
endif
endif

@@ -184,7 +175,7 @@ hdrinst := -rR -f $(srctree)/scripts/Makefile.headersinst obj
.PHONY: altarch-dir
# All the files in the normal arch dir must be created first, since we test
# for their existence.
-altarch-dir: $(subdir-y) $(header-y) $(unifdef-y) $(objhdr-y)
+altarch-dir: $(subdir-y) $(header-y) $(objhdr-y)
$(Q)$(MAKE) $(hdrinst)=include/asm-$(ALTARCH) dst=include/asm-$(ALTARCH)
$(Q)$(MAKE) $(hdrinst)=include/asm dst=include/asm$(BIASMDIR)

--
1.5.4.1.143.ge7e51

2008-06-08 20:37:04

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, 8 Jun 2008, David Woodhouse wrote:
> On Sun, 2008-06-08 at 13:17 +0200, Sam Ravnborg wrote:
> > #if !defined(CONFIG_M68K) || !defined(__KERNEL__)
>
> That's just scary, and broken for m68k where in userspace neither
> CONFIG_M68K nor __KERNEL__ will be defined, so the unwanted ac_ahz
> member will actually show up and break the binary compatibility.
>
> Assuming we _don't_ want the ac_ahz member to be included on m68k, this
> should fix it (is __mc68000__ the right thing to use?)...
>
> diff --git a/include/linux/acct.h b/include/linux/acct.h
> index e8cae54..228473b 100644
> --- a/include/linux/acct.h
> +++ b/include/linux/acct.h
> @@ -58,8 +58,7 @@ struct acct
> comp_t ac_minflt; /* Minor Pagefaults */
> comp_t ac_majflt; /* Major Pagefaults */
> comp_t ac_swaps; /* Number of Swaps */
> -/* m68k had no padding here. */
> -#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
> +#ifndef __mc68000__ /* m68k had no padding here. */
> __u16 ac_ahz; /* AHZ */
> #endif
> __u32 ac_exitcode; /* Exitcode */
>

JFYI, this was introduced by (from the `full-history-linux' tree):

commit 83245ea9c7212315e2f265f60e966534ba97a08a
Author: Tim Schmielau <[email protected]>
Date: Thu Jun 17 18:01:23 2004 -0700

[PATCH] BSD accounting format rework

BSD accounting format rework:

Use all explicit and implicit padding in struct acct to

- correctly report 32 bit uid/gid,
- correctly report jobs (e.g., daemons) running longer than 497 days,
- increase the precision of ac_etime from 2^-13 to 2^-20
(i.e., from ~6 hours to ~1 min. after a year)
- store the current AHZ value.
- allow cross-platform processing of the accounting file
(limited for m68k which has a different size struct acct).
- introduce versioning for smooth transition to incompatible formats in
the future. Currently the following version numbers are defined:
0: old format (until 2.6.7) with 16 bit uid/gid
1: extended variant (binary compatible to v0 on M68K)
2: extended variant (binary compatible to v0 on everything except M68K)
3: a new binary incompatible format (64 bytes)
4: new binary incompatible format (128 bytes).
layout of its first 64 bytes is the same as for v3.
5: marks second half of new binary incompatible format (128 bytes)
(layout is not yet defined)

All this is accomplished without breaking binary compatibility. 32 bit
uid/gid support is compatible with the patch previously floating around and
used e.g. by Red Hat.

This patch also introduces a config option for a new, binary incompatible
"version 3" format that

- is uniform across and properly aligned on all platforms
- stores pid and ppid
- uses AHZ==100 on all platforms (allows to report longer times)

Much of the compatibility glue goes away when v1/v2 support is removed from
the kernel. Such a patch is at

http://www.physik3.uni-rostock.de/tim/kernel/2.7/acct-cleanup-04.patch

and might be applied in the 2.7 timeframe.

The new v3 format is source compatible with current GNU acct tools (6.3.5).
However, current GNU acct tools can be compiled for only one format. As the
is no way to pass the kernel configuration to userspace, with my patch it wi
still only support the old v2 format. Only if v1/v2 support is removed from
the kernel, recompiling GNU acct tools will yield v3 support.

A preliminary take at the corresponding work on cross-platform userspace too
(GNU acct package) is at

http://www.physik3.uni-rostock.de/tim/kernel/utils/acct/

This version of the package is able to read any of the v0/v2/v3 formats,
regardless of byte-order (untested), even within the same file.
Cross-platform compatibility with m68k (v1 format) is not yet implemented, b
native use on m68k should work (untested). pid and ppid are currently only
shown by the dump-acct utility.

Thanks to Arthur Corliss, Albert Cahalan and Ragnar Kjørstad for their
comments, and to Albert Cahalan for the u64->IEEE float conversion code.

Signed-off-by: Tim Schmielau <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2008-06-08 20:37:51

by Vegard Nossum

[permalink] [raw]
Subject: Re: [PATCH 6/6] kbuild: optimize headers_* targets

Hi Sam,

Just one more comment from me :-)

On Sun, Jun 8, 2008 at 10:07 PM, Sam Ravnborg <[email protected]> wrote:
> diff --git a/scripts/headers_install.pl b/scripts/headers_install.pl
> new file mode 100644
> index 0000000..283c055
> --- /dev/null
> +++ b/scripts/headers_install.pl
> @@ -0,0 +1,43 @@
> +#!/usr/bin/perl
> +#
> +# headers_install prepare the listed header files for use in
> +# user space and copy the files to their destination.
> +#
> +# Usage: headers_install.pl odir installdir [files...]
> +# odir: dir to open files
> +# install: dir to install the files
> +# files: list of files to check
> +#
> +# Step in preparation for users space:
> +# 1) Drop all use of compiler.h definitions
> +# 2) Drop include of compiler.h
> +# 3) Drop all sections defined out by __KERNEL__ (using unifdef)
> +
> +use strict;
> +use warnings;
> +
> +my ($odir, $installdir, @files) = @ARGV;
> +
> +my $ret = 0;
> +my $unifdef = "scripts/unifdef -U__KERNEL__";
> +
> +foreach my $file (@files) {
> + my $tmpfile = "$installdir/$file.tmp";
> + open(my $infile, '<', "$odir/$file") or die "$odir/$file: $!\n";
> + open(my $outfile, '>', "$tmpfile") or die "$tmpfile: $!\n";
> + while (my $line = <$infile>) {
> + $line =~ s/([\s(])__user\s/$1/g;
> + $line =~ s/([\s(])__force\s/$1/g;
> + $line =~ s/([\s(])__iomem\s/$1/g;
> + $line =~ s/\s__attribute_const__\s/ /g;
> + $line =~ s/\s__attribute_const__$//g;
> + $line =~ s/^#include <linux\/compiler.h>//;
> + printf $outfile "%s", $line;
> + }
> + close $outfile;
> + close $infile;
> + $ret = system $unifdef . " $tmpfile > $installdir/$file";

This seems flawed as we'll always exit with the $ret of the last file.
Do you intend to abort on the first error, or go as far as possible?

Maybe you can use something like this:

system ... or warn "$file: $!\n"
$ret = $? unless $ret;

(This should preserve the $ret from the first process that fails with
a non-zero exit code.)

> + unlink $tmpfile;
> +}
> +
> +exit $ret;

What do you think?


Vegard

--
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
-- E. W. Dijkstra, EWD1036

2008-06-08 20:59:15

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

It doesn't look like headers_check_all will work after your patches --
but to be fair I don't think it worked _before_ them either. When you
run headers_install_all, there's no asm/ directory -- and thus the check
will fail for any headers in linux/ which try to include <asm/*.h>.

Did this _ever_ work?

--
dwmw2

2008-06-08 21:12:58

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, Jun 08, 2008 at 09:58:54PM +0100, David Woodhouse wrote:
> It doesn't look like headers_check_all will work after your patches --
> but to be fair I don't think it worked _before_ them either. When you
> run headers_install_all, there's no asm/ directory -- and thus the check
> will fail for any headers in linux/ which try to include <asm/*.h>.
>
> Did this _ever_ work?
It relied on what was installed in asm previously resulting in
arbitary failures - which confused me a little.

What I did in headers_check.pl was to introduce a second try.
If we see a #include <asm/file.h> then we try both of:
asm/file.h (first)
asm-$arch/file.h (if the asm/file.h did not succeed)

So with my version it actually works as expected and we
hit a bug in cris.
I've also implmented the feature that we stop on error so
when cris has a bug we do not try the rest.

It is both good and bad.

Sam

2008-06-08 21:14:50

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 6/6] kbuild: optimize headers_* targets

On Sun, Jun 08, 2008 at 10:37:41PM +0200, Vegard Nossum wrote:
> Hi Sam,
>
> Just one more comment from me :-)
>
> On Sun, Jun 8, 2008 at 10:07 PM, Sam Ravnborg <[email protected]> wrote:
> > diff --git a/scripts/headers_install.pl b/scripts/headers_install.pl
> > new file mode 100644
> > index 0000000..283c055
> > --- /dev/null
> > +++ b/scripts/headers_install.pl
> > @@ -0,0 +1,43 @@
> > +#!/usr/bin/perl
> > +#
> > +# headers_install prepare the listed header files for use in
> > +# user space and copy the files to their destination.
> > +#
> > +# Usage: headers_install.pl odir installdir [files...]
> > +# odir: dir to open files
> > +# install: dir to install the files
> > +# files: list of files to check
> > +#
> > +# Step in preparation for users space:
> > +# 1) Drop all use of compiler.h definitions
> > +# 2) Drop include of compiler.h
> > +# 3) Drop all sections defined out by __KERNEL__ (using unifdef)
> > +
> > +use strict;
> > +use warnings;
> > +
> > +my ($odir, $installdir, @files) = @ARGV;
> > +
> > +my $ret = 0;
> > +my $unifdef = "scripts/unifdef -U__KERNEL__";
> > +
> > +foreach my $file (@files) {
> > + my $tmpfile = "$installdir/$file.tmp";
> > + open(my $infile, '<', "$odir/$file") or die "$odir/$file: $!\n";
> > + open(my $outfile, '>', "$tmpfile") or die "$tmpfile: $!\n";
> > + while (my $line = <$infile>) {
> > + $line =~ s/([\s(])__user\s/$1/g;
> > + $line =~ s/([\s(])__force\s/$1/g;
> > + $line =~ s/([\s(])__iomem\s/$1/g;
> > + $line =~ s/\s__attribute_const__\s/ /g;
> > + $line =~ s/\s__attribute_const__$//g;
> > + $line =~ s/^#include <linux\/compiler.h>//;
> > + printf $outfile "%s", $line;
> > + }
> > + close $outfile;
> > + close $infile;
> > + $ret = system $unifdef . " $tmpfile > $installdir/$file";
>
> This seems flawed as we'll always exit with the $ret of the last file.
> Do you intend to abort on the first error, or go as far as possible?
>
> Maybe you can use something like this:
>
> system ... or warn "$file: $!\n"
> $ret = $? unless $ret;
>
> (This should preserve the $ret from the first process that fails with
> a non-zero exit code.)
>
> > + unlink $tmpfile;
> > +}
> > +
> > +exit $ret;
>
> What do you think?
[Not much - late here....]

Thanks - I will try to fix it somehow.
But I hope that someone with more perl fu than me will redo
the unifdef stuff so I do not have to worry...

Sam

2008-06-09 06:23:07

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Sun, Jun 08, 2008 at 09:58:54PM +0100, David Woodhouse wrote:
> It doesn't look like headers_check_all will work after your patches --
> but to be fair I don't think it worked _before_ them either. When you
> run headers_install_all, there's no asm/ directory -- and thus the check
> will fail for any headers in linux/ which try to include <asm/*.h>.

Accidentially I left an older hack in my patch:
+headers_check_all: headers_install headers_install_all
^^^^^^^^^^^^^^^
+ $(Q)$(MAKE) $(hdr-inst)=include HDRCHECK=1


This made us install both to asm/ and asm-x86 (as this is an x86 box).

I will remove this hack before hitting kbuild-next.

Sam

2008-06-09 10:19:33

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH 5/6] kbuild: error out early in make headers_install

On Sun, 2008-06-08 at 22:07 +0200, Sam Ravnborg wrote:
> Fix the a.out.h case by setting SRCARCH and error
> out early in case of an error.
> The a.out.h case failed with the *_all targets.

I wanted to do the same trick ($(if $(wildcard asm-$(SRCARCH)/a.out.h)))
for exporting linux/a.out.h too -- that should never have been
unexported, and really should be put back before 2.6.26 release (I think
I sent a patch, already).

I suppose we'll have to also export it ifeq ($(SRCARCH),).

--
dwmw2

2008-06-26 02:14:40

by Tim Schmielau

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

[apologies for the late reply, I do not follow lkml anymore]

On Sun, 2008-06-08 at 14:17 +0100, David Woodhouse wrote:
> On Sun, 2008-06-08 at 13:17 +0200, Sam Ravnborg wrote:
> > #if !defined(CONFIG_M68K) || !defined(__KERNEL__)
>
> That's just scary, and broken for m68k where in userspace neither
> CONFIG_M68K nor __KERNEL__ will be defined, so the unwanted ac_ahz
> member will actually show up and break the binary compatibility.
> Assuming we _don't_ want the ac_ahz member to be included on m68k,
> this
> should fix it (is __mc68000__ the right thing to use?)...
>
> diff --git a/include/linux/acct.h b/include/linux/acct.h
> index e8cae54..228473b 100644
> --- a/include/linux/acct.h
> +++ b/include/linux/acct.h
> @@ -58,8 +58,7 @@ struct acct
> comp_t ac_minflt; /* Minor Pagefaults */
> comp_t ac_majflt; /* Major Pagefaults */
> comp_t ac_swaps; /* Number of Swaps */
> -/* m68k had no padding here. */
> -#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
> +#ifndef __mc68000__ /* m68k had no padding here. */
> __u16 ac_ahz; /* AHZ */
> #endif
> __u32 ac_exitcode; /* Exitcode */
>
This would make cross-compiled m68k kernels silently write wrong
accounting files.

I personally believe it more likely to find an m68k user cross-compiling
his kernels than one including kernel headers in userspace, which was
frowned upon at the time the above code was written (and the number
of BSD accounting users on m68k was estimated as nil, anyways).

If we want this header to work on m68k in userspace, something even
more scary like the following approximation to a mind-reading device
would be needed:

#if !defined(CONFIG_M68K) || (!defined(__KERNEL__) && !
defined(__mc68000__))

Tim

2008-06-26 07:14:00

by Adrian Bunk

[permalink] [raw]
Subject: Re: [PATCH] Speed up "make headers_*"

On Thu, Jun 26, 2008 at 02:59:21AM +0100, Tim Schmielau wrote:
> [apologies for the late reply, I do not follow lkml anymore]
>
> On Sun, 2008-06-08 at 14:17 +0100, David Woodhouse wrote:
>> On Sun, 2008-06-08 at 13:17 +0200, Sam Ravnborg wrote:
>> > #if !defined(CONFIG_M68K) || !defined(__KERNEL__)
>>
>> That's just scary, and broken for m68k where in userspace neither
>> CONFIG_M68K nor __KERNEL__ will be defined, so the unwanted ac_ahz
>> member will actually show up and break the binary compatibility.
>> Assuming we _don't_ want the ac_ahz member to be included on m68k,
>> this
>> should fix it (is __mc68000__ the right thing to use?)...
>>
>> diff --git a/include/linux/acct.h b/include/linux/acct.h
>> index e8cae54..228473b 100644
>> --- a/include/linux/acct.h
>> +++ b/include/linux/acct.h
>> @@ -58,8 +58,7 @@ struct acct
>> comp_t ac_minflt; /* Minor Pagefaults */
>> comp_t ac_majflt; /* Major Pagefaults */
>> comp_t ac_swaps; /* Number of Swaps */
>> -/* m68k had no padding here. */
>> -#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
>> +#ifndef __mc68000__ /* m68k had no padding here. */
>> __u16 ac_ahz; /* AHZ */
>> #endif
>> __u32 ac_exitcode; /* Exitcode */

struct acct as provided by the kernel anyway differs from the one
shipped by glibc in /usr/include/sys/acct.h since your
"BSD accounting format rework" patch back in 2004.

Does the pre-v3 stuff actually work?
My first impression is that it doesn't and should therefore be removed.

> This would make cross-compiled m68k kernels silently write wrong
> accounting files.
>...

Cross-compiling can't make any difference here.

> Tim

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