From bf144254616ea436d28f6e3603adbd7cb195311e Mon Sep 17 00:00:00 2001 From: Ralph Castain Date: Fri, 5 Apr 2024 06:39:21 -0600 Subject: [PATCH] Repair the binding algorithm If we use one cpu from an object, then we will get a NULL response if we ask for the next object of that type within the remaining cpuset since not all of the cpus in the object are still available. This problem resulted from the recent change to only use available cpus in PRRTE topologies. So instead scan across the cpus, check to see if it is inside the object of interest - if so, then we can bind to that cpu, if not then we keep searching. Signed-off-by: Ralph Castain --- src/hwloc/hwloc_base_util.c | 4 ++-- src/mca/rmaps/base/rmaps_base_binding.c | 18 ++++++++---------- src/mca/rmaps/base/rmaps_base_support_fns.c | 18 +++++------------- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/src/hwloc/hwloc_base_util.c b/src/hwloc/hwloc_base_util.c index b14e5b1f26..c99b67548c 100644 --- a/src/hwloc/hwloc_base_util.c +++ b/src/hwloc/hwloc_base_util.c @@ -1328,7 +1328,7 @@ void prte_hwloc_get_binding_info(hwloc_const_cpuset_t cpuset, /* if the cpuset is all zero, then something is wrong */ if (hwloc_bitmap_iszero(cpuset)) { - snprintf(cores, sz, "\n%*c\n", 20, ' '); + snprintf(cores, sz, "\n%*c\n", 20, ' '); } /* if the cpuset includes all available cpus, and @@ -1401,7 +1401,7 @@ char *prte_hwloc_base_cset2str(hwloc_const_cpuset_t cpuset, /* if the cpuset is all zero, then something is wrong */ if (hwloc_bitmap_iszero(cpuset)) { - return strdup("NOT MAPPED"); + return strdup("EMPTY CPUSET"); } /* if the cpuset includes all available cpus, and diff --git a/src/mca/rmaps/base/rmaps_base_binding.c b/src/mca/rmaps/base/rmaps_base_binding.c index 551100ea60..e5b5cce8a1 100644 --- a/src/mca/rmaps/base/rmaps_base_binding.c +++ b/src/mca/rmaps/base/rmaps_base_binding.c @@ -16,7 +16,7 @@ * Copyright (c) 2015-2017 Research Organization for Information Science * and Technology (RIST). All rights reserved. * Copyright (c) 2018 Inria. All rights reserved. - * Copyright (c) 2021-2023 Nanook Consulting All rights reserved. + * Copyright (c) 2021-2024 Nanook Consulting All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -61,6 +61,7 @@ static int bind_generic(prte_job_t *jdata, prte_proc_t *proc, hwloc_obj_type_t type; hwloc_obj_t target; hwloc_cpuset_t tgtcpus, tmpcpus; + int nobjs, n; pmix_output_verbose(5, prte_rmaps_base_framework.framework_output, "mca:rmaps: bind %s with policy %s", @@ -82,18 +83,18 @@ static int bind_generic(prte_job_t *jdata, prte_proc_t *proc, #endif hwloc_bitmap_and(prte_rmaps_base.baseset, options->target, tgtcpus); - trg_obj = NULL; - /* find the first object of that type in the target that has at least one available CPU */ - tmp_obj = hwloc_get_next_obj_inside_cpuset_by_type(node->topology->topo, - prte_rmaps_base.baseset, - options->hwb, NULL); - while (NULL != tmp_obj) { + nobjs = hwloc_get_nbobjs_by_type(node->topology->topo, options->hwb); + + for (n=0; n < nobjs; n++) { + tmp_obj = hwloc_get_obj_by_type(node->topology->topo, options->hwb, n); #if HWLOC_API_VERSION < 0x20000 tmpcpus = tmp_obj->allowed_cpuset; #else tmpcpus = tmp_obj->cpuset; #endif hwloc_bitmap_and(prte_rmaps_base.available, node->available, tmpcpus); + hwloc_bitmap_and(prte_rmaps_base.available, prte_rmaps_base.available, prte_rmaps_base.baseset); + if (options->use_hwthreads) { ncpus = hwloc_bitmap_weight(prte_rmaps_base.available); } else { @@ -112,9 +113,6 @@ static int bind_generic(prte_job_t *jdata, prte_proc_t *proc, trg_obj = tmp_obj; break; } - tmp_obj = hwloc_get_next_obj_inside_cpuset_by_type(node->topology->topo, - prte_rmaps_base.baseset, - options->hwb, tmp_obj); } if (NULL == trg_obj) { /* there aren't any appropriate targets under this object */ diff --git a/src/mca/rmaps/base/rmaps_base_support_fns.c b/src/mca/rmaps/base/rmaps_base_support_fns.c index 84389b21a9..2eb02bb3af 100644 --- a/src/mca/rmaps/base/rmaps_base_support_fns.c +++ b/src/mca/rmaps/base/rmaps_base_support_fns.c @@ -631,27 +631,18 @@ int prte_rmaps_base_get_ncpus(prte_node_t *node, { int ncpus; -#if HWLOC_API_VERSION < 0x20000 - hwloc_obj_t root; - root = hwloc_get_root_obj(node->topology->topo); if (NULL == options->job_cpuset) { - hwloc_bitmap_copy(prte_rmaps_base.available, root->allowed_cpuset); + hwloc_bitmap_copy(prte_rmaps_base.available, node->available); } else { - hwloc_bitmap_and(prte_rmaps_base.available, root->allowed_cpuset, options->job_cpuset); + hwloc_bitmap_and(prte_rmaps_base.available, node->available, options->job_cpuset); } if (NULL != obj) { +#if HWLOC_API_VERSION < 0x20000 hwloc_bitmap_and(prte_rmaps_base.available, prte_rmaps_base.available, obj->allowed_cpuset); - } #else - if (NULL == options->job_cpuset) { - hwloc_bitmap_copy(prte_rmaps_base.available, hwloc_topology_get_allowed_cpuset(node->topology->topo)); - } else { - hwloc_bitmap_and(prte_rmaps_base.available, hwloc_topology_get_allowed_cpuset(node->topology->topo), options->job_cpuset); - } - if (NULL != obj) { hwloc_bitmap_and(prte_rmaps_base.available, prte_rmaps_base.available, obj->cpuset); - } #endif + } if (options->use_hwthreads) { ncpus = hwloc_bitmap_weight(prte_rmaps_base.available); } else { @@ -664,6 +655,7 @@ int prte_rmaps_base_get_ncpus(prte_node_t *node, */ ncpus = hwloc_get_nbobjs_inside_cpuset_by_type(node->topology->topo, prte_rmaps_base.available, HWLOC_OBJ_CORE); } + return ncpus; }