Regression in "kbuild: fix if_change and friends to consider argument order"

Em Ter, 2016-06-07 às 20:29 +0900, Masahiro Yamada escreveu:
> 2016-06-07 19:48 GMT+09:00 Michal Marek <mmarek@suse.cz>:
> > 
> > On 2016-06-07 12:03, Masahiro Yamada wrote:
> > > 
> > > 2016-06-07 18:58 GMT+09:00 Michal Marek <mmarek@suse.cz>:
> > > > 
> > > > On 2016-06-07 11:38, Michal Marek wrote:
> > > > > 
> > > > > On 2016-06-07 03:38, Zanoni, Paulo R wrote:
> > > > > > 
> > > > > > Hi
> > > > > > 
> > > > > > I recently noticed that alternating between "make" and
> > > > > > "make targz-pkg"
> > > > > > rebuilds the whole Kernel. This was not happening before.
> > > > > > As a Kernel
> > > > > > developer, my build/install/test environment heavily relies
> > > > > > on the fact
> > > > > > that "make targz-pkg" only quickly generates the tarball if
> > > > > > everything
> > > > > > is already built, so this change is heavily impacting my
> > > > > > development
> > > > > > environment.
> > > > > > 
> > > > > > I did some bisection and concluded that the first bad
> > > > > > commit is:
> > > > > > 
> > > > > > commit 9c8fa9bc08f60ac657751daba9fccf828a36cfed
> > > > > > Author: Masahiro Yamada
> > > > > > Date:   Sat May 7 15:48:26 2016 +0900
> > > > > >     kbuild: fix if_change and friends to consider argument
> > > > > > order
> > > > > > 
> > > > > > I also verified that if I just revert this commit on top of
> > > > > > the
> > > > > > most recent tree it goes back to the usual behavior.
> > > > > > 
> > > > > > I read the commit message and it seems that some unneeded
> > > > > > rebuilds are
> > > > > > somewhat expected, but I can't understand why such a change
> > > > > > in the
> > > > > > command line like the one I did triggers everything to be
> > > > > > rebuilt.
> > > > > > IMHO, it really shouldn't. I also wonder that maybe the
> > > > > > regression I'm
> > > > > > experiencing was not expected in the original change, so
> > > > > > maybe there's
> > > > > > a way to keep the original improvement caused by the
> > > > > > mentioned patch
> > > > > > without the regression I'm experiencing.
> > > > > > 
> > > > > > How to reproduce (exact commands I used at every bisect
> > > > > > step):
> > > > > > 
> > > > > > $ make tinyconfig
> > > > > > $ time make -j4 V=2 # this should build things
> > > > > > $ time make -j4 V=2 # just to make sure nothing will be
> > > > > > rebuilt
> > > > > > $ time make -j4 V=2 targz-pkg
> > > > > I can reproduce it.
> > > > Try the attached patch.
> > > > 
> > > > Michal
> > > > 
> > > 
> > > Right.
> > > 
> > > I had already sent a similar patch.
> > > https://patchwork.kernel.org/patch/9159863/
> > I see. I hadn't read all my mail before replying.
> > 
> > 
> > > 
> > > My concern is it is effectively
> > > reverting e8f5bdb02ce0.
> > > I hope Rik can comment on that.
> > My patch just resets NOSTDINC_FLAGS before including the arch
> > Makefile.
> > 
> Ah, I missed that, sorry.
> 
> Then, yours should be no problem.

I tested both patches you provided:
 - kbuild: do not append NOSTDINC_FLAGS to avoid rebuild in package
targets
 - kbuild: Initialize NOSTDINC_CFLAGS

Both seem to improve the situation to a point where, at least for a
tinyconfig, timings are acceptable. But it's important to notice that
none of the changes are equivalent to just reverting the first bad
commit. We still recompile some additional files that were not compiled
with the full revert. Let me show you:

$ # starting from a clean tree
$ git revert 9c8fa9bc08f60ac657751daba9fccf828a36cfed
$ time make -j4 V=2
$ time make -j4 V=2 targz-pkg 2>&1 > /tmp/revert-log.patch
$ git reset --hard HEAD~1 # go back to a clean tree, no revert
$ git am patches/0001-kbuild-Initialize-NOSTDINC_CFLAGS.patch
$ time make -j4 V=2
$ time make -j4 V=2 targz-pkg 2>&1 > /tmp/marek-fix-log.patch
$ diff -Nrup /tmp/revert-log.patch /tmp/marek-fix-log.patch 

I also wondered that maybe we need to also initialize
KBUILD_LDFLAGS_MODULE, but it seems what fixed the problem was just
LDFLAGS_vmlinux. So I'm not sure if we'll need this. I also have no
idea whether this would cause other unintended regressions. It's up to
you, Makefile maintainers, to judge. 

Thanks for the quick responses!

> 
> 
>

Regression in "kbuild: fix if_change and friends to consider argument order"

Commit Message

Patch