| Message ID | 1453505859-20419-3-git-send-email-gary.robertson@linaro.org |
|---|---|
| State | New |
| Headers | show |
I'm just thinking, maybe there is reason to add some interface to take free CPUs from allowed cpumask, not allocated by other threads. Smth like odp_cpumask_free_worker and odp_cpumask_free_control(). Then use it in main thread and in helper function for executing worker threads. It can be useful in case if main thread executes several threads then after a while decides to execute several other threads, in case of "free" API there cannot be overlaps. On 23.01.16 01:37, Gary S. Robertson wrote: > These code changes depend on the addition of control and worker > cpumask pointers to the ODP initialization parameter data structure, > and implement the change in behavior suggested with that patch. > They serve as the 'glue' between the input of the new ODP API > initial cpuset masks and the use of those new cpumasks by the > ODP application or instance. > > Specifically: if both of the cpumask pointers are NULL or > if neither of the new cpumasks are populated prior to > calling odp_init_global(), then the behavior for allocation of > control and worker cpumasks is unchanged from its current > (pre-patch) state. > > However, if the cpumasks are populated and their pointers initialized > prior to calling odp_init_global() then that routine saves > their contents into global variables for later reference. > Then when odp_cpumask_default_control() or odp_cpumask_default_worker() > are called they build the caller's cpumasks based on the > saved contents of the pre-populated cpumask input. > > These changes allow multiple concurrent ODP applications > or instances to be given CPU resources which do not conflict > with one another, so multiple ODP instances can coexist harmoniously > with any isolation support on the underlying platform > as well as with one another. > > Signed-off-by: Gary S. Robertson <gary.robertson@linaro.org> > --- > platform/linux-generic/include/odp_internal.h | 2 ++ > platform/linux-generic/odp_cpumask_task.c | 48 ++++++++++++++++++++++----- > platform/linux-generic/odp_init.c | 19 +++++++++++ > platform/linux-generic/odp_system_info.c | 13 +++----- > 4 files changed, 65 insertions(+), 17 deletions(-) > > diff --git a/platform/linux-generic/include/odp_internal.h b/platform/linux-generic/include/odp_internal.h > index cdbed7d..7488b3b 100644 > --- a/platform/linux-generic/include/odp_internal.h > +++ b/platform/linux-generic/include/odp_internal.h > @@ -38,6 +38,8 @@ struct odp_global_data_s { > odp_log_func_t log_fn; > odp_abort_func_t abort_fn; > odp_system_info_t system_info; > + odp_cpumask_t control_cpus; > + odp_cpumask_t worker_cpus; > }; > > enum init_stage { > diff --git a/platform/linux-generic/odp_cpumask_task.c b/platform/linux-generic/odp_cpumask_task.c > index c5093e0..a5a9791 100644 > --- a/platform/linux-generic/odp_cpumask_task.c > +++ b/platform/linux-generic/odp_cpumask_task.c > @@ -17,10 +17,27 @@ int odp_cpumask_default_worker(odp_cpumask_t *mask, int num) > int ret, cpu, i; > cpu_set_t cpuset; > > - ret = pthread_getaffinity_np(pthread_self(), > - sizeof(cpu_set_t), &cpuset); > - if (ret != 0) > - ODP_ABORT("failed to read CPU affinity value\n"); > + CPU_ZERO(&cpuset); > + > + /* > + * If the available worker cpumask was specified prior to global init, > + * then allocate worker CPUs from that cpumask. > + */ > + if (odp_cpumask_count(&odp_global_data.worker_cpus)) { > + for (i = 0; i < CPU_SETSIZE; i++) > + if (odp_cpumask_isset(&odp_global_data.worker_cpus, i)) > + CPU_SET(i, &cpuset); > + } else { > + /* > + * No initial worker cpumask was specified. > + * So allocate worker CPUs from the available CPUs defined > + * by the default OS environment. > + */ > + ret = pthread_getaffinity_np(pthread_self(), > + sizeof(cpu_set_t), &cpuset); > + if (ret != 0) > + ODP_ABORT("failed to read CPU affinity value\n"); > + } > > odp_cpumask_zero(mask); > > @@ -28,7 +45,7 @@ int odp_cpumask_default_worker(odp_cpumask_t *mask, int num) > * If no user supplied number or it's too large, then attempt > * to use all CPUs > */ > - if (0 == num || CPU_SETSIZE < num) > + if (0 == num || CPU_COUNT(&cpuset) < num) > num = CPU_COUNT(&cpuset); > > /* build the mask, allocating down from highest numbered CPU */ > @@ -48,10 +65,25 @@ int odp_cpumask_default_worker(odp_cpumask_t *mask, int num) > > int odp_cpumask_default_control(odp_cpumask_t *mask, int num ODP_UNUSED) > { > + int cpu_count = 0; > + > odp_cpumask_zero(mask); > - /* By default all control threads on CPU 0 */ > - odp_cpumask_set(mask, 0); > - return 1; > + > + /* > + * If the available control cpumask was specified prior to global init, > + * then allocate control CPUs from that cpumask. > + */ > + cpu_count = odp_cpumask_count(&odp_global_data.control_cpus); > + if (cpu_count) > + odp_cpumask_copy(mask, &odp_global_data.control_cpus); > + > + /* CPU 0 must always be usable for control threads */ > + if (!odp_cpumask_isset(mask, 0)) { > + odp_cpumask_set(mask, 0); > + cpu_count++; > + } > + > + return cpu_count; > } > > int odp_cpumask_all_available(odp_cpumask_t *mask) > diff --git a/platform/linux-generic/odp_init.c b/platform/linux-generic/odp_init.c > index 6ad3320..38850ce 100644 > --- a/platform/linux-generic/odp_init.c > +++ b/platform/linux-generic/odp_init.c > @@ -23,6 +23,25 @@ int odp_init_global(const odp_init_t *params, > odp_global_data.log_fn = params->log_fn; > if (params->abort_fn != NULL) > odp_global_data.abort_fn = params->abort_fn; > + /* > + * Save the control and worker cpumask contents > + * in a globally accessible data structure > + * so odp_cpumask_default_control(), > + * odp_cpumask_default_worker(), and any > + * isolation support logic may reference them later. > + */ > + if (params->control_cpus && > + odp_cpumask_count(params->control_cpus)) > + odp_cpumask_copy(&odp_global_data.control_cpus, > + params->control_cpus); > + else > + odp_cpumask_zero(&odp_global_data.control_cpus); > + if (params->worker_cpus && > + odp_cpumask_count(params->worker_cpus)) > + odp_cpumask_copy(&odp_global_data.worker_cpus, > + params->worker_cpus); > + else > + odp_cpumask_zero(&odp_global_data.worker_cpus); > } > > if (odp_time_init_global()) { > diff --git a/platform/linux-generic/odp_system_info.c b/platform/linux-generic/odp_system_info.c > index de28fab..70ebf57 100644 > --- a/platform/linux-generic/odp_system_info.c > +++ b/platform/linux-generic/odp_system_info.c > @@ -8,6 +8,7 @@ > > #include <odp/system_info.h> > #include <odp_internal.h> > +#include <odp/cpumask.h> > #include <odp_debug_internal.h> > #include <odp/align.h> > #include <odp/cpu.h> > @@ -39,19 +40,13 @@ typedef struct { > > > /* > - * Report the number of CPUs in the affinity mask of the main thread > + * Report the number of CPUs available to this ODP application / instance > */ > static int sysconf_cpu_count(void) > { > - cpu_set_t cpuset; > - int ret; > - > - ret = pthread_getaffinity_np(pthread_self(), > - sizeof(cpuset), &cpuset); > - if (ret != 0) > - return 0; > + odp_cpumask_t mask; > > - return CPU_COUNT(&cpuset); > + return odp_cpumask_all_available(&mask); > } > > #if defined __x86_64__ || defined __i386__ || defined __OCTEON__ || \ >
My expectations were to confine this to helper functions which would extend odp_init_local() and odp_term_local(). The isolation helpers could wrap these functions and could modify the cpumasks which were passed into the ODP application / instance so that as a CPU was assigned to a thread it was removed from the available cpumask for that control/worker category - and when the thread terminated it could return its CPU to the available cpumask. It probably would help to make this cleaner if we use a .configure option to -enable_isolation or not - then incorporate that feature setting into compile-time conditionals in headers so it can be totally transparent to applications code... that is calling the 'wrapped' functions if isolation is enabled or the 'unwrapped' base functions if isolation is not enabled. Additionally - even if isolation support is compiled in - my helper functions will allow its execution to be turned on or off on a per application/instance basis via command line options. On Mon, Jan 25, 2016 at 10:12 AM, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > I'm just thinking, maybe there is reason to add some interface to > take free CPUs from allowed cpumask, not allocated by other threads. > Smth like odp_cpumask_free_worker and odp_cpumask_free_control(). > Then use it in main thread and in helper function for executing worker > threads. > It can be useful in case if main thread executes several threads then after > a while > decides to execute several other threads, in case of "free" API there cannot > be overlaps. > > > On 23.01.16 01:37, Gary S. Robertson wrote: >> >> These code changes depend on the addition of control and worker >> cpumask pointers to the ODP initialization parameter data structure, >> and implement the change in behavior suggested with that patch. >> They serve as the 'glue' between the input of the new ODP API >> initial cpuset masks and the use of those new cpumasks by the >> ODP application or instance. >> >> Specifically: if both of the cpumask pointers are NULL or >> if neither of the new cpumasks are populated prior to >> calling odp_init_global(), then the behavior for allocation of >> control and worker cpumasks is unchanged from its current >> (pre-patch) state. >> >> However, if the cpumasks are populated and their pointers initialized >> prior to calling odp_init_global() then that routine saves >> their contents into global variables for later reference. >> Then when odp_cpumask_default_control() or odp_cpumask_default_worker() >> are called they build the caller's cpumasks based on the >> saved contents of the pre-populated cpumask input. >> >> These changes allow multiple concurrent ODP applications >> or instances to be given CPU resources which do not conflict >> with one another, so multiple ODP instances can coexist harmoniously >> with any isolation support on the underlying platform >> as well as with one another. >> >> Signed-off-by: Gary S. Robertson <gary.robertson@linaro.org> >> --- >> platform/linux-generic/include/odp_internal.h | 2 ++ >> platform/linux-generic/odp_cpumask_task.c | 48 >> ++++++++++++++++++++++----- >> platform/linux-generic/odp_init.c | 19 +++++++++++ >> platform/linux-generic/odp_system_info.c | 13 +++----- >> 4 files changed, 65 insertions(+), 17 deletions(-) >> >> diff --git a/platform/linux-generic/include/odp_internal.h >> b/platform/linux-generic/include/odp_internal.h >> index cdbed7d..7488b3b 100644 >> --- a/platform/linux-generic/include/odp_internal.h >> +++ b/platform/linux-generic/include/odp_internal.h >> @@ -38,6 +38,8 @@ struct odp_global_data_s { >> odp_log_func_t log_fn; >> odp_abort_func_t abort_fn; >> odp_system_info_t system_info; >> + odp_cpumask_t control_cpus; >> + odp_cpumask_t worker_cpus; >> }; >> >> enum init_stage { >> diff --git a/platform/linux-generic/odp_cpumask_task.c >> b/platform/linux-generic/odp_cpumask_task.c >> index c5093e0..a5a9791 100644 >> --- a/platform/linux-generic/odp_cpumask_task.c >> +++ b/platform/linux-generic/odp_cpumask_task.c >> @@ -17,10 +17,27 @@ int odp_cpumask_default_worker(odp_cpumask_t *mask, >> int num) >> int ret, cpu, i; >> cpu_set_t cpuset; >> >> - ret = pthread_getaffinity_np(pthread_self(), >> - sizeof(cpu_set_t), &cpuset); >> - if (ret != 0) >> - ODP_ABORT("failed to read CPU affinity value\n"); >> + CPU_ZERO(&cpuset); >> + >> + /* >> + * If the available worker cpumask was specified prior to global >> init, >> + * then allocate worker CPUs from that cpumask. >> + */ >> + if (odp_cpumask_count(&odp_global_data.worker_cpus)) { >> + for (i = 0; i < CPU_SETSIZE; i++) >> + if >> (odp_cpumask_isset(&odp_global_data.worker_cpus, i)) >> + CPU_SET(i, &cpuset); >> + } else { >> + /* >> + * No initial worker cpumask was specified. >> + * So allocate worker CPUs from the available CPUs defined >> + * by the default OS environment. >> + */ >> + ret = pthread_getaffinity_np(pthread_self(), >> + sizeof(cpu_set_t), &cpuset); >> + if (ret != 0) >> + ODP_ABORT("failed to read CPU affinity value\n"); >> + } >> >> odp_cpumask_zero(mask); >> >> @@ -28,7 +45,7 @@ int odp_cpumask_default_worker(odp_cpumask_t *mask, int >> num) >> * If no user supplied number or it's too large, then attempt >> * to use all CPUs >> */ >> - if (0 == num || CPU_SETSIZE < num) >> + if (0 == num || CPU_COUNT(&cpuset) < num) >> num = CPU_COUNT(&cpuset); >> >> /* build the mask, allocating down from highest numbered CPU */ >> @@ -48,10 +65,25 @@ int odp_cpumask_default_worker(odp_cpumask_t *mask, >> int num) >> >> int odp_cpumask_default_control(odp_cpumask_t *mask, int num ODP_UNUSED) >> { >> + int cpu_count = 0; >> + >> odp_cpumask_zero(mask); >> - /* By default all control threads on CPU 0 */ >> - odp_cpumask_set(mask, 0); >> - return 1; >> + >> + /* >> + * If the available control cpumask was specified prior to global >> init, >> + * then allocate control CPUs from that cpumask. >> + */ >> + cpu_count = odp_cpumask_count(&odp_global_data.control_cpus); >> + if (cpu_count) >> + odp_cpumask_copy(mask, &odp_global_data.control_cpus); >> + >> + /* CPU 0 must always be usable for control threads */ >> + if (!odp_cpumask_isset(mask, 0)) { >> + odp_cpumask_set(mask, 0); >> + cpu_count++; >> + } >> + >> + return cpu_count; >> } >> >> int odp_cpumask_all_available(odp_cpumask_t *mask) >> diff --git a/platform/linux-generic/odp_init.c >> b/platform/linux-generic/odp_init.c >> index 6ad3320..38850ce 100644 >> --- a/platform/linux-generic/odp_init.c >> +++ b/platform/linux-generic/odp_init.c >> @@ -23,6 +23,25 @@ int odp_init_global(const odp_init_t *params, >> odp_global_data.log_fn = params->log_fn; >> if (params->abort_fn != NULL) >> odp_global_data.abort_fn = params->abort_fn; >> + /* >> + * Save the control and worker cpumask contents >> + * in a globally accessible data structure >> + * so odp_cpumask_default_control(), >> + * odp_cpumask_default_worker(), and any >> + * isolation support logic may reference them later. >> + */ >> + if (params->control_cpus && >> + odp_cpumask_count(params->control_cpus)) >> + odp_cpumask_copy(&odp_global_data.control_cpus, >> + params->control_cpus); >> + else >> + odp_cpumask_zero(&odp_global_data.control_cpus); >> + if (params->worker_cpus && >> + odp_cpumask_count(params->worker_cpus)) >> + odp_cpumask_copy(&odp_global_data.worker_cpus, >> + params->worker_cpus); >> + else >> + odp_cpumask_zero(&odp_global_data.worker_cpus); >> } >> >> if (odp_time_init_global()) { >> diff --git a/platform/linux-generic/odp_system_info.c >> b/platform/linux-generic/odp_system_info.c >> index de28fab..70ebf57 100644 >> --- a/platform/linux-generic/odp_system_info.c >> +++ b/platform/linux-generic/odp_system_info.c >> @@ -8,6 +8,7 @@ >> >> #include <odp/system_info.h> >> #include <odp_internal.h> >> +#include <odp/cpumask.h> >> #include <odp_debug_internal.h> >> #include <odp/align.h> >> #include <odp/cpu.h> >> @@ -39,19 +40,13 @@ typedef struct { >> >> >> /* >> - * Report the number of CPUs in the affinity mask of the main thread >> + * Report the number of CPUs available to this ODP application / instance >> */ >> static int sysconf_cpu_count(void) >> { >> - cpu_set_t cpuset; >> - int ret; >> - >> - ret = pthread_getaffinity_np(pthread_self(), >> - sizeof(cpuset), &cpuset); >> - if (ret != 0) >> - return 0; >> + odp_cpumask_t mask; >> >> - return CPU_COUNT(&cpuset); >> + return odp_cpumask_all_available(&mask); >> } >> >> #if defined __x86_64__ || defined __i386__ || defined __OCTEON__ || \ >> > > -- > Regards, > Ivan Khoronzhuk
diff --git a/platform/linux-generic/include/odp_internal.h b/platform/linux-generic/include/odp_internal.h index cdbed7d..7488b3b 100644 --- a/platform/linux-generic/include/odp_internal.h +++ b/platform/linux-generic/include/odp_internal.h @@ -38,6 +38,8 @@ struct odp_global_data_s { odp_log_func_t log_fn; odp_abort_func_t abort_fn; odp_system_info_t system_info; + odp_cpumask_t control_cpus; + odp_cpumask_t worker_cpus; }; enum init_stage { diff --git a/platform/linux-generic/odp_cpumask_task.c b/platform/linux-generic/odp_cpumask_task.c index c5093e0..a5a9791 100644 --- a/platform/linux-generic/odp_cpumask_task.c +++ b/platform/linux-generic/odp_cpumask_task.c @@ -17,10 +17,27 @@ int odp_cpumask_default_worker(odp_cpumask_t *mask, int num) int ret, cpu, i; cpu_set_t cpuset; - ret = pthread_getaffinity_np(pthread_self(), - sizeof(cpu_set_t), &cpuset); - if (ret != 0) - ODP_ABORT("failed to read CPU affinity value\n"); + CPU_ZERO(&cpuset); + + /* + * If the available worker cpumask was specified prior to global init, + * then allocate worker CPUs from that cpumask. + */ + if (odp_cpumask_count(&odp_global_data.worker_cpus)) { + for (i = 0; i < CPU_SETSIZE; i++) + if (odp_cpumask_isset(&odp_global_data.worker_cpus, i)) + CPU_SET(i, &cpuset); + } else { + /* + * No initial worker cpumask was specified. + * So allocate worker CPUs from the available CPUs defined + * by the default OS environment. + */ + ret = pthread_getaffinity_np(pthread_self(), + sizeof(cpu_set_t), &cpuset); + if (ret != 0) + ODP_ABORT("failed to read CPU affinity value\n"); + } odp_cpumask_zero(mask); @@ -28,7 +45,7 @@ int odp_cpumask_default_worker(odp_cpumask_t *mask, int num) * If no user supplied number or it's too large, then attempt * to use all CPUs */ - if (0 == num || CPU_SETSIZE < num) + if (0 == num || CPU_COUNT(&cpuset) < num) num = CPU_COUNT(&cpuset); /* build the mask, allocating down from highest numbered CPU */ @@ -48,10 +65,25 @@ int odp_cpumask_default_worker(odp_cpumask_t *mask, int num) int odp_cpumask_default_control(odp_cpumask_t *mask, int num ODP_UNUSED) { + int cpu_count = 0; + odp_cpumask_zero(mask); - /* By default all control threads on CPU 0 */ - odp_cpumask_set(mask, 0); - return 1; + + /* + * If the available control cpumask was specified prior to global init, + * then allocate control CPUs from that cpumask. + */ + cpu_count = odp_cpumask_count(&odp_global_data.control_cpus); + if (cpu_count) + odp_cpumask_copy(mask, &odp_global_data.control_cpus); + + /* CPU 0 must always be usable for control threads */ + if (!odp_cpumask_isset(mask, 0)) { + odp_cpumask_set(mask, 0); + cpu_count++; + } + + return cpu_count; } int odp_cpumask_all_available(odp_cpumask_t *mask) diff --git a/platform/linux-generic/odp_init.c b/platform/linux-generic/odp_init.c index 6ad3320..38850ce 100644 --- a/platform/linux-generic/odp_init.c +++ b/platform/linux-generic/odp_init.c @@ -23,6 +23,25 @@ int odp_init_global(const odp_init_t *params, odp_global_data.log_fn = params->log_fn; if (params->abort_fn != NULL) odp_global_data.abort_fn = params->abort_fn; + /* + * Save the control and worker cpumask contents + * in a globally accessible data structure + * so odp_cpumask_default_control(), + * odp_cpumask_default_worker(), and any + * isolation support logic may reference them later. + */ + if (params->control_cpus && + odp_cpumask_count(params->control_cpus)) + odp_cpumask_copy(&odp_global_data.control_cpus, + params->control_cpus); + else + odp_cpumask_zero(&odp_global_data.control_cpus); + if (params->worker_cpus && + odp_cpumask_count(params->worker_cpus)) + odp_cpumask_copy(&odp_global_data.worker_cpus, + params->worker_cpus); + else + odp_cpumask_zero(&odp_global_data.worker_cpus); } if (odp_time_init_global()) { diff --git a/platform/linux-generic/odp_system_info.c b/platform/linux-generic/odp_system_info.c index de28fab..70ebf57 100644 --- a/platform/linux-generic/odp_system_info.c +++ b/platform/linux-generic/odp_system_info.c @@ -8,6 +8,7 @@ #include <odp/system_info.h> #include <odp_internal.h> +#include <odp/cpumask.h> #include <odp_debug_internal.h> #include <odp/align.h> #include <odp/cpu.h> @@ -39,19 +40,13 @@ typedef struct { /* - * Report the number of CPUs in the affinity mask of the main thread + * Report the number of CPUs available to this ODP application / instance */ static int sysconf_cpu_count(void) { - cpu_set_t cpuset; - int ret; - - ret = pthread_getaffinity_np(pthread_self(), - sizeof(cpuset), &cpuset); - if (ret != 0) - return 0; + odp_cpumask_t mask; - return CPU_COUNT(&cpuset); + return odp_cpumask_all_available(&mask); } #if defined __x86_64__ || defined __i386__ || defined __OCTEON__ || \
These code changes depend on the addition of control and worker cpumask pointers to the ODP initialization parameter data structure, and implement the change in behavior suggested with that patch. They serve as the 'glue' between the input of the new ODP API initial cpuset masks and the use of those new cpumasks by the ODP application or instance. Specifically: if both of the cpumask pointers are NULL or if neither of the new cpumasks are populated prior to calling odp_init_global(), then the behavior for allocation of control and worker cpumasks is unchanged from its current (pre-patch) state. However, if the cpumasks are populated and their pointers initialized prior to calling odp_init_global() then that routine saves their contents into global variables for later reference. Then when odp_cpumask_default_control() or odp_cpumask_default_worker() are called they build the caller's cpumasks based on the saved contents of the pre-populated cpumask input. These changes allow multiple concurrent ODP applications or instances to be given CPU resources which do not conflict with one another, so multiple ODP instances can coexist harmoniously with any isolation support on the underlying platform as well as with one another. Signed-off-by: Gary S. Robertson <gary.robertson@linaro.org> --- platform/linux-generic/include/odp_internal.h | 2 ++ platform/linux-generic/odp_cpumask_task.c | 48 ++++++++++++++++++++++----- platform/linux-generic/odp_init.c | 19 +++++++++++ platform/linux-generic/odp_system_info.c | 13 +++----- 4 files changed, 65 insertions(+), 17 deletions(-)