lkl: fix incremental build via tools/lkl#635
Conversation
`make -C tools/lkl` had two related problems: 1. Correctness: modifying any kernel source file did not trigger a rebuild. tools/lkl/Makefile's lib/lkl.o rule depended only on bin/stat and $(DOT_CONFIG), so the recursive kernel make was never invoked when only kernel sources changed. Host libraries got relinked over a stale lib/lkl.o, masking the failure. 2. Performance: even with no source changes, several pipeline stages unconditionally bumped output mtimes, causing the whole chain (objcopy -> install -> all ~200 LKL headers -> every host .o -> liblkl.a -> every .so) to fire on every make. The patch consists of following modifications: tools/lkl/Makefile: add FORCE to lib/lkl.o so the recursive make is always entered; kbuild's own incremental check then decides what to rebuild from source timestamps. arch/lkl/include/uapi/asm/Kbuild: declare syscall_defs.h as generated-y. Without this, scripts/Makefile.asm-generic treats the file as a stale wrapper and REMOVEs it on every build, forcing arch/lkl/Makefile to re-extract it via objcopy. arch/lkl/Makefile: rewrite the lkl.o and syscall_defs.h rules to objcopy to a tmp file and cmp before replacing $@. vmlinux is PHONY in the top-level Makefile so its recipe always runs; if_changed cannot be used here (newer-prereqs filters out PHONY prereqs and would silently skip real vmlinux content changes), so an in-recipe cmp preserves mtime on no-op rebuilds while still updating on real changes. Pass -p to the install cp so the preserved mtime propagates to tools/lkl/lib/lkl.o. arch/lkl/scripts/headers_install.py: stage the two-pass install. The first pass writes scripts/headers_install.sh output to dst.raw. The second pass (update_header) reads dst.raw, produces the final lkl_-prefixed content, and only writes dst when the content differs from the existing destination. After applying this patch, editing any kernel source correctly propagates to the final libraries, and a no-op `make` in tools/lkl performs no host-side work -- only the two unavoidable OBJCOPY invocations against the PHONY vmlinux target. A no-op make would run as follows: ``` make -C tools/lkl MMU=1 -j128 make: Entering directory '/path/to/linux/tools/lkl' CALL scripts/checksyscalls.sh OBJCOPY lkl.o OBJCOPY arch/lkl/include/generated/uapi/asm/syscall_defs.h make -C ../.. ARCH=lkl install INSTALL_PATH=/path/to/linux/tools/lkl/ INSTALL linux/tools/lkl//lib/lkl.o make: Leaving directory '/path/to/linux/tools/lkl' ``` Signed-off-by: Cheng Lingfei <1599101385@qq.com>
Same issue commit 8b61f60 ("lkl tests: avoid variable test names") fixed for disk.sh: lklfuse.sh passes the random mktemp paths $file/$dir /$lock_file as arguments to lkl_test_run, which embeds them into the test name. The GitHub test reporter then sees 64 "removed" and 64 "added" tests on every CI run because the temp paths differ run to run. See lkl#635 (comment). Drop the random-path arguments from lkl_test_run calls and reference the outer-scope $file/$dir/$lock_file directly inside each function. $fstype stays as an argument since it is stable across runs. Also update the commented-out stress-ng call so a future uncomment does not re-introduce the issue. Signed-off-by: Cheng Lingfei <1599101385@qq.com>
| os.system(self.srctree+"/scripts/headers_install.sh %s %s" % (self.relpath2abspath(h), | ||
| out_dir + "/" + os.path.basename(h))) | ||
| new_headers.add(out_dir + "/" + os.path.basename(h)) | ||
| # Install to a .raw tmp first. update_header() will read the raw |
There was a problem hiding this comment.
Could we instead set mtime of the processed header to the one of the original header and only process it again if the source mtime is newer than the target mtime?
There was a problem hiding this comment.
I think a per-header source-vs-target mtime check is not safe here. The processed output of one header is not determined only by that header: headers_install.py first scans all selected headers and builds a global set of
includes/defines/structs/unions to prefix.
Let's consider the following case: A may contain an existing reference to an incomplete/opaque struct type, such as "struct foo *", even if no header previously defined "struct foo". If another header later adds the actual definition of "struct foo", the global set of struct tags discovered by headers_install.py changes, and A's already-existing reference now needs to be prefixed as well. A's source file did not change, but its processed output should. Consequently, skipping A just because its source mtime is not newer than the target mtime could leave stale installed headers.
Also, setting the installed header mtime back to the source mtime could hide output changes caused by changes in the processing scripts or prefixing rules. That is why I think content comparison is the safer approach: regenerate the processed content, but only update the installed header when the content really changed.
|
thanks too, looks good to me. |
ddiss
left a comment
ddiss
left a comment
There was a problem hiding this comment.
Acked-by: David Disseldorp <ddiss@suse.de>
Avoid reading the existing header when its st_size differs from the newly generated content length, since the contents cannot match in that case. Keep the full byte-for-byte comparison when sizes are equal. Signed-off-by: Cheng Lingfei <chenglingfei@foxmail.com>
4 participants
make -C tools/lklhad two related problems:Correctness: modifying any kernel source file did not trigger a rebuild. tools/lkl/Makefile's lib/lkl.o rule depended only on bin/stat and $(DOT_CONFIG), so the recursive kernel make was never invoked when only kernel sources changed. Host libraries got relinked over a stale lib/lkl.o, masking the failure.
Performance: even with no source changes, several pipeline stages unconditionally bumped output mtimes, causing the whole chain (objcopy -> install -> all ~200 LKL headers -> every host .o -> liblkl.a -> every .so) to fire on every make.
The patch consists of following modifications:
tools/lkl/Makefile: add FORCE to lib/lkl.o so the recursive make is always entered; kbuild's own incremental check then decides what to rebuild from source timestamps.
arch/lkl/include/uapi/asm/Kbuild: declare syscall_defs.h as generated-y. Without this, scripts/Makefile.asm-generic treats the file as a stale wrapper and REMOVEs it on every build, forcing arch/lkl/Makefile to re-extract it via objcopy.
arch/lkl/Makefile: rewrite the lkl.o and syscall_defs.h rules to objcopy to a tmp file and cmp before replacing $@. vmlinux is PHONY in the top-level Makefile so its recipe always runs; if_changed cannot be used here (newer-prereqs filters out PHONY prereqs and would silently skip real vmlinux content changes), so an in-recipe cmp preserves mtime on no-op rebuilds while still updating on real changes. Pass -p to the install cp so the preserved mtime propagates to tools/lkl/lib/lkl.o.
arch/lkl/scripts/headers_install.py: stage the two-pass install. The first pass writes scripts/headers_install.sh output to dst.raw. The second pass (update_header) reads dst.raw, produces the final lkl_-prefixed content, and only writes dst when the content differs from the existing destination.
After applying this patch, editing any kernel source correctly propagates to the final libraries, and a no-op
makein tools/lkl performs no host-side work -- only the two unavoidable OBJCOPY invocations against the PHONY vmlinux target. A no-op make would run as follows: