Since this patch requires the API change it would be better to send those two together as a patch set. See other comments below.
> -----Original Message----- > From: EXT Gary S. Robertson [mailto:gary.robert...@linaro.org] > Sent: Saturday, January 23, 2016 1:38 AM > To: mike.hol...@linaro.org; bill.fischo...@linaro.org; > maxim.uva...@linaro.org; anders.rox...@linaro.org; > stuart.has...@linaro.org; petri.savolai...@linaro.org > Cc: lng-odp@lists.linaro.org; Gary S. Robertson > Subject: [lng-odp][API_NEXT PATCH V3 01/01] linux-generic: add support for > initial cpumasks > > 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.robert...@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"); > + } Maybe this belongs to global_init and this function would just copy out the already initialized mask. See under. > > 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++; > + } If user has passed a mask that has only e.g. CPU 1 set, implementation should return only that and not add CPU 0 into the mask. So, this would be an else branch (by default CPU0, if user didn't pass a mask). > + > + 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); The check for the count is redundant. Also, I think the mask should be initialized already here - instead of expecting that user will call odp_cpumask_default_xxx(). If user don't call it, other operations may fail later since these mask are empty. /* Sanity check masks */ if (params->control_cpus && params->worker_cpus) if (masks overlap) return -1; if (params->control_cpus) if (odp_cpumask_last(params->control_cpus) > maximum CPU id in the system) return -1; ... if (params->control_cpus) /* copy user defined CPUs */ odp_cpumask_copy(&odp_global_data.control_cpus, params->control_cpus); else /* Use implementation defined defaults */ odp_cpumask_zero(&odp_global_data.control_cpus); ... set default CPUs ... -Petri > + 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__ || \ > -- > 1.9.1 _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
