| Message ID | 20241203151530.16882-6-chrubis@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | First new shell library converted test | expand |
Hi! On 12/3/24 16:15, Cyril Hrubis wrote: > PATH=$PATH:$PWD/../../../lib/:$PWD/testcases/lib/:: ./vma05.sh Is this an error? It doesn't look like a correct commit message. > Signed-off-by: Cyril Hrubis<chrubis@suse.cz> > --- > testcases/kernel/mem/vma/vma05.sh | 97 ++++++++++++++++--------------- > 1 file changed, 50 insertions(+), 47 deletions(-) Andrea
Hi! On 12/3/24 16:15, Cyril Hrubis wrote: > PATH=$PATH:$PWD/../../../lib/:$PWD/testcases/lib/:: ./vma05.sh Is this an error? It doesn't look like a correct commit message. > Signed-off-by: Cyril Hrubis<chrubis@suse.cz> > --- > testcases/kernel/mem/vma/vma05.sh | 97 ++++++++++++++++--------------- > 1 file changed, 50 insertions(+), 47 deletions(-) Andrea
Hi! > > PATH=$PATH:$PWD/../../../lib/:$PWD/testcases/lib/:: ./vma05.sh > Is this an error? It doesn't look like a correct commit message. Ah, I forget to add "To test the testcase run:" or something along the lines.
Hi Cyril, > +# env > +# { > +# "needs_root": true, > +# "needs_tmpdir": true, > +# "needs_cmds": ["gdb"], > +# "save_restore": [ > +# ["/proc/sys/kernel/core_pattern", "TBROK"], > +# ["/proc/sys/kernel/core_uses_pid", "TBROK"] C API .save_restore has 3 members: struct tst_path_val { const char *path; const char *val; unsigned int flags; }; Why don't you use it here? (e.g. NULL). Kind regards, Petr > +# ], > +# "tags": [ > +# ["linux-git", "103efcd9aac1"], > +# ["linux-git", "b6558c4a2378"], > +# ["linux-git", "e5b97dde514f"] > +# ] > +# } > +# --- > -TST_SETUP=setup > -TST_CLEANUP=cleanup > -TST_TESTFUNC=vma_report_check > -TST_NEEDS_ROOT=1 > -TST_NEEDS_TMPDIR=1 > -TST_NEEDS_CMDS="gdb" > - > -CORE_LIMIT=$(ulimit -c) > -CORE_PATTERN=$(cat /proc/sys/kernel/core_pattern) > -CORE_USES_PID=$(cat /proc/sys/kernel/core_uses_pid) > - > -setup() > -{ > - ulimit -c unlimited > - echo "core" > /proc/sys/kernel/core_pattern > - echo 0 > /proc/sys/kernel/core_uses_pid > - unset DEBUGINFOD_URLS > -} > +. tst_loader.sh > -cleanup() > -{ > - ulimit -c "$CORE_LIMIT" > - echo "$CORE_PATTERN" > /proc/sys/kernel/core_pattern > - echo $CORE_USES_PID > /proc/sys/kernel/core_uses_pid > -} > +ulimit -c unlimited > +echo "core" > /proc/sys/kernel/core_pattern > +echo 0 > /proc/sys/kernel/core_uses_pid > -vma_report_check() > -{ > - if [ $(uname -m) = "x86_64" ]; then > - if LINE=$(grep "vsyscall" /proc/self/maps); then > - RIGHT="ffffffffff600000-ffffffffff601000[[:space:]][r-]-xp" > - if echo "$LINE" | grep -q "$RIGHT"; then > - tst_res TPASS "[vsyscall] reported correctly" > - else > - tst_res TFAIL "[vsyscall] reporting wrong" > - fi > +if [ $(uname -m) = "x86_64" ]; then > + if LINE=$(grep "vsyscall" /proc/self/maps); then > + RIGHT="ffffffffff600000-ffffffffff601000[[:space:]][r-]-xp" > + if echo "$LINE" | grep -q "$RIGHT"; then > + tst_res TPASS "[vsyscall] reported correctly" > + else > + tst_res TFAIL "[vsyscall] reporting wrong" > fi > fi > +fi > - rm -rf core* > - { vma05_vdso; } > /dev/null 2>&1 > - [ -f core ] || tst_brk TBROK "missing core file" > +rm -rf core* > +{ vma05_vdso; } > /dev/null 2>&1 > +[ -f core ] || tst_brk TBROK "missing core file" > - TRACE=$(gdb -silent -ex="thread apply all backtrace" -ex="quit"\ > - vma05_vdso ./core* 2> /dev/null) > - if echo "$TRACE" | grep -qF "??"; then > - tst_res TFAIL "[vdso] bug not patched" > - else > - tst_res TPASS "[vdso] backtrace complete" > - fi > -} > +TRACE=$(gdb -silent -ex="thread apply all backtrace" -ex="quit"\ > + vma05_vdso ./core* 2> /dev/null) > -. tst_test.sh > -tst_run > +if echo "$TRACE" | grep -qF "??"; then > + tst_res TFAIL "[vdso] bug not patched" > +else > + tst_res TPASS "[vdso] backtrace complete" > +fi
Hi Cyril, > +# env > +# { > +# "needs_root": true, > +# "needs_tmpdir": true, > +# "needs_cmds": ["gdb"], Maybe to add here also "uname" ? > +# "save_restore": [ > +# ["/proc/sys/kernel/core_pattern", "TBROK"], > +# ["/proc/sys/kernel/core_uses_pid", "TBROK"] > +# ], > +# "tags": [ > +# ["linux-git", "103efcd9aac1"], > +# ["linux-git", "b6558c4a2378"], > +# ["linux-git", "e5b97dde514f"] > +# ] > +# } > +# --- ... > -vma_report_check() > -{ > - if [ $(uname -m) = "x86_64" ]; then > - if LINE=$(grep "vsyscall" /proc/self/maps); then > - RIGHT="ffffffffff600000-ffffffffff601000[[:space:]][r-]-xp" > - if echo "$LINE" | grep -q "$RIGHT"; then > - tst_res TPASS "[vsyscall] reported correctly" > - else > - tst_res TFAIL "[vsyscall] reporting wrong" > - fi > +if [ $(uname -m) = "x86_64" ]; then > + if LINE=$(grep "vsyscall" /proc/self/maps); then > + RIGHT="ffffffffff600000-ffffffffff601000[[:space:]][r-]-xp" > + if echo "$LINE" | grep -q "$RIGHT"; then > + tst_res TPASS "[vsyscall] reported correctly" > + else > + tst_res TFAIL "[vsyscall] reporting wrong" > fi > fi > +fi > - rm -rf core* > - { vma05_vdso; } > /dev/null 2>&1 > - [ -f core ] || tst_brk TBROK "missing core file" > +rm -rf core* > +{ vma05_vdso; } > /dev/null 2>&1 > +[ -f core ] || tst_brk TBROK "missing core file" Test timeouts for me: # PATH=.:~/ltp.git/testcases/lib/:$PATH vma05.sh tst_tmpdir.c:316: TINFO: Using /tmp/LTP_vmaN8CQtx as tmpdir (tmpfs filesystem) tst_test.c:1890: TINFO: LTP version: 20240930-113-gffde06520 tst_test.c:1894: TINFO: Tested kernel: 6.13.0-rc1-1.g492f944-default #1 SMP PREEMPT_DYNAMIC Mon Dec 2 08:55:00 UTC 2024 (492f944) x86_64 tst_test.c:1725: TINFO: Timeout per run is 0h 00m 30s vma05.sh:57: TPASS: [vsyscall] reported correctly Test timeouted, sending SIGKILL! tst_test.c:1775: TINFO: Killed the leftover descendant processes tst_test.c:1781: TINFO: If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1 tst_test.c:1783: TBROK: Test killed! (timeout?) HINT: You _MAY_ be missing kernel fixes: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=103efcd9aac1 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b6558c4a2378 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e5b97dde514f Summary: passed 1 failed 0 broken 1 skipped 0 warnings 0 But it fails on master, therefore problem on the system I test. Although on master it was more obvious that the problem is due core dump file not being created. In new version it just timeouts. PATH=.:~/ltp.git/testcases/lib/:$PATH LTPROOT=$LTPROOT vma05.sh vma05 1 TINFO: Running: vma05.sh vma05 1 TINFO: Tested kernel: Linux ts 6.13.0-rc1-1.g492f944-default #1 SMP PREEMPT_DYNAMIC Mon Dec 2 08:55:00 UTC 2024 (492f944) x86_64 x86_64 x86_64 GNU/Linux vma05 1 TINFO: Using /tmp/LTP_vma05.I8VXfhyFt6 as tmpdir (tmpfs filesystem) vma05 1 TINFO: timeout per run is 0h 5m 0s vma05 1 TPASS: [vsyscall] reported correctly vma05 1 TBROK: missing core file vma05 1 TINFO: AppArmor enabled, this may affect test results vma05 1 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root) vma05 1 TINFO: loaded AppArmor profiles: none Summary: passed 1 failed 0 broken 1 skipped 0 warnings 0 Kind regards, Petr
Hi! > > +# env > > +# { > > +# "needs_root": true, > > +# "needs_tmpdir": true, > > +# "needs_cmds": ["gdb"], > > +# "save_restore": [ > > +# ["/proc/sys/kernel/core_pattern", "TBROK"], > > +# ["/proc/sys/kernel/core_uses_pid", "TBROK"] > C API .save_restore has 3 members: > > struct tst_path_val { > const char *path; > const char *val; > unsigned int flags; > }; > > Why don't you use it here? (e.g. NULL). I did look at the parsed metadata in the ltp.json to check if we match the format there and it seems that the parsing is broken, so there is no point in designing the interface before we fix the C parser I guess. This is there for the ksm01: "save_restore": [ [ "/sys/kernel/mm/ksm/run", "TST_SR_TBROK" ], [ "/sys/kernel/mm/ksm/sleep_millisecs", "TST_SR_TBROK" ], [ "/sys/kernel/mm/ksm/max_page_sharing", "TST_SR_SKIP_MISSING", "TST_SR_TCONF_RO" ], [ "/sys/kernel/mm/ksm/merge_across_nodes", "1", "TST_SR_SKIP_MISSING", "TST_SR_TCONF_RO" ], [ "/sys/kernel/mm/ksm/smart_scan", "0", "TST_SR_SKIP_MISSING", "TST_SR_TBROK_RO" ] ], This is because the anonymous structures are parsed by the array parsing code in the metaparse.c which is overly simplistics. And there are more bugs in there unfortunately. I can fix the most pressing problems there: - missing macro expansion - missing NULL in the middle of intiliaziation - properly concatenate entries - limited recursive #include directive Which will produce much saner output, but we will likely never be able to produce clean enough output without using a proper tooling that can do compile time arithmetics. Yes we have things like .needs_hugepages with number of hugepages defined as (5 + 1) * 5 after a macro expansion. Which ends up in the JSON as "(ARSZ + 1) * LOOP" which ends up as "(50 + 1)*5" after this patch. Which is stil way better than the mess we had there before. Anyways, I will send a patch for the metaparse.c tomorrow, so that we can fix that first.
diff --git a/testcases/kernel/mem/vma/vma05.sh b/testcases/kernel/mem/vma/vma05.sh index e1ef1014e..908a05850 100755 --- a/testcases/kernel/mem/vma/vma05.sh +++ b/testcases/kernel/mem/vma/vma05.sh @@ -1,6 +1,15 @@ #!/bin/sh +# # SPDX-License-Identifier: GPL-2.0-or-later +# # Copyright (C) 2017 Red Hat, Inc. +# Copyright (C) 2024 Cyril Hrubis <chrubis@suse.cz> +# +# --- +# doc +# +# [Description] +# # Regression test if the vsyscall and vdso VMA regions are reported correctly. # # While [vsyscall] is mostly deprecated with newer systems, there is @@ -15,58 +24,52 @@ # VM_ALWAYSDUMP)). As a consequence of this bug, VMAs were not included # in core dumps which resulted in eg. incomplete backtraces and invalid # core dump files created by gdb. +# --- +# +# --- +# env +# { +# "needs_root": true, +# "needs_tmpdir": true, +# "needs_cmds": ["gdb"], +# "save_restore": [ +# ["/proc/sys/kernel/core_pattern", "TBROK"], +# ["/proc/sys/kernel/core_uses_pid", "TBROK"] +# ], +# "tags": [ +# ["linux-git", "103efcd9aac1"], +# ["linux-git", "b6558c4a2378"], +# ["linux-git", "e5b97dde514f"] +# ] +# } +# --- -TST_SETUP=setup -TST_CLEANUP=cleanup -TST_TESTFUNC=vma_report_check -TST_NEEDS_ROOT=1 -TST_NEEDS_TMPDIR=1 -TST_NEEDS_CMDS="gdb" - -CORE_LIMIT=$(ulimit -c) -CORE_PATTERN=$(cat /proc/sys/kernel/core_pattern) -CORE_USES_PID=$(cat /proc/sys/kernel/core_uses_pid) - -setup() -{ - ulimit -c unlimited - echo "core" > /proc/sys/kernel/core_pattern - echo 0 > /proc/sys/kernel/core_uses_pid - unset DEBUGINFOD_URLS -} +. tst_loader.sh -cleanup() -{ - ulimit -c "$CORE_LIMIT" - echo "$CORE_PATTERN" > /proc/sys/kernel/core_pattern - echo $CORE_USES_PID > /proc/sys/kernel/core_uses_pid -} +ulimit -c unlimited +echo "core" > /proc/sys/kernel/core_pattern +echo 0 > /proc/sys/kernel/core_uses_pid -vma_report_check() -{ - if [ $(uname -m) = "x86_64" ]; then - if LINE=$(grep "vsyscall" /proc/self/maps); then - RIGHT="ffffffffff600000-ffffffffff601000[[:space:]][r-]-xp" - if echo "$LINE" | grep -q "$RIGHT"; then - tst_res TPASS "[vsyscall] reported correctly" - else - tst_res TFAIL "[vsyscall] reporting wrong" - fi +if [ $(uname -m) = "x86_64" ]; then + if LINE=$(grep "vsyscall" /proc/self/maps); then + RIGHT="ffffffffff600000-ffffffffff601000[[:space:]][r-]-xp" + if echo "$LINE" | grep -q "$RIGHT"; then + tst_res TPASS "[vsyscall] reported correctly" + else + tst_res TFAIL "[vsyscall] reporting wrong" fi fi +fi - rm -rf core* - { vma05_vdso; } > /dev/null 2>&1 - [ -f core ] || tst_brk TBROK "missing core file" +rm -rf core* +{ vma05_vdso; } > /dev/null 2>&1 +[ -f core ] || tst_brk TBROK "missing core file" - TRACE=$(gdb -silent -ex="thread apply all backtrace" -ex="quit"\ - vma05_vdso ./core* 2> /dev/null) - if echo "$TRACE" | grep -qF "??"; then - tst_res TFAIL "[vdso] bug not patched" - else - tst_res TPASS "[vdso] backtrace complete" - fi -} +TRACE=$(gdb -silent -ex="thread apply all backtrace" -ex="quit"\ + vma05_vdso ./core* 2> /dev/null) -. tst_test.sh -tst_run +if echo "$TRACE" | grep -qF "??"; then + tst_res TFAIL "[vdso] bug not patched" +else + tst_res TPASS "[vdso] backtrace complete" +fi
PATH=$PATH:$PWD/../../../lib/:$PWD/testcases/lib/:: ./vma05.sh Signed-off-by: Cyril Hrubis <chrubis@suse.cz> --- testcases/kernel/mem/vma/vma05.sh | 97 ++++++++++++++++--------------- 1 file changed, 50 insertions(+), 47 deletions(-)