Re: [PATCH v4 3/3] test/threads: add unit test for thread API

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

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);