On Thu, 17 Jul 2025 17:19:02 +0800 Enze Li lienze@kylinos.cn wrote:
Hi Enze,
Thank you for the patch! I just have a few comments about the patch.
The current test scripts contain duplicated root permission checks in multiple locations. This patch consolidates these checks into _common.sh to eliminate code redundancy.
Is there a reason we named the file _common.sh? IIRC there are no other files that begin with an underscore, so it might be confusing for users. Maybe remaining it to damon_common.sh might fit better with the convention used by other selftests.
[...snip...]
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
Hm, I think factoring out check_dependencies() is a good idea, but maybe we should keep ksft_skip in here since other checks in the script use the value? My 2c is that it might make it unnecessarily opaque for others. Same comment applies for the other files as well.
But I will let SJ comment on this more ;)
Thank you for your patch, I hope you have a great day! Joshua
Sent using hkml (https://github.com/sjp38/hackermail)