From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751190AbbCMFGe (ORCPT ); Fri, 13 Mar 2015 01:06:34 -0400 Received: from condef005-v.nifty.com ([210.131.4.242]:60973 "EHLO condef005-v.nifty.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750853AbbCMFGc (ORCPT ); Fri, 13 Mar 2015 01:06:32 -0400 X-Greylist: delayed 348 seconds by postgrey-1.27 at vger.kernel.org; Fri, 13 Mar 2015 01:06:31 EDT X-Nifty-SrcIP: [209.85.217.181] MIME-Version: 1.0 In-Reply-To: <20150311100101.GA30787@sepie.suse.cz> References: <15447.1425431256@turing-police.cc.vt.edu> <20150311100101.GA30787@sepie.suse.cz> Date: Fri, 13 Mar 2015 13:59:48 +0900 Message-ID: Subject: Re: [RFC PATCH] Don't reset timestamps in include/generated if not needed From: Masahiro Yamada To: Michal Marek Cc: Linus Torvalds , Valdis Kletnieks , Andrew Morton , Linux Kernel Mailing List , Linux Kbuild mailing list Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi. 2015-03-11 19:01 GMT+09:00 Michal Marek : > On Sun, Mar 08, 2015 at 04:08:06PM -0700, Linus Torvalds wrote: >> On Tue, Mar 3, 2015 at 5:07 PM, Valdis Kletnieks >> wrote: >> > >> > Kbuild regenerates bounds.h and asm-offsets.h, resetting the timestamps >> > and forcing rebuilds even if the contents haven't changed. Add a bit of >> > shell magic to only replace the file if the contents have in fact changed, >> > which should speed up git bisects and similar. >> > ... >> > RFC because I can't wrap my head around why this wasn't done ages ago. >> > If I'm missing something something obvious, please apply a cluestick. :) > > We do not regenerate the files *always*, but only if bounds.c / > asm-offsets.c or one of the headers they include change. So the rule is > not any worse than the rule for compiling a regular .c file into an .o > file. You can use ccache to avoid repeated rebuilds. Right. > However, I can see that especially bounds.h ends up being included in > many places, so cutting its dependencies helps in some cases. Agreed. > >> > Lightly tested - if I rm one of those two files, it gets rebuilt. If I >> > insert some whitespace, it gets replaced. If I don't touch it, the datestamp >> > doesn't change. >> >> I don't think this is wrong, but I'd really prefer to do the whole >> move-if-changed thing as a separate rule, and a separate command. >> >> IOW, could we please have something that generates the "build.h" file >> by separately creating a new temporary file, and then having the >> traditional kind of move-if-changed definition >> >> define move-if-changed >> if ! cmp -s $(1) $(2); then mv -f $(1) $(2); else rm -f $(1); fi >> endef > > We already have it and it is called "filechk." Valdis, can you check if > the below patch works equally well for you? This looks almost nice, but a few comments below. > diff --git a/Kbuild b/Kbuild > index ab8ded9..47d0630 100644 > --- a/Kbuild > +++ b/Kbuild > @@ -13,8 +13,9 @@ define sed-y > s:->::; p;}" > endef > > -quiet_cmd_offsets = GEN $@ > -define cmd_offsets > +# Use filechk to avoid rebuilds when a header changes, but the resulting file > +# does not > +define filechk_offsets > (set -e; \ > echo "#ifndef $2"; \ > echo "#define $2"; \ > @@ -24,9 +25,9 @@ define cmd_offsets > echo " * This file was generated by Kbuild"; \ > echo " */"; \ > echo ""; \ > - sed -ne $(sed-y) $<; \ > + sed -ne $(sed-y); \ > echo ""; \ > - echo "#endif" ) > $@ > + echo "#endif" ) > endef > > ##### > @@ -44,7 +45,7 @@ kernel/bounds.s: kernel/bounds.c FORCE > > $(obj)/$(bounds-file): kernel/bounds.s Kbuild You are checking the resulting file content, so the dependency on "Kbuild" is not necessary. Instead, you need to add "FORCE" so that this rule is always invoked. > $(Q)mkdir -p $(dir $@) You can drop this line because filechk automatically creates the output directory. > - $(call cmd,offsets,__LINUX_BOUNDS_H__) > + $(call filechk,offsets,__LINUX_BOUNDS_H__) To sum up, I want this part to look like this: $(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s FORCE $(call cmd,filechk,__ASM_OFFSETS_H__) > ##### > # 2) Generate asm-offsets.h > @@ -63,7 +64,7 @@ arch/$(SRCARCH)/kernel/asm-offsets.s: arch/$(SRCARCH)/kernel/asm-offsets.c \ > $(call if_changed_dep,cc_s_c) > > $(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s Kbuild > - $(call cmd,offsets,__ASM_OFFSETS_H__) > + $(call filechk,offsets,__ASM_OFFSETS_H__) Ditto. -- Best Regards Masahiro Yamada