Message-ID: <Y6WUlth8KrR6EcsI@bergen.fjasle.eu>
Date:   Fri, 23 Dec 2022 12:44:22 +0100
From:   Nicolas Schier <nicolas@...sle.eu>
To:     Masahiro Yamada <masahiroy@...nel.org>
Cc:     linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: [PATCH] kbuild: use .NOTINTERMEDIATE for future GNU Make versions

On Sun 11 Dec 2022 12:10:59 GMT, Masahiro Yamada wrote:
> In Kbuild, some files are generated by chains of pattern/implicit 
> rules.
> For example, *.dtb.o files in drivers/of/unittest-data/Makefile are
> generated by the chain of 3 pattern rules, like this:
> 
>   %.dts  ->  %.dtb  ->  %.dtb.S  ->  %.dtb.o
> 
> Here, %.dts is the real source, %.dtb.o is the final target.
> %.dtb and %.dtb.S are called "intermediate files".
> 
> As GNU Make manual [1] says, intermediate files are treated differently
> in two ways:
> 
>  (a) The first difference is what happens if the intermediate file does
>    not exist. If an ordinary file 'b' does not exist, and make considers
>    a target that depends on 'b', it invariably creates 'b' and then
>    updates the target from 'b'. But if 'b' is an intermediate file, then
>    make can leave well enough alone: it won't create 'b' unless one of
>    its prerequisites is out of date. This means the target depending
>    on 'b' won't be rebuilt either, unless there is some other reason
>    to update that target: for example the target doesn't exist or a
>    different prerequisite is newer than the target.
> 
>  (b) The second difference is that if make does create 'b' in order to
>    update something else, it deletes 'b' later on after it is no longer
>    needed. Therefore, an intermediate file which did not exist before
>    make also does not exist after make. make reports the deletion to
>    you by printing a 'rm' command showing which file it is deleting.
> 
> Actually, (b) is problematic for Kbuild because most of the build rules
> depend on FORCE and the if_changed* macros really determine if the
> target should be updated. So, all missing files, whether they are
> intermediate or not, are always rebuilt.
> 
> To see why (b) is a problem, delete ".SECONDARY:" from
> scripts/Kbuild.include, and repeat this command:
> 
>   $ make allmodconfig drivers/of/unittest-data/
> 
> The intermediate files will be deleted, which results in rebuilding
> intermediate and final objects in the next run of make.
> 
> In the old days, people suppressed (b) in inconsistent ways.
> As commit 54a702f70589 ("kbuild: mark $(targets) as .SECONDARY and
> remove .PRECIOUS markers") noted, you should not use .PRECIOUS because
> .PRECIOUS has the following behavior (c), which is not likely what you
> want.
> 
>  (c) If make is killed or interrupted during the execution of their
>    recipes, the target is not deleted. Also, the target is not deleted
>    on error even if .DELETE_ON_ERROR is specified.
> 
> .SECONDARY is a much better way to disable (b), but a small problem
> is that .SECONDARY enables (a), which gives a side-effect to $?;
> prerequisites marked as .SECONDARY do not appear in $?. This is a
> drawback for Kbuild.
> 
> I thought it was a bug and opened a bug report. As Paul, the GNU Make
> maintainer, concluded in [2], this is not a bug.
> 
> A good news is that, GNU Make 4.4 added the perfect solution,
> .NOTINTERMEDIATE, which cancels both (a) and (b).
> 
> For clarificaton, my understanding of .INTERMEDIATE, .SECONDARY,
> .PRECIOUS and .NOTINTERMEDIATE are as follows:
> 
>                         (a)         (b)         (c)
>   .INTERMEDIATE        enable      enable      disable
>   .SECONDARY           enable      disable     disable
>   .PRECIOUS            disable     disable     enable
>   .NOTINTERMEDIATE     disable     disable     disable
> 
> However, GNU Make 4.4 has a bug for the global .NOTINTERMEDIATE. [3]
> It was fixed by commit 6164608900ad ("[SV 63417] Ensure global
> .NOTINTERMEDIATE disables all intermediates"), and will be available
> in the next release of GNU Make.
> 
> The following is the gain for .NOTINTERMEDIATE:
> 
>   [Current Make]
> 
>       $ make allnoconfig vmlinux
>           [ full build ]
>       $ rm include/linux/device.h
>       $ make vmlinux
>         CALL    scripts/checksyscalls.sh
> 
>   Make does not notice the removal of <linux/device.h>.
> 
>   [Future Make]
> 
>       $ make-latest allnoconfig vmlinux
>           [ full build ]
>       $ rm include/linux/device.h
>       $ make-latest vmlinux
>         CC      arch/x86/kernel/asm-offsets.s
>       In file included from ./include/linux/writeback.h:13,
>                        from ./include/linux/memcontrol.h:22,
>                        from ./include/linux/swap.h:9,
>                        from ./include/linux/suspend.h:5,
>                        from arch/x86/kernel/asm-offsets.c:13:
>       ./include/linux/blk_types.h:11:10: fatal error: linux/device.h: No such file or directory
>          11 | #include <linux/device.h>
>             |          ^~~~~~~~~~~~~~~~
>       compilation terminated.
>       make-latest[1]: *** [scripts/Makefile.build:114: arch/x86/kernel/asm-offsets.s] Error 1
>       make-latest: *** [Makefile:1282: prepare0] Error 2
> 
>   Make notices the removal of <linux/device.h>, and rebuilds objects
>   that depended on <linux/device.h>. There exists a source file that
>   includes <linux/device.h>, and it raises an error.
> 
> To see detailed background information, refer to commit 2d3b1b8f0da7
> ("kbuild: drop $(wildcard $^) check in if_changed* for faster rebuild").
> 
> [1]: https://www.gnu.org/software/make/manual/make.html#Chained-Rules
> [2]: https://savannah.gnu.org/bugs/?55532
> [3]: https://savannah.gnu.org/bugs/?63417
> 
> Signed-off-by: Masahiro Yamada <masahiroy@...nel.org>
> ---
> 
>  scripts/Kbuild.include | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 

I really do appreciate the detailed and enlightening reasoning.

Reviewed-by: Nicolas Schier <nicolas@...sle.eu>

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists