diff mbox

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

Message ID 20160607215237.GA13237@sepie.suse.cz (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Marek June 7, 2016, 9:52 p.m. UTC 
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(+)

Comments

Zanoni, Paulo R June 8, 2016, 11:29 p.m. UTC | #1 
RW0gVGVyLCAyMDE2LTA2LTA3IMOgcyAyMzo1MiArMDIwMCwgTWljaGFsIE1hcmVrIGVzY3JldmV1
Og0KPiBPbiBUdWUsIEp1biAwNywgMjAxNiBhdCAwMjoxMDoyOFBNICswMDAwLCBaYW5vbmksIFBh
dWxvIFIgd3JvdGU6DQo+ID7CoA0KPiANCj4gRnJvbSBiMzZmYWQ2NWQ2MWZmZmU0YjY2MmQ0YmZi
MWVkNjczYzQ1NWEzNmEyIE1vbiBTZXAgMTcgMDA6MDA6MDANCj4gMjAwMQ0KPiBGcm9tOiBNaWNo
YWwgTWFyZWsgPG1tYXJla0BzdXNlLmNvbT4NCj4gRGF0ZTogVHVlLCA3IEp1biAyMDE2IDExOjU3
OjAyICswMjAwDQo+IFN1YmplY3Q6IFtQQVRDSF0ga2J1aWxkOiBJbml0aWFsaXplIGV4cG9ydGVk
IHZhcmlhYmxlcw0KPiANCj4gVGhlIE5PU1RESU5DX0ZMQUdTIHZhcmlhYmxlIGlzIGV4cG9ydGVk
LCBzbyBpdCBuZWVkcyB0byBiZSBjbGVhcmVkIHRvDQo+IGF2b2lkIGR1cGxpY2F0aW5nIGl0cyBj
b250ZW50IHdoZW4gcnVubmluZyBtYWtlIGZyb20gd2l0aGluIG1ha2UNCj4gKGUuZy4NCj4gaW4g
dGhlIHBhY2thZ2luZyB0YXJnZXRzKS4gVGhpcyBiZWNhbWUgYW4gaXNzdWUgYWZ0ZXIgY29tbWl0
DQo+IDljOGZhOWJjMDhmNiAoImtidWlsZDogZml4IGlmX2NoYW5nZSBhbmQgZnJpZW5kcyB0byBj
b25zaWRlciBhcmd1bWVudA0KPiBvcmRlciIpLCB3aGljaCBubyBsb25nZXIgaWdub3JlcyB0aGUg
ZHVwbGljYXRlIG9wdGlvbnMuIEFzIFBhdWxvDQo+IFphbm9uaQ0KPiBwb2ludHMgb3V0LCB0aGUg
TERGTEFHU192bWxpbnV4IHZhcmlhYmxlIGhhcyB0aGUgc2FtZSBwcm9ibGVtLg0KPiANCj4gUmVw
b3J0ZWQtYnk6ICJaYW5vbmksIFBhdWxvIFIiIDxwYXVsby5yLnphbm9uaUBpbnRlbC5jb20+DQo+
IEZpeGVzOiA5YzhmYTliYzA4ZjYgKCJrYnVpbGQ6IGZpeCBpZl9jaGFuZ2UgYW5kIGZyaWVuZHMg
dG8gY29uc2lkZXINCj4gYXJndW1lbnQgb3JkZXIiKQ0KPiBTaWduZWQtb2ZmLWJ5OiBNaWNoYWwg
TWFyZWsgPG1tYXJla0BzdXNlLmNvbT4NCg0KV29ya3MgZm9yIG1lLg0KDQpUZXN0ZWQtYnk6IFBh
dWxvIFphbm9uaSA8cGF1bG8uci56YW5vbmlAaW50ZWwuY29tPg0KDQpUaGFua3MgYSBsb3QhDQoN
Cj4gLS0tDQo+IMKgTWFrZWZpbGUgfCAyICsrDQo+IMKgMSBmaWxlIGNoYW5nZWQsIDIgaW5zZXJ0
aW9ucygrKQ0KPiANCj4gZGlmZiAtLWdpdCBhL01ha2VmaWxlIGIvTWFrZWZpbGUNCj4gaW5kZXgg
MGY3MGRlNjNjZmRiLi5hZjBjNDYzZTkwOGYgMTAwNjQ0DQo+IC0tLSBhL01ha2VmaWxlDQo+ICsr
KyBiL01ha2VmaWxlDQo+IEBAIC0zNjMsMTEgKzM2MywxMyBAQCBDSEVDSwkJPSBzcGFyc2UNCj4g
wqANCj4gwqBDSEVDS0ZMQUdTwqDCoMKgwqDCoDo9IC1EX19saW51eF9fIC1EbGludXggLURfX1NU
RENfXyAtRHVuaXggLURfX3VuaXhfXyBcDQo+IMKgCQnCoMKgLVdiaXR3aXNlIC1Xbm8tcmV0dXJu
LXZvaWQgJChDRikNCj4gK05PU1RESU5DX0ZMQUdTwqDCoD0NCj4gwqBDRkxBR1NfTU9EVUxFwqDC
oMKgPQ0KPiDCoEFGTEFHU19NT0RVTEXCoMKgwqA9DQo+IMKgTERGTEFHU19NT0RVTEXCoMKgPQ0K
PiDCoENGTEFHU19LRVJORUwJPQ0KPiDCoEFGTEFHU19LRVJORUwJPQ0KPiArTERGTEFHU192bWxp
bnV4ID0NCj4gwqBDRkxBR1NfR0NPVgk9IC1mcHJvZmlsZS1hcmNzIC1mdGVzdC1jb3ZlcmFnZSAt
Zm5vLXRyZWUtbG9vcC0NCj4gaW0gLVduby1tYXliZS11bmluaXRpYWxpemVkDQo+IMKgQ0ZMQUdT
X0tDT1YJPSAtZnNhbml0aXplLWNvdmVyYWdlPXRyYWNlLXBjDQo+IMKg
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thorsten Leemhuis June 26, 2016, 10:43 a.m. UTC | #2 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Marek June 27, 2016, 8:10 p.m. UTC | #3 
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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