[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 | 83 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 85 insertions(+)
create mode 100644 app/test/test_threads.c
Comments
2022-04-01 06:29 (UTC-0700), Tyler Retzlaff:
[...]
> +static int
> +test_thread_affinity(void)
> +{
> + pthread_t id;
> + rte_thread_t thread_id;
> +
> + RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, NULL) == 0,
> + "Failed to create thread");
> + thread_id.opaque_id = id;
The need for this hack means that the new API is unusable in practice.
I think functions to get the current thread ID and to compare IDs
must be the part of this series to accept this unit test patch.
On Fri, Apr 08, 2022 at 05:01:45PM +0300, Dmitry Kozlyuk wrote:
> 2022-04-01 06:29 (UTC-0700), Tyler Retzlaff:
> [...]
> > +static int
> > +test_thread_affinity(void)
> > +{
> > + pthread_t id;
> > + rte_thread_t thread_id;
> > +
> > + RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, NULL) == 0,
> > + "Failed to create thread");
> > + thread_id.opaque_id = id;
>
> The need for this hack means that the new API is unusable in practice.
> I think functions to get the current thread ID and to compare IDs
> must be the part of this series to accept this unit test patch.
are you proposing adding rte_thread_self and rte_thread_equals to this
series? i'm not sure i understand how that helps? while maybe the unit
test could be re-written in some fashion to use rte_thread_self it still
wouldn't do much for practically using the api outside of the test.
keep in mind i have no intention of combining api introduction and
integration into the same series so the hack is limited to the unit test
for now. it is my expectation that as subsequent series are merged the
hacks will be replaced with new api additions. i'm also not entertaining
the idea of growing this series further as we will just end up back
where we started with a series that is too large and nobody will
properly review or be bold enough to ack.
the quickest path to eliminate the hack is to improve the velocity of
getting the smaller series reviewed and merged. this is where i would
prefer to focus our effort.
if there is an absolute need to start using the api introduced in this
series outside of the unit test we could as a part of the __experimental
apis offer a function that formalizes the conversion to rte_thread_t from
pthread_t. the function would only be kept as long as needed and removed
when no longer necessary.
thanks
2022-04-09 01:56 (UTC-0700), Tyler Retzlaff:
> On Fri, Apr 08, 2022 at 05:01:45PM +0300, Dmitry Kozlyuk wrote:
> > 2022-04-01 06:29 (UTC-0700), Tyler Retzlaff:
> > [...]
> > > +static int
> > > +test_thread_affinity(void)
> > > +{
> > > + pthread_t id;
> > > + rte_thread_t thread_id;
> > > +
> > > + RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, NULL) == 0,
> > > + "Failed to create thread");
> > > + thread_id.opaque_id = id;
> >
> > The need for this hack means that the new API is unusable in practice.
> > I think functions to get the current thread ID and to compare IDs
> > must be the part of this series to accept this unit test patch.
>
> are you proposing adding rte_thread_self and rte_thread_equals to this
> series? i'm not sure i understand how that helps? while maybe the unit
> test could be re-written in some fashion to use rte_thread_self it still
> wouldn't do much for practically using the api outside of the test.
Just rte_thread_self() should be enough; see below.
How this helps: the series becomes complete, because there is enough API to
verify it works without assumptions about implementation.
[...]
> if there is an absolute need to start using the api introduced in this
> series outside of the unit test we could as a part of the __experimental
> apis offer a function that formalizes the conversion to rte_thread_t from
> pthread_t. the function would only be kept as long as needed and removed
> when no longer necessary.
I rather meant this:
main thread:
init barrier
create thread
enter barrier (wait)
use shared thread ID
thread:
init shared thread ID from rte_thread_self()
enter barrier (will unblock)
After you will add rte_thread_create() in the subsequent series,
the same logic will suit for checking that rte_thread_self()
returns the proper ID.
@@ -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,83 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2021-2022 Microsoft.
+ */
+
+#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 void *
+thread_main(void *arg)
+{
+ (void)arg;
+ 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, NULL) == 0,
+ "Failed to create thread");
+ thread_id.opaque_id = id;
+
+ 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(0 == memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)),
+ "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(0 == memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)),
+ "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(0 == memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)),
+ "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);