[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CA+CK2bAWe15SkcvWx_hRHvT-RAcudKQ1hRV1htuWanh9Mbh_YA@mail.gmail.com>
Date: Tue, 25 Nov 2025 13:42:25 -0500
From: Pasha Tatashin <pasha.tatashin@...een.com>
To: Mike Rapoport <rppt@...nel.org>
Cc: pratyush@...nel.org, jasonmiu@...gle.com, graf@...zon.com,
dmatlack@...gle.com, rientjes@...gle.com, corbet@....net,
rdunlap@...radead.org, ilpo.jarvinen@...ux.intel.com, kanie@...ux.alibaba.com,
ojeda@...nel.org, aliceryhl@...gle.com, masahiroy@...nel.org,
akpm@...ux-foundation.org, tj@...nel.org, yoann.congal@...le.fr,
mmaurer@...gle.com, roman.gushchin@...ux.dev, chenridong@...wei.com,
axboe@...nel.dk, mark.rutland@....com, jannh@...gle.com,
vincent.guittot@...aro.org, hannes@...xchg.org, dan.j.williams@...el.com,
david@...hat.com, joel.granados@...nel.org, rostedt@...dmis.org,
anna.schumaker@...cle.com, song@...nel.org, linux@...ssschuh.net,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, linux-mm@...ck.org,
gregkh@...uxfoundation.org, tglx@...utronix.de, mingo@...hat.com,
bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
rafael@...nel.org, dakr@...nel.org, bartosz.golaszewski@...aro.org,
cw00.choi@...sung.com, myungjoo.ham@...sung.com, yesanishhere@...il.com,
Jonathan.Cameron@...wei.com, quic_zijuhu@...cinc.com,
aleksander.lobakin@...el.com, ira.weiny@...el.com,
andriy.shevchenko@...ux.intel.com, leon@...nel.org, lukas@...ner.de,
bhelgaas@...gle.com, wagi@...nel.org, djeffery@...hat.com,
stuart.w.hayes@...il.com, ptyadav@...zon.de, lennart@...ttering.net,
brauner@...nel.org, linux-api@...r.kernel.org, linux-fsdevel@...r.kernel.org,
saeedm@...dia.com, ajayachandra@...dia.com, jgg@...dia.com, parav@...dia.com,
leonro@...dia.com, witu@...dia.com, hughd@...gle.com, skhawaja@...gle.com,
chrisl@...nel.org
Subject: Re: [PATCH v7 19/22] selftests/liveupdate: add test infrastructure
and scripts
On Mon, Nov 24, 2025 at 2:54 AM Mike Rapoport <rppt@...nel.org> wrote:
>
> On Sat, Nov 22, 2025 at 05:23:46PM -0500, Pasha Tatashin wrote:
> > Subject: [PATCH v7 19/22] selftests/liveupdate: add test infrastructure and scripts
>
> Maybe ^ end to end
Done.
>
> > Add the testing infrastructure required to verify the liveupdate
> > feature. This includes a custom init process, a test orchestration
> > script, and a batch runner.
>
> And say here that it's end to end test.
Done
> > +static int is_stage_2(void)
> > +{
> > + char cmdline[COMMAND_LINE_SIZE];
> > + ssize_t len;
> > + int fd;
> > +
> > + fd = open("/proc/cmdline", O_RDONLY);
> > + if (fd < 0)
> > + return 0;
> > +
> > + len = read(fd, cmdline, sizeof(cmdline) - 1);
> > + close(fd);
> > +
> > + if (len < 0)
> > + return 0;
>
> Shouldn't we bail out of the test if read of command line failed?
Sure, done.
> > +function cleanup() {
> > + local exit_code=$?
> > +
> > + if [ -z "$workspace_dir" ]; then
> > + ktap_finished
> > + return
> > + fi
> > +
> > + if [ $exit_code -ne 0 ]; then
> > + echo "# Test failed (exit code $exit_code)."
> > + echo "# Workspace preserved at: $workspace_dir"
> > + elif [ "$KEEP_WORKSPACE" -eq 1 ]; then
> > + echo "# Workspace preserved (user request) at: $workspace_dir"
> > + else
> > + rm -fr "$workspace_dir"
> > + fi
> > + ktap_finished
>
> exit $exit_code
Done
> > +function build_kernel() {
> > + local build_dir=$1
> > + local make_cmd=$2
> > + local kimage=$3
> > + local target_arch=$4
> > +
> > + local kconfig="$build_dir/.config"
> > + local common_conf="$test_dir/config"
> > + local arch_conf="$test_dir/config.$target_arch"
> > +
> > + echo "# Building kernel in: $build_dir"
> > + $make_cmd defconfig
> > +
> > + local fragments=""
> > + if [[ -f "$common_conf" ]]; then
> > + fragments="$fragments $common_conf"
> > + fi
>
> Without this CONFIG_LIVEUPDATE won't be set
> > +
> > + if [[ -f "$arch_conf" ]]; then
> > + fragments="$fragments $arch_conf"
> > + fi
> > +
> > + if [[ -n "$fragments" ]]; then
> > + "$kernel_dir/scripts/kconfig/merge_config.sh" \
> > + -Q -m -O "$build_dir" "$kconfig" $fragments >> /dev/null
> > + fi
>
> I believe you can just
>
> cat $common_conf $fragments > $build_dir/.config
> make olddefconfig
>
> without running defconfig at the beginning
> It will build faster, just make sure to add CONFIG_SERIAL_ to $arch_conf
I will look into that, so how performance really changes, I liked
using merge_config.sh as it does not print warnings.
>
> > + $make_cmd olddefconfig
> > + $make_cmd "$kimage"
> > + $make_cmd headers_install INSTALL_HDR_PATH="$headers_dir"
> > +}
> > +
> > +function mkinitrd() {
> > + local build_dir=$1
> > + local kernel_path=$2
> > + local test_name=$3
> > +
> > + # 1. Compile the test binary and the init process
>
> Didn't find 2. ;-)
> Don't think we want the numbering here, plain comments are fine
Updated comment.
>
> > + "$CROSS_COMPILE"gcc -static -O2 \
> > + -I "$headers_dir/include" \
> > + -I "$test_dir" \
> > + -o "$workspace_dir/test_binary" \
> > + "$test_dir/$test_name.c" "$test_dir/luo_test_utils.c"
>
> This will have hard time cross-compiling with -nolibc toolchains
Hm, it works for me, I am not sure with nolibc cross compiler, am I
missing something?
>
> > +
> > + "$CROSS_COMPILE"gcc -s -static -Os -nostdinc -nostdlib \
> > + -fno-asynchronous-unwind-tables -fno-ident \
> > + -fno-stack-protector \
> > + -I "$headers_dir/include" \
> > + -I "$kernel_dir/tools/include/nolibc" \
> > + -o "$workspace_dir/init" "$test_dir/init.c"
>
> This failed for me with gcc 14.2.0 (Debian 14.2.0-19):
Updated, removed the extra const, and static.
>
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c: In function ‘run_test’:
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:111:65: error: initializer element is not constant
> 111 | static const char *const argv[] = {TEST_BINARY, stage_arg, NULL};
> | ^~~~~~~~~
>
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:111:65: note: (near initialization for ‘argv[1]’)
> /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:113:37: error: passing argument 2 of ‘execve’ from incompatible pointer type [-Wincompatible-pointer-types]
> 113 | execve(TEST_BINARY, argv, NULL);
> | ^~~~
> | |
> | const char * const*
> In file included from /home/mike/git/linux/tools/testing/selftests/liveupdate/init.c:16:
> /usr/include/unistd.h:572:52: note: expected ‘char * const*’ but argument is of type ‘const char * const*’
> 572 | extern int execve (const char *__path, char *const __argv[],
> | ~~~~~~~~~~~~^~~~~~~~
>
> > +
> > + cat > "$workspace_dir/cpio_list_inner" <<EOF
> > +dir /dev 0755 0 0
> > +dir /proc 0755 0 0
> > +dir /debugfs 0755 0 0
> > +nod /dev/console 0600 0 0 c 5 1
>
> Don't you need /dev/liveupdate node?
That should be created by the kernel itself.
>
> > +file /init $workspace_dir/init 0755 0 0
> > +file /test_binary $workspace_dir/test_binary 0755 0 0
> > +EOF
> > +
> > + # Generate inner_initrd.cpio
> > + "$build_dir/usr/gen_init_cpio" "$workspace_dir/cpio_list_inner" > "$workspace_dir/inner_initrd.cpio"
> > +
> > + cat > "$workspace_dir/cpio_list" <<EOF
> > +dir /dev 0755 0 0
> > +dir /proc 0755 0 0
> > +dir /debugfs 0755 0 0
> > +nod /dev/console 0600 0 0 c 5 1
>
> And here as well.
Not needed.
>
> > +file /init $workspace_dir/init 0755 0 0
> > +file /kernel $kernel_path 0644 0 0
> > +file /test_binary $workspace_dir/test_binary 0755 0 0
> > +file /initrd.img $workspace_dir/inner_initrd.cpio 0644 0 0
> > +EOF
> > +
> > + # Generate the final initrd
> > + "$build_dir/usr/gen_init_cpio" "$workspace_dir/cpio_list" > "$initrd"
> > + local size=$(du -h "$initrd" | cut -f1)
> > +}
> > +
> > +function run_qemu() {
> > + local qemu_cmd=$1
> > + local cmdline=$2
> > + local kernel_path=$3
> > + local serial="$workspace_dir/qemu.serial"
> > +
> > + local accel="-accel tcg"
> > + local host_machine=$(uname -m)
> > +
> > + [[ "$host_machine" == "arm64" ]] && host_machine="aarch64"
> > + [[ "$host_machine" == "x86_64" ]] && host_machine="x86_64"
> > +
> > + if [[ "$qemu_cmd" == *"$host_machine"* ]]; then
> > + if [ -w /dev/kvm ]; then
> > + accel="-accel kvm"
>
> Just pass both kvm and tcg and let qemu complain.
I hated those warnings, this is why I added this "if" in the first place :-)
Thank you for your reviews, I am going to send this patch separately
from this series, so let's continue the discussion there.
Pasha
Powered by blists - more mailing lists