On 26 May 2016 at 07:39, Yi He <[email protected]> wrote:
> Provide helper APIs to set and get cpu affinity in ODP threads,
> and set cpu affinity to the 1st available control cpu for all odp
> test/validation programs in odp_cunit_common library.
>
> Signed-off-by: Yi He <[email protected]>
> ---
> helper/include/odp/helper/linux.h | 23 ++++++++-
> helper/linux.c | 54 +++++++++++++++++++++
> helper/test/odpthreads.c | 81
> +++++++++++++++++++++++++++----
> test/validation/common/odp_cunit_common.c | 21 ++++++--
> 4 files changed, 165 insertions(+), 14 deletions(-)
>
> diff --git a/helper/include/odp/helper/linux.h
> b/helper/include/odp/helper/linux.h
> index 2e89833..238bcf8 100644
> --- a/helper/include/odp/helper/linux.h
> +++ b/helper/include/odp/helper/linux.h
> @@ -122,7 +122,6 @@ int odph_linux_pthread_create(odph_linux_pthread_t
> *pthread_tbl,
> */
> void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int num);
>
> -
> /**
> * Fork a process
> *
> @@ -200,6 +199,28 @@ int odph_odpthreads_create(odph_odpthread_t
> *thread_tbl,
> int odph_odpthreads_join(odph_odpthread_t *thread_tbl);
>
> /**
> + * Set CPU affinity of the current odp thread
> + *
> + * CPU affinity determines the CPU core on which the thread is
> + * eligible to run.
> + *
> + * @param cpu The affinity CPU core
> + *
> + * @return 0 on success, -1 on failure
> + */
> +int odph_odpthread_setaffinity(const int cpu);
> +
> +/**
> + * Get CPU affinity of the current odp thread
> + *
> + * CPU affinity determines the CPU core on which the thread is
> + * eligible to run.
> + *
> + * @return positive cpu ID on success, -1 on failure
> + */
> +int odph_odpthread_getaffinity(void);
> +
> +/**
> * Merge getopt options
> *
> * Given two sets of getopt options (each containing possibly both short
> diff --git a/helper/linux.c b/helper/linux.c
> index 6366694..8b9dbe8 100644
> --- a/helper/linux.c
> +++ b/helper/linux.c
> @@ -12,6 +12,7 @@
> #include <sys/types.h>
> #include <sys/wait.h>
> #include <sys/prctl.h>
> +#include <sys/syscall.h>
>
> #include <stdlib.h>
> #include <string.h>
> @@ -492,6 +493,59 @@ int odph_odpthreads_join(odph_odpthread_t *thread_tbl)
> return (retval < 0) ? retval : terminated;
> }
>
> +/* man gettid() notes:
> + * Glibc does not provide a wrapper for this system call;
> + */
> +static inline pid_t __gettid(void)
> +{
> + return (pid_t)syscall(SYS_gettid);
> +}
> +
> +int odph_odpthread_setaffinity(const int cpu)
> +{
> + cpu_set_t cpuset;
> +
> + CPU_ZERO(&cpuset);
> + CPU_SET(cpu, &cpuset);
> +
> + /* determine main process or pthread based on
> + * equality of thread and thread group IDs.
> + */
> + if (__gettid() == getpid()) {
> + return sched_setaffinity(
> + 0, /* pid zero means calling process */
> + sizeof(cpu_set_t), &cpuset);
> + }
> +
> + /* on error, they return a nonzero error number. */
> + return (0 == pthread_setaffinity_np(
> + pthread_self(), sizeof(cpu_set_t), &cpuset)) ? 0 : -1;
> +}
> +
> +int odph_odpthread_getaffinity(void)
> +{
> + int cpu, result;
> + cpu_set_t cpuset;
> +
> + CPU_ZERO(&cpuset);
> + if (__gettid() == getpid()) {
> + result = sched_getaffinity(
> + 0, sizeof(cpu_set_t), &cpuset);
> + } else {
> + result = pthread_getaffinity_np(
> + pthread_self(), sizeof(cpu_set_t), &cpuset);
> + }
> +
> + /* ODP thread mean to run on single CPU core */
> + if ((result == 0) && (CPU_COUNT(&cpuset) == 1)) {
> + for (cpu = 0; cpu < CPU_SETSIZE; cpu++) {
> + if (CPU_ISSET(cpu, &cpuset))
> + return cpu;
> + }
> + }
> + return -1;
> +}
> +
> /*
> * return the number of elements in an array of getopt options, excluding
> the
> * terminating {0,0,0,0}
> diff --git a/helper/test/odpthreads.c b/helper/test/odpthreads.c
> index bba4fa5..9d1bd81 100644
> --- a/helper/test/odpthreads.c
> +++ b/helper/test/odpthreads.c
> @@ -10,17 +10,48 @@
> * the option passed to the program (--odph_proc, --odph_thread or both)
> */
>
> +#include <unistd.h>
> +#include <stdlib.h>
> +
> #include <test_debug.h>
> #include <odp_api.h>
> #include <odp/helper/linux.h>
>
> #define NUMBER_WORKERS 16
> +
> +/* register odp_term_local/global() calls atexit() */
> +static void main_exit(void);
> +
> +/* ODP application instance */
> +static odp_instance_t odp_instance;
> +
> static int worker_fn(void *arg TEST_UNUSED)
> {
> + int cpu;
> + odp_cpumask_t workers;
> +
> /* depend on the odp helper to call odp_init_local */
>
> printf("Worker thread on CPU %d\n", odp_cpu_id());
>
>
As you will send a new version (see below), I suggest the following comment
here:(I was at first a bit confused about why these 2 lines there.)
/* verify CPU affinity was already set and among the allowed worker cpu*/
+ odp_cpumask_zero(&workers);
> + odp_cpumask_default_worker(&workers, NUMBER_WORKERS);
> +
> + /* verify CPU affinity was already set */
>
and you can drop this comment then
> + cpu = odph_odpthread_getaffinity();
> + if ((cpu < 0) || !odp_cpumask_isset(&workers, cpu)) {
> + printf("Worker thread(%d)'s CPU "
> + "affinity was invalid.\n", odp_cpu_id());
> + return EXIT_FAILURE;
>
hmm. I think I would prefer return -1 here as EXIT_FAILURE is really meant
to be used as process exit code. odpthread may well be implemented as
processes, as you know, so this is a bit subjective I admit. They may as
well be pthread.
Give it a thought and tell me what you think. I will not nack this patch
based on that. (i.e. either send a new version or state you dont agree)
> + }
> +
> + /* verify helper API is workable by re-configure the same */
> + if (odph_odpthread_setaffinity(cpu) != 0) {
> + printf("Re-configure worker thread(%d)'s "
> + "CPU affinity failed.\n", odp_cpu_id());
> + return EXIT_FAILURE;
>
same comment here
+ }
> +
> /* depend on the odp helper to call odp_term_local */
>
> return 0;
>
well, if you think EXIT_FAILURE is applicable, this zero should be
EXIT_SUCCESS for consistency!, so now, you'll have to send another version
(sorry). As mentionned earlyer, my preference would go to -1 and 0 rather
than these process exit codes.
@@ -29,28 +60,55 @@ static int worker_fn(void *arg TEST_UNUSED)
> /* Create additional dataplane opdthreads */
> int main(int argc, char *argv[])
> {
> - odp_instance_t instance;
> odph_odpthread_params_t thr_params;
> odph_odpthread_t thread_tbl[NUMBER_WORKERS];
> odp_cpumask_t cpu_mask;
> int num_workers;
> - int cpu;
> + int cpu, affinity;
> int ret;
> char cpumaskstr[ODP_CPUMASK_STR_SIZE];
>
> /* let helper collect its own arguments (e.g. --odph_proc) */
> odph_parse_options(argc, argv, NULL, NULL);
>
> - if (odp_init_global(&instance, NULL, NULL)) {
> + if (odp_init_global(&odp_instance, NULL, NULL)) {
> LOG_ERR("Error: ODP global init failed.\n");
> exit(EXIT_FAILURE);
> }
>
> - if (odp_init_local(instance, ODP_THREAD_CONTROL)) {
> + if (odp_init_local(odp_instance, ODP_THREAD_CONTROL)) {
> LOG_ERR("Error: ODP local init failed.\n");
> exit(EXIT_FAILURE);
> }
>
> + /* register termination callback */
> + atexit(main_exit);
> +
> + odp_cpumask_zero(&cpu_mask);
> + /* allocate the 1st available control cpu to main process */
> + if (odp_cpumask_default_control(&cpu_mask, 1) != 1) {
> + LOG_ERR("Allocate main process CPU core failed.\n");
> + exit(EXIT_FAILURE);
> + }
> +
> + cpu = odp_cpumask_first(&cpu_mask);
> + if (odph_odpthread_setaffinity(cpu) != 0) {
> + LOG_ERR("Set main process affinify to "
> + "cpu(%d) failed.\n", cpu);
> + exit(EXIT_FAILURE);
> + }
> +
> + /* read back affinity to verify */
> + affinity = odph_odpthread_getaffinity();
> + if ((affinity < 0) || (cpu != affinity)) {
> + LOG_ERR("Verify main process affinity failed: "
> + "set(%d) read(%d).\n", cpu, affinity);
> + exit(EXIT_FAILURE);
> + }
> + cpu = 0;
> + affinity = 0;
> + odp_cpumask_zero(&cpu_mask);
> +
> /* discover how many opdthreads this system can support */
> num_workers = odp_cpumask_default_worker(&cpu_mask,
> NUMBER_WORKERS);
> if (num_workers < NUMBER_WORKERS) {
> @@ -78,7 +136,7 @@ int main(int argc, char *argv[])
> thr_params.start = worker_fn;
> thr_params.arg = NULL;
> thr_params.thr_type = ODP_THREAD_WORKER;
> - thr_params.instance = instance;
> + thr_params.instance = odp_instance;
>
> odph_odpthreads_create(&thread_tbl[0], &cpu_mask, &thr_params);
>
> @@ -86,15 +144,18 @@ int main(int argc, char *argv[])
> if (ret < 0)
> exit(EXIT_FAILURE);
>
> + return 0;
> +}
> +
> +static void main_exit(void)
> +{
> if (odp_term_local()) {
> LOG_ERR("Error: ODP local term failed.\n");
> - exit(EXIT_FAILURE);
> + _exit(EXIT_FAILURE);
> }
>
> - if (odp_term_global(instance)) {
> + if (odp_term_global(odp_instance)) {
> LOG_ERR("Error: ODP global term failed.\n");
> - exit(EXIT_FAILURE);
> + _exit(EXIT_FAILURE);
> }
> -
> - return 0;
> }
> diff --git a/test/validation/common/odp_cunit_common.c
> b/test/validation/common/odp_cunit_common.c
> index 7df9aa6..2337c92 100644
> --- a/test/validation/common/odp_cunit_common.c
> +++ b/test/validation/common/odp_cunit_common.c
> @@ -333,9 +333,24 @@ int odp_cunit_update(odp_suiteinfo_t testsuites[])
> int odp_cunit_register(odp_suiteinfo_t testsuites[])
> {
> /* call test executable init hook, if any */
> - if (global_init_term.global_init_ptr &&
> - ((*global_init_term.global_init_ptr)(&instance) != 0))
> - return -1;
> + if (global_init_term.global_init_ptr) {
> + if ((*global_init_term.global_init_ptr)(&instance) == 0) {
> + /* After ODP initialization, set main thread's
> + * CPU affinity to the 1st available control CPU
> core
> + */
> + int cpu = 0;
> + odp_cpumask_t cpuset;
> +
> + odp_cpumask_zero(&cpuset);
> + if (odp_cpumask_default_control(&cpuset, 1) == 1) {
> + cpu = odp_cpumask_first(&cpuset);
> + odph_odpthread_setaffinity(cpu);
> + }
> + } else {
> + /* ODP initialization failed */
> + return -1;
> + }
> + }
>
> CU_set_error_action(CUEA_ABORT);
>
> --
> 2.7.4
>
>
Sorry :-)
Christophe.
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
