Skip to content

lkl: fix incremental build via tools/lkl #635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thehajime merged 2 commits into from
May 27, 2026
Merged

lkl: fix incremental build via tools/lkl #635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ad6b8b
lkl: fix incremental build via tools/lkl
clingfei May 20, 2026
8ee6a43
Skip installed header read when sizes differ
clingfei May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions arch/lkl/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,36 @@ archprepare: arch/lkl/include/generated/uapi/asm/config.h

all: lkl.o arch/lkl/include/generated/uapi/asm/syscall_defs.h

# The recipe runs on every build because 'vmlinux' is PHONY in the top-level
# Makefile, so we cannot use $(if_changed) (newer-prereqs filters out PHONY,
# making it miss real vmlinux content changes). Instead, write to a tmp file
# and cmp before replacing, so $@'s mtime is preserved on no-op rebuilds and
# downstream consumers (e.g. tools/lkl/liblkl.a) are not relinked needlessly.
quiet_cmd_link_lkl = OBJCOPY $@
cmd_link_lkl = $(OBJCOPY) $(if $(KEEP_EH_FRAMES),,-R .eh_frame) \
-R .syscall_defs \
$(foreach sym,$(LKL_ENTRY_POINTS),-G$(prefix)$(sym)) \
--prefix-symbols=$(prefix) $< $@.new && \
{ if cmp -s $@.new $@; then rm -f $@.new; \
else mv -f $@.new $@; fi; }

lkl.o: vmlinux
$(OBJCOPY) $(if $(KEEP_EH_FRAMES),,-R .eh_frame) -R .syscall_defs $(foreach sym,$(LKL_ENTRY_POINTS),-G$(prefix)$(sym)) --prefix-symbols=$(prefix) vmlinux lkl.o
$(call cmd,link_lkl)

quiet_cmd_gen_syscall_defs = OBJCOPY $@
cmd_gen_syscall_defs = $(OBJCOPY) -j .syscall_defs -O binary \
--set-section-flags .syscall_defs=alloc $< $@.raw && \
sed 's/\x0//g' $@.raw > $@.new && rm -f $@.raw && \
{ if cmp -s $@.new $@; then rm -f $@.new; \
else mv -f $@.new $@; fi; }

arch/lkl/include/generated/uapi/asm/syscall_defs.h: vmlinux
$(OBJCOPY) -j .syscall_defs -O binary --set-section-flags .syscall_defs=alloc $< $@
$(Q) export tmpfile=$(shell mktemp); \
sed 's/\x0//g' $@ > $$tmpfile; mv $$tmpfile $@ ; rm -f $$tmpfile
$(call cmd,gen_syscall_defs)

install: scripts_unifdef
@echo " INSTALL $(INSTALL_PATH)/lib/lkl.o"
@mkdir -p $(INSTALL_PATH)/lib/
@cp lkl.o $(INSTALL_PATH)/lib/
@cp -p lkl.o $(INSTALL_PATH)/lib/
@$(srctree)/arch/lkl/scripts/headers_install.py \
$(subst -j,-j$(NPROC),$(findstring -j,$(MAKEFLAGS))) \
$(INSTALL_PATH)/include
Expand Down
1 change: 1 addition & 0 deletions arch/lkl/include/uapi/asm/Kbuild
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
generic-y += kvm_para.h

generated-y += config.h
generated-y += syscall_defs.h

# no header-y since we need special user headers handling
# see arch/lkl/script/headers.py
27 changes: 21 additions & 6 deletions arch/lkl/scripts/headers_install.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,16 @@ def install_headers(self):
os.makedirs(out_dir)
except:
pass
print(" INSTALL\t%s" % (out_dir + "/" + os.path.basename(h)))
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
Copy link
Copy Markdown
Member

@tavip tavip May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Author

@clingfei clingfei May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

# content, produce the final prefixed version, and only overwrite
# the final destination when content differs - preserving mtime
# on no-op rebuilds so downstream host objects are not recompiled.
raw = out_dir + "/" + os.path.basename(h) + ".raw"
ret = os.system(self.srctree+"/scripts/headers_install.sh %s %s" %
(self.relpath2abspath(h), raw))
if ret != 0:
sys.exit(1)
new_headers.add(raw)

self.headers = new_headers

Expand Down Expand Up @@ -161,7 +167,9 @@ def find_all_symbols(self):
self.defines.add("__NR_stime")

def update_header(self, h):
print(" REPLACE\t%s" % h)
# h is a .raw tmp file produced by install_headers(); the final
# destination is h without the .raw suffix.
dst = h[:-len(".raw")]
content = open(h).read()
for i in self.includes:
search_str = r"(#[ \t]*include[ \t]*[<\"][ \t]*)" + i + r"([ \t]*[>\"])"
Expand All @@ -184,7 +192,14 @@ def update_header(self, h):
search_str = r"(\W?union\s+)" + s + r"(\W)"
replace_str = "\\1" + self.lkl_prefix(s) + "\\2"
content = re.sub(search_str, replace_str, content, flags = re.MULTILINE)
open(h, 'w').write(content)
content = content.encode()
existing = None
if os.path.exists(dst) and os.stat(dst).st_size == len(content):
existing = open(dst, 'rb').read()
if content != existing:
print(" INSTALL\t%s" % dst)
open(dst, 'wb').write(content)
os.unlink(h)

def update_headers(self):
p = multiprocessing.Pool(args.jobs)
Expand Down
6 changes: 5 additions & 1 deletion tools/lkl/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ $(DOT_CONFIG): $(OUTPUT)/kernel.config
$(Q)$(MAKE) -C ../.. ARCH=lkl $(KOPT) syncconfig

# rule to build lkl.o
$(OUTPUT)lib/lkl.o: bin/stat $(DOT_CONFIG)
# FORCE is required so the recursive kernel build is always invoked; kbuild
# inside the sub-make handles incremental compilation based on source-file
# timestamps. Without FORCE, changes to kernel sources are silently ignored
# because lib/lkl.o has no dependency on them.
$(OUTPUT)lib/lkl.o: bin/stat $(DOT_CONFIG) FORCE
# this workaround is for arm32 linker (ld.gold)
$(Q)export PATH="$(srctree)/tools/lkl/bin/:${PATH}" ;\
$(MAKE) -C ../.. ARCH=lkl $(KOPT)
Expand Down
Loading