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

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

[Zanoni, Paulo R]

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

In bad commits, the last command will trigger a rebuild of the whole
Kernel, mentioning "due to command line change" as a reason to rebuild
every object file. In good commits, the last command will finish after
a few seconds, not rebuilding anything, just giving us a nice tarball.

Also notice that if you do wait for the whole targz-pkg build, and
later go back to "make -j4 V=2" (without targz-pkg), it will, again,
rebuild everything, which is also not the old behavior.

After some investigation, it looks like the .obj.cmd files are what's
compared here, so let me provide you an example that will hopefully
help you debug:

$ time make -j4 V=2
$ cp init/.main.o.cmd /tmp/main-before.cmd
$ time make -j4 V=2 targz-pkg
$ diff -Nrup /tmp/main-before.cmd init/.main.o.cmd > /tmp/cmd-
diff.patch

You can access the diff file here:
  https://people.freedesktop.org/~pzanoni/cmd-diff.patch

If you take a look, you'll see that the only difference is that the cmd
file generated during targz-pkg contains these extra chars in the
cmd_init/main.o declaration:
  -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/5/include

But please notice that these arguments are repeated! The main-
before.cmd file contains those arguments only once, while the new file
contains them twice. I don't know why "make targz-pkg" makes these
arguments appear twice. Maybe fixing this would be a better way to
solve the regression instead of just reverting the "first bad commit"?

I didn't debug the problem further. I'm hoping the info I provided is
enough for you to keep things moving, but if you have any patches or
tips on where I could continue digging, please tell me. I suppose the
problem should be easily reproducible everywhere.

In the meantime, since I can't pay the price of 2 full rebuilds for
every tested Kernel, I suppose I'll be carrying a local revert, but I'd
be really happy to see an upstream solution.

Thanks,
Paulo

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

[Michal Marek]

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.

Michal

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

[Michal Marek]

