[v4,3/3] test/threads: add unit test for thread API
Checks
Commit Message
Establish unit test for testing thread api. Initial unit tests
for rte_thread_{get,set}_affinity_by_id().
Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
app/test/meson.build | 2 ++
app/test/test_threads.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 91 insertions(+)
create mode 100644 app/test/test_threads.c
Comments
2022-04-26 00:50 (UTC-0700), Tyler Retzlaff:
> Establish unit test for testing thread api. Initial unit tests
> for rte_thread_{get,set}_affinity_by_id().
>
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
> app/test/meson.build | 2 ++
> app/test/test_threads.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 91 insertions(+)
> create mode 100644 app/test/test_threads.c
>
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 5fc1dd1..5a9d69b 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -133,6 +133,7 @@ test_sources = files(
> 'test_tailq.c',
> 'test_thash.c',
> 'test_thash_perf.c',
> + 'test_threads.c',
> 'test_timer.c',
> 'test_timer_perf.c',
> 'test_timer_racecond.c',
> @@ -238,6 +239,7 @@ fast_tests = [
> ['reorder_autotest', true],
> ['service_autotest', true],
> ['thash_autotest', true],
> + ['threads_autotest', true],
> ['trace_autotest', true],
> ]
>
> diff --git a/app/test/test_threads.c b/app/test/test_threads.c
> new file mode 100644
> index 0000000..0ca6745
> --- /dev/null
> +++ b/app/test/test_threads.c
> @@ -0,0 +1,89 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (C) 2022 Microsoft Corporation
> + */
> +
> +#include <string.h>
> +#include <pthread.h>
> +
> +#include <rte_thread.h>
> +#include <rte_debug.h>
> +
> +#include "test.h"
> +
> +RTE_LOG_REGISTER(threads_logtype_test, test.threads, INFO);
> +
> +static uint32_t thread_id_ready;
> +
> +static void *
> +thread_main(void *arg)
> +{
> + *(rte_thread_t *)arg = rte_thread_self();
> + __atomic_store_n(&thread_id_ready, 1, __ATOMIC_RELEASE);
> +
> + return NULL;
> +}
> +
> +static int
> +test_thread_affinity(void)
> +{
> + pthread_t id;
> + rte_thread_t thread_id;
> +
> + RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
> + "Failed to create thread");
> +
> + while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
> + ;
> +
> + rte_cpuset_t cpuset0;
Variables should be declared at the beginning of the block.
> + RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset0) == 0,
> + "Failed to get thread affinity");
> +
> + rte_cpuset_t cpuset1;
> + RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset1) == 0,
> + "Failed to get thread affinity");
> + RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
> + "Affinity should be stable");
> +
> + RTE_TEST_ASSERT(rte_thread_set_affinity_by_id(thread_id, &cpuset1) == 0,
> + "Failed to set thread affinity");
> + RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset0) == 0,
> + "Failed to get thread affinity");
> + RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
> + "Affinity should be stable");
> +
> + size_t i;
> + for (i = 1; i < CPU_SETSIZE; i++)
> + if (CPU_ISSET(i, &cpuset0)) {
> + CPU_ZERO(&cpuset0);
> + CPU_SET(i, &cpuset0);
> +
> + break;
> + }
> + RTE_TEST_ASSERT(rte_thread_set_affinity_by_id(thread_id, &cpuset0) == 0,
> + "Failed to set thread affinity");
> + RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset1) == 0,
> + "Failed to get thread affinity");
> + RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
> + "Affinity should be stable");
The message is not really relevant to the check done.
"Retrieved affinity differs from requested"?
I think this is the only check worth keeping in this test.
The fist one is speculative: if we expect that a wrong implementation may
change affinity sporadically, the test can't prove it doesn't.
The second one isn't stronger than this one.
> +
> + return 0;
> +}
> +
> +static struct unit_test_suite threads_test_suite = {
> + .suite_name = "threads autotest",
> + .setup = NULL,
> + .teardown = NULL,
> + .unit_test_cases = {
> + TEST_CASE(test_thread_affinity),
> + TEST_CASES_END()
> + }
> +};
> +
> +static int
> +test_threads(void)
> +{
> + return unit_test_suite_runner(&threads_test_suite);
> +}
> +
> +REGISTER_TEST_COMMAND(threads_autotest, test_threads);
On Sun, May 01, 2022 at 12:18:36PM +0300, Dmitry Kozlyuk wrote:
> 2022-04-26 00:50 (UTC-0700), Tyler Retzlaff:
> > Establish unit test for testing thread api. Initial unit tests
> > for rte_thread_{get,set}_affinity_by_id().
> >
> > Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> > app/test/meson.build | 2 ++
> > app/test/test_threads.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 91 insertions(+)
> > create mode 100644 app/test/test_threads.c
> >
> > diff --git a/app/test/meson.build b/app/test/meson.build
> > index 5fc1dd1..5a9d69b 100644
> > --- a/app/test/meson.build
> > +++ b/app/test/meson.build
> > @@ -133,6 +133,7 @@ test_sources = files(
> > 'test_tailq.c',
> > 'test_thash.c',
> > 'test_thash_perf.c',
> > + 'test_threads.c',
> > 'test_timer.c',
> > 'test_timer_perf.c',
> > 'test_timer_racecond.c',
> > @@ -238,6 +239,7 @@ fast_tests = [
> > ['reorder_autotest', true],
> > ['service_autotest', true],
> > ['thash_autotest', true],
> > + ['threads_autotest', true],
> > ['trace_autotest', true],
> > ]
> >
> > diff --git a/app/test/test_threads.c b/app/test/test_threads.c
> > new file mode 100644
> > index 0000000..0ca6745
> > --- /dev/null
> > +++ b/app/test/test_threads.c
> > @@ -0,0 +1,89 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright (C) 2022 Microsoft Corporation
> > + */
> > +
> > +#include <string.h>
> > +#include <pthread.h>
> > +
> > +#include <rte_thread.h>
> > +#include <rte_debug.h>
> > +
> > +#include "test.h"
> > +
> > +RTE_LOG_REGISTER(threads_logtype_test, test.threads, INFO);
> > +
> > +static uint32_t thread_id_ready;
> > +
> > +static void *
> > +thread_main(void *arg)
> > +{
> > + *(rte_thread_t *)arg = rte_thread_self();
> > + __atomic_store_n(&thread_id_ready, 1, __ATOMIC_RELEASE);
> > +
> > + return NULL;
> > +}
> > +
> > +static int
> > +test_thread_affinity(void)
> > +{
> > + pthread_t id;
> > + rte_thread_t thread_id;
> > +
> > + RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
> > + "Failed to create thread");
> > +
> > + while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
> > + ;
> > +
> > + rte_cpuset_t cpuset0;
>
> Variables should be declared at the beginning of the block.
just curious because we don't require c99 compiler or because it is
not compliant with dpdk style/convention?
regardless, will fix just curious.
>
> > + RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset0) == 0,
> > + "Failed to get thread affinity");
> > +
> > + rte_cpuset_t cpuset1;
> > + RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset1) == 0,
> > + "Failed to get thread affinity");
> > + RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
> > + "Affinity should be stable");
> > +
> > + RTE_TEST_ASSERT(rte_thread_set_affinity_by_id(thread_id, &cpuset1) == 0,
> > + "Failed to set thread affinity");
> > + RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset0) == 0,
> > + "Failed to get thread affinity");
> > + RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
> > + "Affinity should be stable");
> > +
> > + size_t i;
> > + for (i = 1; i < CPU_SETSIZE; i++)
> > + if (CPU_ISSET(i, &cpuset0)) {
> > + CPU_ZERO(&cpuset0);
> > + CPU_SET(i, &cpuset0);
> > +
> > + break;
> > + }
> > + RTE_TEST_ASSERT(rte_thread_set_affinity_by_id(thread_id, &cpuset0) == 0,
> > + "Failed to set thread affinity");
> > + RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset1) == 0,
> > + "Failed to get thread affinity");
> > + RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
> > + "Affinity should be stable");
>
> The message is not really relevant to the check done.
> "Retrieved affinity differs from requested"?
will fix.
>
> I think this is the only check worth keeping in this test.
> The fist one is speculative: if we expect that a wrong implementation may
> change affinity sporadically, the test can't prove it doesn't.
if you're saying it isn't deterministic i partially agree. regardless
many bugs are not deterministic. e.g. composite/split initialization bugs
as a class, there could be side-effects get/get since the function is
not pure.
enforcing a rule that tests only ever fail for deterministic reasons
would eliminate opportunity to hint at various classes of initialization
bugs. as an example right now pthread_create shim has one of these class
of bugs and there is no deterministic test that guarantees 100% failure
though a non-deterministic test enabled today would show a high rate of
failure warranting investigation before now.
the only downside is that if a test like this does fail it is possible
to be unrelated to the implementation of the feature being tested and
that can be misleading and i admit frustrating.
> The second one isn't stronger than this one.
i agree that the set/get/compare is duplicated i'll remove it the
first occurence but the get/get/compare i think should stay.
>
> > +
> > + return 0;
> > +}
> > +
> > +static struct unit_test_suite threads_test_suite = {
> > + .suite_name = "threads autotest",
> > + .setup = NULL,
> > + .teardown = NULL,
> > + .unit_test_cases = {
> > + TEST_CASE(test_thread_affinity),
> > + TEST_CASES_END()
> > + }
> > +};
> > +
> > +static int
> > +test_threads(void)
> > +{
> > + return unit_test_suite_runner(&threads_test_suite);
> > +}
> > +
> > +REGISTER_TEST_COMMAND(threads_autotest, test_threads);
@@ -133,6 +133,7 @@ test_sources = files(
'test_tailq.c',
'test_thash.c',
'test_thash_perf.c',
+ 'test_threads.c',
'test_timer.c',
'test_timer_perf.c',
'test_timer_racecond.c',
@@ -238,6 +239,7 @@ fast_tests = [
['reorder_autotest', true],
['service_autotest', true],
['thash_autotest', true],
+ ['threads_autotest', true],
['trace_autotest', true],
]
new file mode 100644
@@ -0,0 +1,89 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2022 Microsoft Corporation
+ */
+
+#include <string.h>
+#include <pthread.h>
+
+#include <rte_thread.h>
+#include <rte_debug.h>
+
+#include "test.h"
+
+RTE_LOG_REGISTER(threads_logtype_test, test.threads, INFO);
+
+static uint32_t thread_id_ready;
+
+static void *
+thread_main(void *arg)
+{
+ *(rte_thread_t *)arg = rte_thread_self();
+ __atomic_store_n(&thread_id_ready, 1, __ATOMIC_RELEASE);
+
+ return NULL;
+}
+
+static int
+test_thread_affinity(void)
+{
+ pthread_t id;
+ rte_thread_t thread_id;
+
+ RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
+ "Failed to create thread");
+
+ while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+ ;
+
+ rte_cpuset_t cpuset0;
+ RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset0) == 0,
+ "Failed to get thread affinity");
+
+ rte_cpuset_t cpuset1;
+ RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset1) == 0,
+ "Failed to get thread affinity");
+ RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+ "Affinity should be stable");
+
+ RTE_TEST_ASSERT(rte_thread_set_affinity_by_id(thread_id, &cpuset1) == 0,
+ "Failed to set thread affinity");
+ RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset0) == 0,
+ "Failed to get thread affinity");
+ RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+ "Affinity should be stable");
+
+ size_t i;
+ for (i = 1; i < CPU_SETSIZE; i++)
+ if (CPU_ISSET(i, &cpuset0)) {
+ CPU_ZERO(&cpuset0);
+ CPU_SET(i, &cpuset0);
+
+ break;
+ }
+ RTE_TEST_ASSERT(rte_thread_set_affinity_by_id(thread_id, &cpuset0) == 0,
+ "Failed to set thread affinity");
+ RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset1) == 0,
+ "Failed to get thread affinity");
+ RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+ "Affinity should be stable");
+
+ return 0;
+}
+
+static struct unit_test_suite threads_test_suite = {
+ .suite_name = "threads autotest",
+ .setup = NULL,
+ .teardown = NULL,
+ .unit_test_cases = {
+ TEST_CASE(test_thread_affinity),
+ TEST_CASES_END()
+ }
+};
+
+static int
+test_threads(void)
+{
+ return unit_test_suite_runner(&threads_test_suite);
+}
+
+REGISTER_TEST_COMMAND(threads_autotest, test_threads);