lkml.org 
[lkml]   [2013]   [Oct]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] perf auto-dep: Speed up feature tests by building them in parallel

* Namhyung Kim <namhyung@kernel.org> wrote:

> Hi Ingo,
>
> On Mon, 30 Sep 2013 18:42:10 +0200, Ingo Molnar wrote:
> > This series (with combo patch attached) implements (much) faster
> > perf-tools feature-auto-detection.
> >
> > I used 3 tricks to implement feature auto-dependencies and to speed up
> > feature detection:
> >
> > - standalone Makefile in config/feature-checks/ built in parallel
> >
> > - split-out standalone .c files in config/feature-checks/*.c
> >
> > - used GCC's auto-dependency generation feature (-MD) to track the
> > effects of system library addition/removal.
>
> I have a memory that this could lead to a nasty build failure. Please
> see the commit b6f4f804108b ("tools lib traceevent: Do not generate
> dependency for system header files").

I think that at least the 'make clean' failure was just a buggy Makefile.
To quote the build error from the commit:

comet:~/tip/tools/lib/traceevent> make clean
make: *** No rule to make target `/usr/lib/gcc/x86_64-redhat-linux/4.7.0/include/stddef.h', needed by `.trace-seq

It suggests that the 'clean' target depended on .d dependency files -
that's a fundamentally incorrect use of -M/-MD auto-dependencies.

> The problem is that it turned out to depend on some compiler headers
> which are located under some directory with a version number. If so,
> when compiler upgraded to a new version, it cannot find the original
> dependencies so fail to build.
>
> $ cat config/feature-checks/test-libelf.d
> test-libelf: test-libelf.c /usr/include/libelf.h /usr/include/sys/types.h \
> /usr/include/features.h /usr/include/stdc-predef.h \
> /usr/include/sys/cdefs.h /usr/include/bits/wordsize.h \
> /usr/include/gnu/stubs.h /usr/include/gnu/stubs-64.h \
> /usr/include/bits/types.h /usr/include/bits/typesizes.h \
> /usr/include/time.h \
> /usr/lib/gcc/x86_64-redhat-linux/4.7.2/include/stddef.h \
> /usr/include/endian.h /usr/include/bits/endian.h \
> /usr/include/bits/byteswap.h /usr/include/bits/byteswap-16.h \
> /usr/include/sys/select.h /usr/include/bits/select.h \
> /usr/include/bits/sigset.h /usr/include/bits/time.h \
> /usr/include/sys/sysmacros.h /usr/include/bits/pthreadtypes.h \
> /usr/include/elf.h \
> /usr/lib/gcc/x86_64-redhat-linux/4.7.2/include/stdint.h \
> /usr/include/stdint.h /usr/include/bits/wchar.h
>
> In this case we are using this for feature-checking, so I guess it'd
> fail to check the feature after upgrade.

The dependencies are re-made by GCC if a target fails and is rebuilt - and
that should include the new header locations.

I checked out the parent commit (8f7c1d07ade5) which still had full -M,
and this is how it utilized dependencies:

# let .d file also depends on the source and header files
define check_deps
@set -e; $(RM) $@; \
$(CC) -M $(CFLAGS) $< > $@.$$$$; \
sed 's,\($*\)\.o[ :]*,\1.o $@ : ,g' < $@.$$$$ > $@; \
$(RM) $@.$$$$
endef

that's not a very robust method either: .d files should be generated via
-MD not via -M and should be included directly into the Makefile, like I
did it in my patch:

-include *.d */*.d

and the .d files themselves are never added as dependencies - they are
re-made by compilation automatically, not by any explicit Makefile rule.
Adding them as dependencies risks circular dependencies, because the only
method to rebuild a .d file is to actually meet the dependencies of a .c
target.

So if done properly I don't think the build failure cited in that
changelog can trigger.

Now, I cannot vouch for -MD blindly, without having seen a lot more
testing, so we might still be forced to disable or limit that auto-dep
trick, but the reasons cited in b6f4f804108b don't seem to be a GCC bug
but a Makefile bug - they just weren't fully understood back then.

Thanks,

Ingo


\
 
 \ /
  Last update: 2013-10-02 08:41    [W:0.217 / U:8.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site