[-- Attachment #1: Type: text/plain, Size: 1651 bytes --]

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


[-- Attachment #2: 0001-kbuild-Initialize-NOSTDINC_CFLAGS.patch --]
[-- Type: text/x-patch, Size: 788 bytes --]

>From 86536cfcd776b5d9e238a4690756028799122a86 Mon Sep 17 00:00:00 2001
From: Michal Marek <mmarek@suse.com>
Date: Tue, 7 Jun 2016 11:57:02 +0200
Subject: [PATCH] kbuild: Initialize NOSTDINC_CFLAGS

The variable is exported, so it needs to be cleared to avoid duplicating
its content when running make from within make (e.g. in the packaging
targets).

Signed-off-by: Michal Marek <mmarek@suse.com>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 0f9cb36d45c2..1a85efa216ac 100644
--- a/Makefile
+++ b/Makefile
@@ -359,6 +359,7 @@ CHECK		= sparse
 
 CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
 		  -Wbitwise -Wno-return-void $(CF)
+NOSTDINC_FLAGS  =
 CFLAGS_MODULE   =
 AFLAGS_MODULE   =
 LDFLAGS_MODULE  =
-- 
2.6.2

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

[Masahiro Yamada]

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/

My concern is it is effectively
reverting e8f5bdb02ce0.
I hope Rik can comment on that.




-- 
Best Regards
Masahiro Yamada

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

[Michal Marek]

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.

Michal

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

[Masahiro Yamada]

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.



-- 
Best Regards
Masahiro Yamada

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

[Zanoni, Paulo R]

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 
--- /tmp/revert-log.patch	2016-06-07 10:30:47.118129155 -0300
+++ /tmp/marek-fix-log.patch	2016-06-07 10:33:00.271359159 -0300
@@ -8,7 +8,35 @@ make KBUILD_SRC=
   CHK     include/generated/asm-offsets.h
   CALL    scripts/checksyscalls.sh - due to target missing
   CHK     include/generated/compile.h
-Kernel: arch/x86/boot/bzImage is ready  (#39)
+  LINK    vmlinux - due to command line change
+  LD      vmlinux.o
+  MODPOST vmlinux.o - due to vmlinux.o not in $(targets)
+  GEN     .version
+  CHK     include/generated/compile.h
+  UPD     include/generated/compile.h
+  CC      init/version.o - due to: include/generated/compile.h
+  LD      init/built-in.o - due to: init/version.o
+  LD      vmlinux
+  SORTEX  vmlinux
+  SYSMAP  System.map
+  CC      arch/x86/boot/version.o - due to:
include/generated/compile.h
+  VOFFSET arch/x86/boot/compressed/../voffset.h - due to: vmlinux
+  OBJCOPY arch/x86/boot/compressed/vmlinux.bin - due to: vmlinux
+  XZKERN  arch/x86/boot/compressed/vmlinux.bin.xz - due to:
arch/x86/boot/compressed/vmlinux.bin
+  CC      arch/x86/boot/compressed/misc.o - due to:
arch/x86/boot/compressed/../voffset.h
+  MKPIGGY arch/x86/boot/compressed/piggy.S - due to:
arch/x86/boot/compressed/vmlinux.bin.xz
+  AS      arch/x86/boot/compressed/piggy.o - due to:
arch/x86/boot/compressed/piggy.S
+  LD      arch/x86/boot/compressed/vmlinux - due to:
arch/x86/boot/compressed/misc.o arch/x86/boot/compressed/piggy.o
+  ZOFFSET arch/x86/boot/zoffset.h - due to:
arch/x86/boot/compressed/vmlinux
+  OBJCOPY arch/x86/boot/vmlinux.bin - due to:
arch/x86/boot/compressed/vmlinux
+  AS      arch/x86/boot/header.o - due to: arch/x86/boot/zoffset.h
+  LD      arch/x86/boot/setup.elf - due to: arch/x86/boot/header.o
arch/x86/boot/version.o
+  OBJCOPY arch/x86/boot/setup.bin - due to: arch/x86/boot/setup.elf
+  BUILD   arch/x86/boot/bzImage - due to: arch/x86/boot/setup.bin
arch/x86/boot/vmlinux.bin
+Setup is 15484 bytes (padded to 15872 bytes).
+System is 361 kB
+CRC 85405354
+Kernel: arch/x86/boot/bzImage is ready  (#40)
 /bin/bash ./scripts/package/buildtar targz-pkg
 './System.map' -> './tar-install/boot/System.map-4.7.0-rc2+'
 './.config' -> './tar-install/boot/config-4.7.0-rc2+'


Now, let's try to debug the vmlinux rebuild:

$ time make -j4 V=2
$ cp .vmlinux.cmd /tmp/vmlinux-before.cmd
$ time make -j4 V=2 targz-pkg
$ diff -Nrup /tmp/vmlinux-before.cmd .vmlinux.cmd

--- /tmp/vmlinux-before.cmd	2016-06-07 10:39:08.791687067 -0300
+++ .vmlinux.cmd	2016-06-07 10:39:52.606774770 -0300
@@ -1 +1 @@
-cmd_vmlinux := /bin/bash scripts/link-vmlinux.sh ld -m elf_i386 --
build-id
+cmd_vmlinux := /bin/bash scripts/link-vmlinux.sh ld -m elf_i386 --
build-id --build-id

(my email client broke the long lines on the patch file, but you can
clearly see that targz-pkg added an extra --build-id to the cmd_vmlinux
declaration)

So after some grepping, I tried to also initialize LDFLAGS_vmlinux, in
the same way you did with NOSTDINC_FLAGS, and it fixed the problem for
me:

$ # This is all on top of Marek's patch
$ git diff
diff --git a/Makefile b/Makefile
index 5b3f0e7..9537635 100644
--- a/Makefile
+++ b/Makefile
@@ -364,6 +364,7 @@ CHECK		= sparse
 CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
 		  -Wbitwise -Wno-return-void $(CF)
 NOSTDINC_FLAGS  =
+LDFLAGS_vmlinux =
 CFLAGS_MODULE   =
 AFLAGS_MODULE   =
 LDFLAGS_MODULE  =
$ time make -j4 V=2
$ time make -j4 V=2 2>&1 > /tmp/full-fix.patch
$ diff -Nrup /tmp/revert-log.patch /tmp/full-fix.patch
--- /tmp/revert-log.patch	2016-06-07 10:30:47.118129155 -0300
+++ /tmp/full-fix.patch	2016-06-07 10:50:44.851252745 -0300
@@ -8,7 +8,7 @@ make KBUILD_SRC=
   CHK     include/generated/asm-offsets.h
   CALL    scripts/checksyscalls.sh - due to target missing
   CHK     include/generated/compile.h
-Kernel: arch/x86/boot/bzImage is ready  (#39)
+Kernel: arch/x86/boot/bzImage is ready  (#45)
 /bin/bash ./scripts/package/buildtar targz-pkg
 './System.map' -> './tar-install/boot/System.map-4.7.0-rc2+'
 './.config' -> './tar-install/boot/config-4.7.0-rc2+'

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!

> 
> 
> 

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

[Michal Marek]

On Tue, Jun 07, 2016 at 02:10:28PM +0000, Zanoni, Paulo R wrote:
> 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:

[...]

> So after some grepping, I tried to also initialize LDFLAGS_vmlinux, in
> the same way you did with NOSTDINC_FLAGS, and it fixed the problem for
> me:

Good catch! I my tests, I was interrupting the build early and only
checking the content of kernel/.bounds.s.cmd; lazy me :). I'm going to
apply the patch below to kbuild.git#rc-fixes.


> 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. 

I did check the += assignments now and I only see

$ awk '/\+=/ { print $1 }' Makefile  | sort -u | while read v; do grep
-m1 "$v" Makefile; done | grep '+='
CLEAN_DIRS  += $(MODVERDIR)
MAKEFLAGS += -rR --include-dir=$(CURDIR)
MRPROPER_DIRS  += include/config usr/include include/generated
\
MRPROPER_FILES += .config .config.old .version .old_version \

The MAKEFLAGS assignment is correct, the CLEAN_DIRS and MRPROPER_*
+= assignments are unnecessary, but none of these variables is exported.
So we are fine _with respect to the main Makefile_. It's possible that
e.g. some arch Makefile has a skeleton in the cupboard. We will see.

Michal


>From b36fad65d61fffe4b662d4bfb1ed673c455a36a2 Mon Sep 17 00:00:00 2001
From: Michal Marek <mmarek@suse.com>
Date: Tue, 7 Jun 2016 11:57:02 +0200
Subject: [PATCH] kbuild: Initialize exported variables

The NOSTDINC_FLAGS variable is exported, so it needs to be cleared to
avoid duplicating its content when running make from within make (e.g.
in the packaging targets). This became an issue after commit
9c8fa9bc08f6 ("kbuild: fix if_change and friends to consider argument
order"), which no longer ignores the duplicate options. As Paulo Zanoni
points out, the LDFLAGS_vmlinux variable has the same problem.

Reported-by: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
Fixes: 9c8fa9bc08f6 ("kbuild: fix if_change and friends to consider argument order")
Signed-off-by: Michal Marek <mmarek@suse.com>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index 0f70de63cfdb..af0c463e908f 100644
--- a/Makefile
+++ b/Makefile
@@ -363,11 +363,13 @@ CHECK		= sparse
 
 CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
 		  -Wbitwise -Wno-return-void $(CF)
+NOSTDINC_FLAGS  =
 CFLAGS_MODULE   =
 AFLAGS_MODULE   =
 LDFLAGS_MODULE  =
 CFLAGS_KERNEL	=
 AFLAGS_KERNEL	=
+LDFLAGS_vmlinux =
 CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im -Wno-maybe-uninitialized
 CFLAGS_KCOV	= -fsanitize-coverage=trace-pc
 
-- 
2.6.2

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

[Zanoni, Paulo R]

Em Ter, 2016-06-07 às 23:52 +0200, Michal Marek escreveu:
> On Tue, Jun 07, 2016 at 02:10:28PM +0000, Zanoni, Paulo R wrote:
> > 
> 
> From b36fad65d61fffe4b662d4bfb1ed673c455a36a2 Mon Sep 17 00:00:00
> 2001
> From: Michal Marek <mmarek@suse.com>
> Date: Tue, 7 Jun 2016 11:57:02 +0200
> Subject: [PATCH] kbuild: Initialize exported variables
> 
> The NOSTDINC_FLAGS variable is exported, so it needs to be cleared to
> avoid duplicating its content when running make from within make
> (e.g.
> in the packaging targets). This became an issue after commit
> 9c8fa9bc08f6 ("kbuild: fix if_change and friends to consider argument
> order"), which no longer ignores the duplicate options. As Paulo
> Zanoni
> points out, the LDFLAGS_vmlinux variable has the same problem.
> 
> Reported-by: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
> Fixes: 9c8fa9bc08f6 ("kbuild: fix if_change and friends to consider
> argument order")
> Signed-off-by: Michal Marek <mmarek@suse.com>

Works for me.

Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Thanks a lot!

> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 0f70de63cfdb..af0c463e908f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -363,11 +363,13 @@ CHECK		= sparse
>  
>  CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
>  		  -Wbitwise -Wno-return-void $(CF)
> +NOSTDINC_FLAGS  =
>  CFLAGS_MODULE   =
>  AFLAGS_MODULE   =
>  LDFLAGS_MODULE  =
>  CFLAGS_KERNEL	=
>  AFLAGS_KERNEL	=
> +LDFLAGS_vmlinux =
>  CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-
> im -Wno-maybe-uninitialized
>  CFLAGS_KCOV	= -fsanitize-coverage=trace-pc
>  

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

[Thorsten Leemhuis]

On 09.06.2016 01:29, Zanoni, Paulo R wrote:
> Em Ter, 2016-06-07 às 23:52 +0200, Michal Marek escreveu:
>> On Tue, Jun 07, 2016 at 02:10:28PM +0000, Zanoni, Paulo R wrote:
>> From b36fad65d61fffe4b662d4bfb1ed673c455a36a2 Mon Sep 17 00:00:00
>> 2001
>> From: Michal Marek <mmarek@suse.com>
>> Date: Tue, 7 Jun 2016 11:57:02 +0200
>> Subject: [PATCH] kbuild: Initialize exported variables
>>
>> The NOSTDINC_FLAGS variable is exported, so it needs to be cleared to
>> avoid duplicating its content when running make from within make
>> (e.g.
>> in the packaging targets). This became an issue after commit
>> 9c8fa9bc08f6 ("kbuild: fix if_change and friends to consider argument
>> order"), which no longer ignores the duplicate options. As Paulo
>> Zanoni
>> points out, the LDFLAGS_vmlinux variable has the same problem.
>>
>> Reported-by: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
>> Fixes: 9c8fa9bc08f6 ("kbuild: fix if_change and friends to consider
>> argument order")
>> Signed-off-by: Michal Marek <mmarek@suse.com>
> Works for me.
> Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Michal, what's the status here? This made it on my 4.7 regressions
report due to the "regression" keyword in the subject. Is this an older
regression or something that will get fixed post 4.7? It's not fixed in
mainline afaics, but maybe I missed something.

Sincerely, your regression tracker for Linux 4.7 (http://bit.ly/28JRmJo)
 Thorsten

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

[Michal Marek]

Dne 26.6.2016 v 12:43 Thorsten Leemhuis napsal(a):
> On 09.06.2016 01:29, Zanoni, Paulo R wrote:
>> Em Ter, 2016-06-07 às 23:52 +0200, Michal Marek escreveu:
>>> On Tue, Jun 07, 2016 at 02:10:28PM +0000, Zanoni, Paulo R wrote:
>>> From b36fad65d61fffe4b662d4bfb1ed673c455a36a2 Mon Sep 17 00:00:00
>>> 2001
>>> From: Michal Marek <mmarek@suse.com>
>>> Date: Tue, 7 Jun 2016 11:57:02 +0200
>>> Subject: [PATCH] kbuild: Initialize exported variables
>>>
>>> The NOSTDINC_FLAGS variable is exported, so it needs to be cleared to
>>> avoid duplicating its content when running make from within make
>>> (e.g.
>>> in the packaging targets). This became an issue after commit
>>> 9c8fa9bc08f6 ("kbuild: fix if_change and friends to consider argument
>>> order"), which no longer ignores the duplicate options. As Paulo
>>> Zanoni
>>> points out, the LDFLAGS_vmlinux variable has the same problem.
>>>
>>> Reported-by: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
>>> Fixes: 9c8fa9bc08f6 ("kbuild: fix if_change and friends to consider
>>> argument order")
>>> Signed-off-by: Michal Marek <mmarek@suse.com>
>> Works for me.
>> Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Michal, what's the status here? This made it on my 4.7 regressions
> report due to the "regression" keyword in the subject.

I forgot to send it to Linus, fixed now. Thanks for the reminder.

Michal