diff mbox series

[v4,4/5] sysctl/sysctl02.sh: Use kconfig shell api

Message ID 1641881435-2351-4-git-send-email-xuyang2018.jy@fujitsu.com
State Accepted
Headers show
Series [v4,1/5] lib/tst_kconfig: Modify the return type of tst_kconfig_check function | expand

Commit Message

Yang Xu Jan. 11, 2022, 6:10 a.m. UTC
Reviewed-by: Li Wang <liwang@redhat.com>
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/commands/sysctl/sysctl02.sh | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

Comments

Petr Vorel Jan. 13, 2022, 10:53 a.m. UTC | #1
Hi Xu, all,

subject: "sysctl/sysctl02.sh: Use kconfig shell api"
nit: s/api/API/

Maybe to explain why it's better thatn simple [ -f /proc/kallsyms ] ?
Isn't it more reliable to use /proc/kallsyms file instead of requiring kconfig?

kconfig is sort of documentation, but only if used as TST_NEEDS_KCONFIGS
(I'm going to work on enabling docparse for shell), which is not this case.

If we agree TST_NEEDS_KCONFIGS is better in this case how about to put
sysctl_test_zero into separate file? It could then define things required setup
in TST_NEEDS_KCONFIGS.

> +++ b/testcases/commands/sysctl/sysctl02.sh
> @@ -20,15 +20,14 @@ TST_CLEANUP=cleanup
>  TST_CNT=4
>  TST_NEEDS_ROOT=1
>  TST_NEEDS_CMDS="sysctl"
> +TST_NEEDS_KCONFIGS="CONFIG_SYSCTL=y, CONFIG_PROC_FS=y"
As Cyril mentioned previously CONFIG_PROC_FS can be dropped
(IMHO mandatory for today's linux).

OK, looking into sources it's disabled in:

arch/h8300/configs/edosk2674_defconfig, the other two are GDB simulators.
=> only this one relevant (Renesas EDOSK2674R)
arch/h8300/configs/h8300h-sim_defconfig
arch/h8300/configs/h8s-sim_defconfig

arch/sh/configs/edosk7705_defconfig
arch/sh/configs/sh7724_generic_defconfig
arch/sh/configs/sh7770_generic_defconfig

=> sh and h8300 are certainly not mainstream :).
I can ask the maintainers whether they really use kernel without CONFIG_PROC_FS.

But I'd still consider to safe to expect CONFIG_PROC_FS=y.

Kind regards,
Petr
Cyril Hrubis Jan. 13, 2022, 3:54 p.m. UTC | #2
Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Petr Vorel Jan. 13, 2022, 4:06 p.m. UTC | #3
Hi Xu, all,

> If we agree TST_NEEDS_KCONFIGS is better in this case how about to put
> sysctl_test_zero into separate file? It could then define things required setup
> in TST_NEEDS_KCONFIGS.
I'm sorry, I overlook config for CONFIG_KASAN.
Splitting into separate file can be done in separate effort.
Thus let's not block this:
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr
diff mbox series

Patch

diff --git a/testcases/commands/sysctl/sysctl02.sh b/testcases/commands/sysctl/sysctl02.sh
index 3964a9829..367cd5743 100755
--- a/testcases/commands/sysctl/sysctl02.sh
+++ b/testcases/commands/sysctl/sysctl02.sh
@@ -20,15 +20,14 @@  TST_CLEANUP=cleanup
 TST_CNT=4
 TST_NEEDS_ROOT=1
 TST_NEEDS_CMDS="sysctl"
+TST_NEEDS_KCONFIGS="CONFIG_SYSCTL=y, CONFIG_PROC_FS=y"
 sys_name="fs.file-max"
 sys_file="/proc/sys/fs/file-max"
-syms_file="/proc/kallsyms"
 
 . tst_test.sh
 
 setup()
 {
-	[ ! -f "$sys_file" ] && tst_brk TCONF "$sys_file not enabled"
 	orig_value=$(cat "$sys_file")
 }
 
@@ -61,17 +60,15 @@  sysctl_test_overflow()
 
 sysctl_test_zero()
 {
-	[ ! -f "$syms_file" ] && tst_brk TCONF "$syms_file not enabled"
+	tst_check_kconfigs "CONFIG_KALLSYMS=y,CONFIG_KALLSYMS_ALL=y,CONFIG_KASAN=y" \
+		|| tst_brk TCONF "kconfig doesn't meet test's requirement!"
+
 	ROD sysctl -w -q $sys_name=0
 
-	if grep -q kasan_report $syms_file; then
-		if dmesg | grep -q "KASAN: global-out-of-bounds in __do_proc_doulongvec_minmax"; then
-			tst_res TFAIL "$sys_file is set 0 and trigger a KASAN error"
-		else
-			tst_res TPASS "$sys_file is set 0 and doesn't trigger a KASAN error"
-		fi
+	if dmesg | grep -q "KASAN: global-out-of-bounds in __do_proc_doulongvec_minmax"; then
+		tst_res TFAIL "$sys_file is set 0 and trigger a KASAN error"
 	else
-		tst_res TCONF "kernel doesn't support KASAN"
+		tst_res TPASS "$sys_file is set 0 and doesn't trigger a KASAN error"
 	fi
 }