Re: [PATCH] selftests/damon: introduce _common.sh to host shared function

[SeongJae Park <sj@kernel.org>]

Hi Enze,

On Thu, 17 Jul 2025 17:19:02 +0800 Enze Li <lienze@kylinos.cn> wrote:

> The current test scripts contain duplicated root permission checks
> in multiple locations.  This patch consolidates these checks into
> _common.sh to eliminate code redundancy.

Thank you for this patch!

> 
> Signed-off-by: Enze Li <lienze@kylinos.cn>
> ---
>  tools/testing/selftests/damon/_common.sh           | 14 ++++++++++++++
>  tools/testing/selftests/damon/lru_sort.sh          |  9 ++-------
>  tools/testing/selftests/damon/reclaim.sh           |  9 ++-------
>  tools/testing/selftests/damon/sysfs.sh             | 12 +-----------
>  .../damon/sysfs_update_removed_scheme_dir.sh       |  9 ++-------
>  5 files changed, 21 insertions(+), 32 deletions(-)
>  create mode 100644 tools/testing/selftests/damon/_common.sh
> 
> diff --git a/tools/testing/selftests/damon/_common.sh b/tools/testing/selftests/damon/_common.sh
> new file mode 100644
> index 000000000000..3920b619c30f
> --- /dev/null
> +++ b/tools/testing/selftests/damon/_common.sh
> @@ -0,0 +1,14 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# Kselftest frmework requirement - SKIP code is 4.
> +ksft_skip=4
> +
> +check_dependencies()
> +{
> +	if [ $EUID -ne 0 ]
> +	then
> +		echo "Run as root"
> +		exit $ksft_skip
> +	fi
> +}
> diff --git a/tools/testing/selftests/damon/lru_sort.sh b/tools/testing/selftests/damon/lru_sort.sh
> index 61b80197c896..0d128d809fd3 100755
> --- a/tools/testing/selftests/damon/lru_sort.sh
> +++ b/tools/testing/selftests/damon/lru_sort.sh
> @@ -1,14 +1,9 @@
>  #!/bin/bash
>  # SPDX-License-Identifier: GPL-2.0
>  
> -# Kselftest framework requirement - SKIP code is 4.
> -ksft_skip=4

As Joshua also pointed out, let's keep ksft_skip definition in each file,
unless there is no more use of the variable.

> +source _common.sh
>  
> -if [ $EUID -ne 0 ]
> -then
> -	echo "Run as root"
> -	exit $ksft_skip
> -fi
> +check_dependencies
>  
>  damon_lru_sort_enabled="/sys/module/damon_lru_sort/parameters/enabled"
>  if [ ! -f "$damon_lru_sort_enabled" ]
[...]

Other than ksft_skip, looks good to me.  Looking forward to your next version
:)


Thanks,
SJ