Re: [PATCH v2 4/6] selftests: livepatch: Check if patched sysfs attribute exists

Petr Mladek
2026-04-17T15:21:34Z

On Mon 2026-04-13 14:26:15, Marcos Paulo de Souza wrote:
> In order to run the selftests on older kernels, check if given kernel
> has support for the attribute. If the attribute is not supported, skip
> the checks.
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> tools/testing/selftests/livepatch/test-sysfs.sh | 38 +++++++++++++++----------
> 1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/tools/testing/selftests/livepatch/test-sysfs.sh b/tools/testing/selftests/livepatch/test-sysfs.sh
> index 58fe1d96997c..a2d649404a63 100755
> --- a/tools/testing/selftests/livepatch/test-sysfs.sh
> +++ b/tools/testing/selftests/livepatch/test-sysfs.sh
> @@ -8,6 +8,8 @@ MOD_LIVEPATCH=test_klp_livepatch
> MOD_LIVEPATCH2=test_klp_callbacks_demo
> MOD_LIVEPATCH3=test_klp_syscall
>

It would be nice to add a comment in which upstream kernel version
the attribute was introduced. It might help to decide when it is
time to remove the check, see below.

> +HAS_PATCH_ATTR=0
> +
> setup_config
>
> # - load a livepatch and verifies the sysfs entries work as expected
> @@ -25,8 +27,12 @@ check_sysfs_rights "$MOD_LIVEPATCH" "stack_order" "-r--r--r--"
> check_sysfs_value "$MOD_LIVEPATCH" "stack_order" "1"
> check_sysfs_rights "$MOD_LIVEPATCH" "transition" "-r--r--r--"
> check_sysfs_value "$MOD_LIVEPATCH" "transition" "0"
> -check_sysfs_rights "$MOD_LIVEPATCH" "vmlinux/patched" "-r--r--r--"
> -check_sysfs_value "$MOD_LIVEPATCH" "vmlinux/patched" "1"
> +
> +if does_sysfs_exists "$MOD_LIVEPATCH/vmlinux" "patched"; then
> + check_sysfs_rights "$MOD_LIVEPATCH" "vmlinux/patched" "-r--r--r--"
> + check_sysfs_value "$MOD_LIVEPATCH" "vmlinux/patched" "1"
> + HAS_PATCH_ATTR=1
> +fi
>
> disable_lp $MOD_LIVEPATCH
>
> @@ -45,23 +51,24 @@ livepatch: '$MOD_LIVEPATCH': completing unpatching transition
> livepatch: '$MOD_LIVEPATCH': unpatching complete
> % rmmod $MOD_LIVEPATCH"
>
> -start_test "sysfs test object/patched"
> +if [[ "$HAS_PATCH_ATTR" == "1" ]]; then
> + start_test "sysfs test object/patched"
>
> -MOD_LIVEPATCH=test_klp_callbacks_demo
> -MOD_TARGET=test_klp_callbacks_mod
> -load_lp $MOD_LIVEPATCH
> + MOD_LIVEPATCH=test_klp_callbacks_demo
> + MOD_TARGET=test_klp_callbacks_mod
> + load_lp $MOD_LIVEPATCH
>
> -# check the "patch" file changes as target module loads/unloads
> -check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0"
> -load_mod $MOD_TARGET
> -check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "1"
> -unload_mod $MOD_TARGET
> -check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0"
> + # check the "patch" file changes as target module loads/unloads
> + check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0"
> + load_mod $MOD_TARGET
> + check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "1"
> + unload_mod $MOD_TARGET
> + check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0"
>
> -disable_lp $MOD_LIVEPATCH
> -unload_lp $MOD_LIVEPATCH
> + disable_lp $MOD_LIVEPATCH
> + unload_lp $MOD_LIVEPATCH
>
> -check_result "% insmod test_modules/test_klp_callbacks_demo.ko
> + check_result "% insmod test_modules/test_klp_callbacks_demo.ko
> livepatch: enabling patch 'test_klp_callbacks_demo'
> livepatch: 'test_klp_callbacks_demo': initializing patching transition
> test_klp_callbacks_demo: pre_patch_callback: vmlinux
> @@ -87,6 +94,7 @@ livepatch: 'test_klp_callbacks_demo': completing unpatching transition
> test_klp_callbacks_demo: post_unpatch_callback: vmlinux
> livepatch: 'test_klp_callbacks_demo': unpatching complete
> % rmmod test_klp_callbacks_demo"
> +fi

The indentation mismatch (code vs check_results) looks a bit ugly.
Well, it is rather common thing in bash scripts. And it might
even help to find where the next text starts.

I could live with it. But I am a bit biased.

I would like to see an ack from a non-SUSE person before
taking this ;-)

Best Regards,
Petr

PS: Otherwise, the patchset look good to me, modulo
the Sashiko AI comments.