On 11/28 15:34:06, Balakrishna Garapati wrote: > With this new proposal cpu affinity is read correctly especially > when using cgroups otherwise wrong cpu mask is set. > > Fixes bug: https://bugs.linaro.org/show_bug.cgi?id=2472 > > Signed-off-by: Balakrishna Garapati <balakrishna.garap...@linaro.org> > --- > > v1 to v2: added Description of the issue to the patch commit log. > v2 to v3: Resending the patch adding the log change from v1 to v2 > > platform/linux-generic/odp_cpumask.c | 69 > +++++++++--------------------------- > 1 file changed, 16 insertions(+), 53 deletions(-) > > diff --git a/platform/linux-generic/odp_cpumask.c > b/platform/linux-generic/odp_cpumask.c > index 6bf2632..7b0d80a 100644 > --- a/platform/linux-generic/odp_cpumask.c > +++ b/platform/linux-generic/odp_cpumask.c > @@ -227,71 +227,34 @@ int odp_cpumask_next(const odp_cpumask_t *mask, int cpu) > */ > static int get_installed_cpus(void)
Should this function be renamed to something like get_available_cpus since it returns the set of CPUs on which the calling thread is eligible to run on instead of the set of CPUs in the entire system? > { > - char *numptr; > - char *endptr; > - long int cpu_idnum; > - DIR *d; > - struct dirent *dir; > + int cpu_idnum; > + cpu_set_t cpuset; > + int ret; > > /* Clear the global cpumasks for control and worker CPUs */ > odp_cpumask_zero(&odp_global_data.control_cpus); > odp_cpumask_zero(&odp_global_data.worker_cpus); > > - /* > - * Scan the /sysfs pseudo-filesystem for CPU info directories. > - * There should be one subdirectory for each installed logical CPU > - */ > - d = opendir("/sys/devices/system/cpu"); > - if (d) { > - while ((dir = readdir(d)) != NULL) { > - cpu_idnum = CPU_SETSIZE; > - > - /* > - * If the current directory entry doesn't represent > - * a CPU info subdirectory then skip to the next entry. > - */ > - if (dir->d_type == DT_DIR) { > - if (!strncmp(dir->d_name, "cpu", 3)) { > - /* > - * Directory name starts with "cpu"... > - * Try to extract a CPU ID number > - * from the remainder of the dirname. > - */ > - errno = 0; > - numptr = dir->d_name; > - numptr += 3; > - cpu_idnum = strtol(numptr, &endptr, > - 10); > - if (errno || (endptr == numptr)) > - continue; > - } else { > - continue; > - } > - } else { > - continue; > - } > - /* > - * If we get here the current directory entry specifies > - * a CPU info subdir for the CPU indexed by cpu_idnum. > - */ > + CPU_ZERO(&cpuset); > + ret = sched_getaffinity(0, sizeof(cpuset), &cpuset); It would be great to add a note in the ODP spec that the application thread calling odp_global_init() must double check its cpuset. E.g. if called from a control plane thread that has already affinitized to control plane CPUs, once odp_global_init() is called will the underlying ODP implementation (and ODP helper) only know about the cpuset of the control plane cores? > - /* Track number of logical CPUs discovered */ > - if (odp_global_data.num_cpus_installed < > - (int)(cpu_idnum + 1)) > - odp_global_data.num_cpus_installed = > - (int)(cpu_idnum + 1); > + if (ret < 0) { > + ODP_ERR("Failed to get cpu affinity"); > + return -1; > + } > > + for (cpu_idnum = 0; cpu_idnum < CPU_SETSIZE - 1; cpu_idnum++) { > + if (CPU_ISSET(cpu_idnum, &cpuset)) { > + odp_global_data.num_cpus_installed++; > /* Add the CPU to our default cpumasks */ > odp_cpumask_set(&odp_global_data.control_cpus, > - (int)cpu_idnum); > + (int)cpu_idnum); > odp_cpumask_set(&odp_global_data.worker_cpus, > - (int)cpu_idnum); > + (int)cpu_idnum); > } > - closedir(d); > - return 0; > - } else { > - return -1; > } > + > + return 0; > } > > /* > -- > 1.9.1 >