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

[Petr Mladek <pmladek@suse.com>]

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.