linux-kernel - Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 04 Apr 2013 11:36:14 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Vineet Gupta <Vineet.Gupta1@...opsys.com>
CC:	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
	linux-kernel@...r.kernel.org, Michal Marek <mmarek@...e.cz>,
	Stephen Warren <swarren@...dia.com>,
	Grant Likely <grant.likely@...retlab.ca>,
	linux-kbuild@...r.kernel.org
Subject: Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

On 04/03/2013 11:30 PM, Vineet Gupta wrote:
> On 04/03/2013 09:48 PM, Stephen Warren wrote:
>> On 04/03/2013 01:14 AM, Vineet Gupta wrote:
>>> forgot to CC linux-arch
>>>
>>> On 04/03/2013 12:42 PM, Vineet Gupta wrote:
>>>> Currently, for every ARC kernel build I see the following:
>>>>
>>>> --------------->8-----------------
>>>>   DTB    arch/arc/boot/dts/angel4.dtb.S
>>>>   AS      arch/arc/boot/dts/angel4.dtb.o
>>>>   LD      arch/arc/boot/dts/built-in.o
>>>> rm arch/arc/boot/dts/angel4.dtb.S        <-- forces rebuild next iter
>>>>   CHK     kernel/config_data.h
>>>> --------------->8-----------------
>> I assume that's because the file is an intermediate file, and only built
>> due to a chain of build rules, and hence make clean it up itself after
>> the build?
> 
> Indeed - I should have made that explicit in the Changelog.
> 
>>>> +.PRECIOUS: $(obj)/%.dtb.S
>>>> +
>>>>  $(obj)/%.dtb.S: $(obj)/%.dtb
>>>>  	$(call cmd,dt_S_dtb)
>> I'm not sure if .PRECIOUS is correct here. That prevents make from
>> deleting the file if make is CTRL-C'd in the middle of generating it.
>> Couldn't that leave a stale/corrupt file around that'd break the build.
>> Judging by:
>>
>> http://www.gnu.org/software/make/manual/html_node/Special-Targets.html
>>
>> I think .SECONDARY might be a better choice? Does that solve the problem
>> you're seeing?
> 
> Technically .SECONDARY is better - however it doesn't seem to work.

Hmmm. It does for me.

$ make --version
GNU Make 3.81

I hacked the ARM makefiles as follows:

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 4737408..70247c6 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -256,6 +256,7 @@ core-y                              +=
arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
 core-y                         += arch/arm/net/
 core-y                         += arch/arm/crypto/
 core-y                         += $(machdirs) $(platdirs)
+core-y                         += arch/arm/boot/dts/

 drivers-$(CONFIG_OPROFILE)      += arch/arm/oprofile/

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index dedca49..af2202e 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -197,3 +197,5 @@ dtbs: $(addprefix $(obj)/, $(dtb-y))
        $(Q)rm -f $(obj)/../*.dtb

 clean-files := *.dtb
+
+obj-y += tegra20-test.dtb.o

and manually created a tegra20-test.dts. For reasons I didn't bother
investigating, the .dtb.S -> .dtb.o conversion failed with syntax
errors, and because the .dtb.S file was an intermediate file, make rm's
it in this case. That's exactly the issue you're seeing.

I then added the following to Makefile.lib:

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index a0ab6d7..fc11a67 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -258,6 +258,8 @@ cmd_dt_S_dtb=
        \
        echo '.balign STRUCT_ALIGNMENT';                \
 ) > $@

+.SECONDARY: $(obj)/%.dtb.S
+
 $(obj)/%.dtb.S: $(obj)/%.dtb
        $(call cmd,dt_S_dtb)

and no longer see make rm'ing the .dtb.S file. So, the .SECONDARY is
behaving as expected, and should fix your problem.

One other problem this highlighted: Unless you also mark the generated
.dtb file as PRECIOUS/SECONDARY (or unless that DTB is included in
targets for some reason) then make also rm's that, since it considers it
intermediate:

  DTC     arch/arm/boot/dts/tegra20-test.dtb
  DTB    arch/arm/boot/dts/tegra20-test.dtb.S
  AS      arch/arm/boot/dts/tegra20-test.dtb.o
[error]
rm arch/arm/boot/dts/tegra20-test.dtb

So, your patch would probably need modification to apply the fix to
*.dtb too, since they might also be intermediate?

Perhaps add something like the following to arch/arc/boot/dts/Makefile?

.SECONDARY: $(obj)/$(builtindtb-y).dtb

> Running make with various debug toggles doesn't seem to be helping with why
> .PRECIOUS works but not this.
> That is also likely reason for a bunch of other .PRECIOUS entries in the same file
> but no .SECONDARY.

The existing .PRECIOUS exist to support lexer/yacc files, for which both
the source .l/.y files are checked in, and the generated
%.lex.c_shipped/%.tab.[cl]_shipped are also checked in. In this case,
the "_shipped" files must be marked PRECIOUS, since they're under source
control and hence should never be deleted. I don't believ this same
situation applies to the .dtb.S files you're having problems with, so I
don't think .PRECIOUS is correct for them.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/