-
Notifications
You must be signed in to change notification settings - Fork 6.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Testcases fixes #60567
Testcases fixes #60567
Conversation
@@ -1,6 +1,7 @@ | |||
CONFIG_ZTEST=y | |||
CONFIG_ZTEST_NEW_API=y | |||
CONFIG_POSIX_API=y | |||
CONFIG_TEST_STACK_SIZE=2048 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you provide some evidence with reproducibility that the test thread stack overflows? Normally that would be part of a bug report.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might make a mistake about this CONFIG_TEST_STACK_SIZE
, I cannot find out why I set it, now remove it.
tests/kernel/spinlock/src/main.c
Outdated
trylock_failures = 0; | ||
trylock_successes = 0; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are already zero-initialized with bss. There is no need to do it explicitly here (since they are not shared between tests.
Arguably, the correct place to do this though would be in a setup function as part of the testsuite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should do it
diff --git a/tests/kernel/spinlock/src/main.c b/tests/kernel/spinlock/src/main.c
index 4ad13373d1..0ab132bf2b 100644
--- a/tests/kernel/spinlock/src/main.c
+++ b/tests/kernel/spinlock/src/main.c
@@ -223,4 +223,11 @@ ZTEST(spinlock, test_trylock)
zassert_true(trylock_successes > 0);
}
-ZTEST_SUITE(spinlock, NULL, NULL, NULL, NULL, NULL);
+static void before(void *ctx)
+{
+ ARG_UNUSED(ctx);
+
+ bounce_done = 0;
+}
+
+ZTEST_SUITE(spinlock, NULL, NULL, before, NULL, NULL);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use a before()
function in the testsuite for reinitializing bounce_done
.
provide some evidence (e.g. via bug report, console output, etc).
Fix the testcases failures by increasing the stack size. Signed-off-by: Jaxson Han <[email protected]>
The test_trylock reuses the cpu1_thread, but there is no way for it to exit. This will cause the thread created twice, as a result, two cpu running the same thread simultaneously cause an unexpected crash. Fix this by adding initialization of resources and also the exit for the cpu1_thread. Signed-off-by: Jaxson Han <[email protected]>
These testcases are not SMP-safe, will fail on all SMP boards. Simply turning them to 1cpu test can not fix the issue. So, setting CONFIG_MP_MAX_NUM_CPUS to 1 as a workaround. Signed-off-by: Jaxson Han <[email protected]>
d16f204
to
e305a3c
Compare
Added more details in the description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that this one was not being joined properly at some point. Thanks!
@povergoing Does executing ztest in SMP environment mean that several tests can run in parallel on different CPUs? If so, then indeed those test may not work in such environment. |
@rlubos Yes it does. Creating 4 threads at a 4 CPU SMP platform means 4 threads running in parallel and even it does not care about the priority of the threads. So I just set it to one CPU to pass the tests. |
Stack overflows on some arm64 platforms, so increase the stack as it's likely reaching the limit of the stack for most of the platforms.
setting
CONFIG_MAIN_STACK_SIZE=2048
because we observe the following platforms are reaching (some already exceeded) the stack limit (1152):fvp_baser_aemv8r_smp:
qemu_cortex_a53:
qemu_riscv64_smp:
qemu_x86_64:
tests: kernel: spinlock: Fix test_trylock thread reusable issue
The test_trylock reuses the cpu1_thread, but there is no way for it to exit. This will cause the thread created twice, as a result, two CPU running the same thread simultaneously cause an unexpected crash.
Analysis:
cpu1_thread
andcpu1_fn
cpu1_fn
never exitcpu1_thread
again but withtrylock_fn
. Remembercpu1_fn
might be running at another core.cpu1_thread
twice but the first one never callsk_thread_abort
. Thus, two samethread struct
living in two_cpu->current
at the same time will cause unexpected behavior.tests: net: lib: lwm2m: Use 1 cpu only as a workaround
These testcases are not SMP-safe, and will fail on all SMP boards. Simply turning them to 1cpu test can not fix the issue. So, setting CONFIG_MP_MAX_NUM_CPUS to 1 as a workaround.
To reproduce: