Re: [PATCH 5/8] selftests: livepatch: sysfs: Split tests of replace attribute

[Petr Mladek]

On Fri 2026-03-13 17:58:36, Marcos Paulo de Souza wrote:
> In order to run the selftests on older kernels, split the sysfs tests to
> another file, making it able to skip the tests when the attributes
> don't exists.
> 
> No functional changes.
> 
> Signed-off-by: Marcos Paulo de Souza <mpdeso...@suse.com>
> ---
>  tools/testing/selftests/livepatch/Makefile         |  1 +
>  .../selftests/livepatch/test-sysfs-replace-attr.sh | 75 
> ++++++++++++++++++++++
>  tools/testing/selftests/livepatch/test-sysfs.sh    | 48 --------------
>  3 files changed, 76 insertions(+), 48 deletions(-)
> 
> diff --git a/tools/testing/selftests/livepatch/Makefile 
> b/tools/testing/selftests/livepatch/Makefile
> index a080eb54a215d..b95aa6e78a273 100644
> --- a/tools/testing/selftests/livepatch/Makefile
> +++ b/tools/testing/selftests/livepatch/Makefile
> @@ -10,6 +10,7 @@ TEST_PROGS := \
>       test-state.sh \
>       test-ftrace.sh \
>       test-sysfs.sh \
> +     test-sysfs-replace-attr.sh \
>       test-syscall.sh \
>       test-kprobe.sh
>  
> diff --git a/tools/testing/selftests/livepatch/test-sysfs-replace-attr.sh 
> b/tools/testing/selftests/livepatch/test-sysfs-replace-attr.sh
> new file mode 100755
> index 0000000000000..d1051211fe320
> --- /dev/null
> +++ b/tools/testing/selftests/livepatch/test-sysfs-replace-attr.sh
> @@ -0,0 +1,75 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 Song Liu <s...@kernel.org>
> +
> +. $(dirname $0)/functions.sh
> +
> +MOD_LIVEPATCH=test_klp_livepatch
> +
> +setup_config
> +
> +# - load a livepatch and verifies the sysfs replace attribute exists
> +
> +start_test "check sysfs replace attribute"
> +
> +load_lp $MOD_LIVEPATCH
> +
> +check_sysfs_exists "$MOD_LIVEPATCH" "replace"
> +file_exists=$?
> +
> +disable_lp $MOD_LIVEPATCH
> +
> +unload_lp $MOD_LIVEPATCH
> +
> +if [[ "$file_exists" == "0" ]]; then
> +     skip "sysfs attribute doesn't exists."

Nit: I would write "is not supported by this kernel version"
     instead of "doesn't exist".

     I think that it better describes the situation.
     "doesn't exist" sounds more like an error description.

> +fi

This extra code is repeated in 6th and 7th patch as well.
It would be nice to hide it into some helper function
so that we could use here something like:

        check_sysfs_attr_or_skip "replace"

or

        if has_klp_sysfs_attr "replace" ; then
                skip "sysfs \"replace\" attribute is not supported"
        fi

Best Regards,
Petr