From bcdb1f45aca3f6dfab2646bfdba99775f728ca3b Mon Sep 17 00:00:00 2001 From: Ralph Castain Date: Wed, 25 Jul 2018 17:42:19 -0700 Subject: [PATCH] Fix the multiple pe/proc option Things got a little out of whack and we weren't actually processing the map-by modifiers, plus an error crept into the display of the binding report. So clean those up. Thanks to @tonyreina for the error report Signed-off-by: Ralph Castain --- opal/mca/pmix/pmix3x/pmix3x_component.c | 2 +- orte/mca/rmaps/base/rmaps_base_frame.c | 34 ++++++++------ .../data_type_support/orte_dt_print_fns.c | 46 ++++++++++--------- 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/opal/mca/pmix/pmix3x/pmix3x_component.c b/opal/mca/pmix/pmix3x/pmix3x_component.c index 9020d44bb1..3477641092 100644 --- a/opal/mca/pmix/pmix3x/pmix3x_component.c +++ b/opal/mca/pmix/pmix3x/pmix3x_component.c @@ -118,7 +118,7 @@ static int external_open(void) OBJ_CONSTRUCT(&mca_pmix_pmix3x_component.dmdx, opal_list_t); version = PMIx_Get_version(); - if ('3' >= version[0]) { + if ('3' > version[0]) { opal_show_help("help-pmix-base.txt", "incorrect-pmix", true, version, "v3.x"); return OPAL_ERROR; diff --git a/orte/mca/rmaps/base/rmaps_base_frame.c b/orte/mca/rmaps/base/rmaps_base_frame.c index 9c71cdd990..9300b338dd 100644 --- a/orte/mca/rmaps/base/rmaps_base_frame.c +++ b/orte/mca/rmaps/base/rmaps_base_frame.c @@ -597,7 +597,7 @@ int orte_rmaps_base_set_mapping_policy(orte_mapping_policy_t *policy, char **device, char *inspec) { char *ck; - char *ptr; + char *ptr, *cptr; orte_mapping_policy_t tmp; int rc; size_t len; @@ -661,20 +661,26 @@ int orte_rmaps_base_set_mapping_policy(orte_mapping_policy_t *policy, return ORTE_ERR_SILENT; } ptr++; // move past the colon - /* check the remaining string for modifiers - may be none, so - * don't emit an error message if the modifier isn't recognized - */ - if (ORTE_ERR_SILENT == (rc = check_modifiers(ptr, &tmp)) && - ORTE_ERR_BAD_PARAM != rc) { - free(spec); - return ORTE_ERR_SILENT; + /* at this point, ck is pointing to the number of procs/object + * and ptr is pointing to the beginning of the string that describes + * the object plus any modifiers. We first check to see if there + * is a comma indicating that there are modifiers to the request */ + if (NULL != (cptr = strchr(ptr, ','))) { + /* there are modifiers, so we terminate the object string + * at the location of the first comma */ + *cptr = '\0'; + /* step over that comma */ + cptr++; + /* now check for modifiers - may be none, so + * don't emit an error message if the modifier + * isn't recognized */ + if (ORTE_ERR_SILENT == (rc = check_modifiers(cptr, &tmp)) && + ORTE_ERR_BAD_PARAM != rc) { + free(spec); + return ORTE_ERR_SILENT; + } } - /* if we found something, then we need to adjust the string */ - if (ORTE_SUCCESS == rc) { - ptr--; - *ptr = '\0'; - } - /* now get the pattern */ + /* now save the pattern */ orte_rmaps_base.ppr = strdup(ck); ORTE_SET_MAPPING_POLICY(tmp, ORTE_MAPPING_PPR); ORTE_SET_MAPPING_DIRECTIVE(tmp, ORTE_MAPPING_GIVEN); diff --git a/orte/runtime/data_type_support/orte_dt_print_fns.c b/orte/runtime/data_type_support/orte_dt_print_fns.c index d017adc1cf..fbcfa69850 100644 --- a/orte/runtime/data_type_support/orte_dt_print_fns.c +++ b/orte/runtime/data_type_support/orte_dt_print_fns.c @@ -13,7 +13,7 @@ * Copyright (c) 2011-2015 Cisco Systems, Inc. All rights reserved. * Copyright (c) 2011-2013 Los Alamos National Security, LLC. * All rights reserved. - * Copyright (c) 2013-2017 Intel, Inc. All rights reserved. + * Copyright (c) 2013-2018 Intel, Inc. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -442,9 +442,12 @@ int orte_dt_print_node(char **output, char *prefix, orte_node_t *src, opal_data_ */ int orte_dt_print_proc(char **output, char *prefix, orte_proc_t *src, opal_data_type_t type) { - char *tmp, *tmp2, *pfx2; - hwloc_obj_t loc=NULL, bd=NULL; - char locale[1024], bind[1024]; + char *tmp, *tmp3, *pfx2; + hwloc_obj_t loc=NULL; + char locale[1024], tmp1[1024], tmp2[1024]; + hwloc_cpuset_t mycpus; + char *str=NULL, *cpu_bitmap=NULL; + /* set default result */ *output = NULL; @@ -470,10 +473,6 @@ int orte_dt_print_proc(char **output, char *prefix, orte_proc_t *src, opal_data_ } if (!orte_devel_level_output) { - hwloc_cpuset_t mycpus; - char tmp1[1024], tmp2[1024]; - char *str=NULL, *cpu_bitmap=NULL; - if (orte_get_attribute(&src->attributes, ORTE_PROC_CPU_BITMAP, (void**)&cpu_bitmap, OPAL_STRING) && NULL != src->node->topology && NULL != src->node->topology->topo) { mycpus = hwloc_bitmap_alloc(); @@ -509,10 +508,10 @@ int orte_dt_print_proc(char **output, char *prefix, orte_proc_t *src, opal_data_ asprintf(&tmp, "\n%sData for proc: %s", pfx2, ORTE_NAME_PRINT(&src->name)); - asprintf(&tmp2, "%s\n%s\tPid: %ld\tLocal rank: %lu\tNode rank: %lu\tApp rank: %d", tmp, pfx2, + asprintf(&tmp3, "%s\n%s\tPid: %ld\tLocal rank: %lu\tNode rank: %lu\tApp rank: %d", tmp, pfx2, (long)src->pid, (unsigned long)src->local_rank, (unsigned long)src->node_rank, src->app_rank); free(tmp); - tmp = tmp2; + tmp = tmp3; if (orte_get_attribute(&src->attributes, ORTE_PROC_HWLOC_LOCALE, (void**)&loc, OPAL_PTR)) { if (NULL != loc) { @@ -525,23 +524,26 @@ int orte_dt_print_proc(char **output, char *prefix, orte_proc_t *src, opal_data_ } else { strcpy(locale, "UNKNOWN"); } - if (orte_get_attribute(&src->attributes, ORTE_PROC_HWLOC_BOUND, (void**)&bd, OPAL_PTR)) { - if (NULL != bd) { - if (OPAL_ERR_NOT_BOUND == opal_hwloc_base_cset2mapstr(bind, sizeof(bind), src->node->topology->topo, bd->cpuset)) { - strcpy(bind, "UNBOUND"); - } - } else { - strcpy(bind, "UNBOUND"); - } + if (orte_get_attribute(&src->attributes, ORTE_PROC_CPU_BITMAP, (void**)&cpu_bitmap, OPAL_STRING) && + NULL != src->node->topology && NULL != src->node->topology->topo) { + mycpus = hwloc_bitmap_alloc(); + hwloc_bitmap_list_sscanf(mycpus, cpu_bitmap); + opal_hwloc_base_cset2mapstr(tmp2, sizeof(tmp2), src->node->topology->topo, mycpus); } else { - strcpy(bind, "UNBOUND"); + snprintf(tmp2, sizeof(tmp2), "UNBOUND"); } - asprintf(&tmp2, "%s\n%s\tState: %s\tApp_context: %ld\n%s\tLocale: %s\n%s\tBinding: %s", tmp, pfx2, - orte_proc_state_to_str(src->state), (long)src->app_idx, pfx2, locale, pfx2, bind); + asprintf(&tmp3, "%s\n%s\tState: %s\tApp_context: %ld\n%s\tLocale: %s\n%s\tBinding: %s", tmp, pfx2, + orte_proc_state_to_str(src->state), (long)src->app_idx, pfx2, locale, pfx2, tmp2); free(tmp); + if (NULL != str) { + free(str); + } + if (NULL != cpu_bitmap) { + free(cpu_bitmap); + } /* set the return */ - *output = tmp2; + *output = tmp3; free(pfx2); return ORTE_SUCCESS;