From 030a5f20544f7fe282daec69405062ed8186dc9d Mon Sep 17 00:00:00 2001 From: Gilles Gouaillardet Date: Sat, 6 Feb 2016 20:10:45 +0900 Subject: [PATCH 1/3] sentinel: use type uintptr_t for sentinel MSB is now automatically cleared when right shifting Thanks George for pointing this --- ompi/communicator/comm.c | 4 ++-- ompi/dpm/dpm.c | 4 ++-- ompi/group/group.h | 4 +++- ompi/group/group_plist.c | 4 +++- ompi/mca/pml/monitoring/pml_monitoring.c | 4 +++- ompi/proc/proc.c | 4 ++-- ompi/proc/proc.h | 9 ++++----- 7 files changed, 19 insertions(+), 14 deletions(-) diff --git a/ompi/communicator/comm.c b/ompi/communicator/comm.c index 87f0940c659..1f34701e037 100644 --- a/ompi/communicator/comm.c +++ b/ompi/communicator/comm.c @@ -18,7 +18,7 @@ * Copyright (c) 2012 Oak Ridge National Labs. All rights reserved. * Copyright (c) 2012-2015 Los Alamos National Security, LLC. * All rights reserved. - * Copyright (c) 2014 Research Organization for Information Science + * Copyright (c) 2014-2016 Research Organization for Information Science * and Technology (RIST). All rights reserved. * Copyright (c) 2014-2015 Intel, Inc. All rights reserved. * Copyright (c) 2015 Mellanox Technologies. All rights reserved. @@ -699,7 +699,7 @@ static int ompi_comm_split_type_get_part (ompi_group_t *group, int *results, int int include = false; if (ompi_proc_is_sentinel (proc)) { - opal_process_name_t proc_name = ompi_proc_sentinel_to_name ((intptr_t) proc); + opal_process_name_t proc_name = ompi_proc_sentinel_to_name ((uintptr_t) proc); u16ptr = &locality; diff --git a/ompi/dpm/dpm.c b/ompi/dpm/dpm.c index 3a2c3d9595a..d2a0c2adf23 100644 --- a/ompi/dpm/dpm.c +++ b/ompi/dpm/dpm.c @@ -169,7 +169,7 @@ int ompi_dpm_connect_accept(ompi_communicator_t *comm, int root, for (i=0; i < size; i++) { opal_process_name_t proc_name; if (ompi_proc_is_sentinel (proc_list[i])) { - proc_name = ompi_proc_sentinel_to_name ((intptr_t) proc_list[i]); + proc_name = ompi_proc_sentinel_to_name ((uintptr_t) proc_list[i]); } else { proc_name = proc_list[i]->super.proc_name; } @@ -532,7 +532,7 @@ static int construct_peers(ompi_group_t *group, opal_list_t *peers) return OMPI_ERR_NOT_FOUND; } if (ompi_proc_is_sentinel (proct)) { - proc_name = ompi_proc_sentinel_to_name ((intptr_t)proct); + proc_name = ompi_proc_sentinel_to_name ((uintptr_t)proct); } else { proc_name = proct->super.proc_name; } diff --git a/ompi/group/group.h b/ompi/group/group.h index 50852acb395..c4ff03b6847 100644 --- a/ompi/group/group.h +++ b/ompi/group/group.h @@ -16,6 +16,8 @@ * Copyright (c) 2012 Oak Ridge National Labs. All rights reserved. * Copyright (c) 2013-2015 Los Alamos National Security, LLC. All rights * reserved. + * Copyright (c) 2016 Research Organization for Information Science + * and Technology (RIST). All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -352,7 +354,7 @@ static inline struct ompi_proc_t *ompi_group_dense_lookup (ompi_group_t *group, /* replace sentinel value with an actual ompi_proc_t */ ompi_proc_t *real_proc = - (ompi_proc_t *) ompi_proc_for_name (ompi_proc_sentinel_to_name ((intptr_t) proc)); + (ompi_proc_t *) ompi_proc_for_name (ompi_proc_sentinel_to_name ((uintptr_t) proc)); if (opal_atomic_cmpset_ptr (group->grp_proc_pointers + peer_id, proc, real_proc)) { OBJ_RETAIN(real_proc); diff --git a/ompi/group/group_plist.c b/ompi/group/group_plist.c index bb3a271036a..62007154f3b 100644 --- a/ompi/group/group_plist.c +++ b/ompi/group/group_plist.c @@ -14,6 +14,8 @@ * Copyright (c) 2007 Cisco Systems, Inc. All rights reserved. * Copyright (c) 2013-2015 Los Alamos National Security, LLC. All rights * reserved. + * Copyright (c) 2016 Research Organization for Information Science + * and Technology (RIST). All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -61,7 +63,7 @@ static struct ompi_proc_t *ompi_group_dense_lookup_raw (ompi_group_t *group, con { if (OPAL_UNLIKELY(ompi_proc_is_sentinel (group->grp_proc_pointers[peer_id]))) { ompi_proc_t *proc = - (ompi_proc_t *) ompi_proc_lookup (ompi_proc_sentinel_to_name ((intptr_t) group->grp_proc_pointers[peer_id])); + (ompi_proc_t *) ompi_proc_lookup (ompi_proc_sentinel_to_name ((uintptr_t) group->grp_proc_pointers[peer_id])); if (NULL != proc) { /* replace sentinel value with an actual ompi_proc_t */ group->grp_proc_pointers[peer_id] = proc; diff --git a/ompi/mca/pml/monitoring/pml_monitoring.c b/ompi/mca/pml/monitoring/pml_monitoring.c index 6316f826ee0..cd848b5f4ce 100644 --- a/ompi/mca/pml/monitoring/pml_monitoring.c +++ b/ompi/mca/pml/monitoring/pml_monitoring.c @@ -4,6 +4,8 @@ * reserved. * Copyright (c) 2013-2015 Inria. All rights reserved. * Copyright (c) 2015 Bull SAS. All rights reserved. + * Copyright (c) 2016 Research Organization for Information Science + * and Technology (RIST). All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -77,7 +79,7 @@ int mca_pml_monitoring_add_procs(struct ompi_proc_t **procs, my_rank = i; /* Extract the peer procname from the procs array */ if( ompi_proc_is_sentinel(procs[i]) ) { - tmp = ompi_proc_sentinel_to_name((intptr_t)procs[i]); + tmp = ompi_proc_sentinel_to_name((uintptr_t)procs[i]); } else { tmp = procs[i]->super.proc_name; } diff --git a/ompi/proc/proc.c b/ompi/proc/proc.c index 3f710dcdf8a..8d00a8ca194 100644 --- a/ompi/proc/proc.c +++ b/ompi/proc/proc.c @@ -14,7 +14,7 @@ * Copyright (c) 2012-2015 Los Alamos National Security, LLC. All rights * reserved. * Copyright (c) 2013-2015 Intel, Inc. All rights reserved - * Copyright (c) 2014-2015 Research Organization for Information Science + * Copyright (c) 2014-2016 Research Organization for Information Science * and Technology (RIST). All rights reserved. * Copyright (c) 2015 Mellanox Technologies. All rights reserved. * @@ -642,7 +642,7 @@ ompi_proc_pack(ompi_proc_t **proclist, int proclistsize, ompi_proc_t *proc = proclist[i]; if (ompi_proc_is_sentinel (proc)) { - proc = ompi_proc_for_name_nolock (ompi_proc_sentinel_to_name ((intptr_t) proc)); + proc = ompi_proc_for_name_nolock (ompi_proc_sentinel_to_name ((uintptr_t) proc)); } /* send proc name */ diff --git a/ompi/proc/proc.h b/ompi/proc/proc.h index 6228458832e..e03999666ff 100644 --- a/ompi/proc/proc.h +++ b/ompi/proc/proc.h @@ -13,7 +13,7 @@ * Copyright (c) 2007-2012 Los Alamos National Security, LLC. All rights * reserved. * Copyright (c) 2013-2014 Intel, Inc. All rights reserved - * Copyright (c) 2015 Research Organization for Information Science + * Copyright (c) 2015-2016 Research Organization for Information Science * and Technology (RIST). All rights reserved. * $COPYRIGHT$ * @@ -369,15 +369,14 @@ static inline bool ompi_proc_is_sentinel (ompi_proc_t *proc) return (intptr_t) proc & 0x1; } -static inline intptr_t ompi_proc_name_to_sentinel (opal_process_name_t name) +static inline uintptr_t ompi_proc_name_to_sentinel (opal_process_name_t name) { - return (*((intptr_t *) &name) << 1) | 0x1; + return (*((uintptr_t *) &name) << 1) | 0x1; } -static inline opal_process_name_t ompi_proc_sentinel_to_name (intptr_t sentinel) +static inline opal_process_name_t ompi_proc_sentinel_to_name (uintptr_t sentinel) { sentinel >>= 1; - sentinel &= 0x7FFFFFFFFFFFFFFF; return *((opal_process_name_t *) &sentinel); } From b55b9e6aeee96fe76768b0438fad11ffb4e49fe4 Mon Sep 17 00:00:00 2001 From: Gilles Gouaillardet Date: Sat, 6 Feb 2016 20:23:08 +0900 Subject: [PATCH 2/3] sentinel: fix sentinel to proc_name conversion converting an opal_process_name_t means the loss of one bit, it was decided to restrict the local job id to 15 bits, so the useful information of an opal_process_name_t can fit in 63 bits. --- ompi/mca/rte/orte/rte_orte.h | 6 +++++- ompi/proc/proc.h | 34 +++++++++++++++++++++++++++++++--- orte/util/name_fns.h | 3 +++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/ompi/mca/rte/orte/rte_orte.h b/ompi/mca/rte/orte/rte_orte.h index 0a4efaa7c0f..b71a6e8323a 100644 --- a/ompi/mca/rte/orte/rte_orte.h +++ b/ompi/mca/rte/orte/rte_orte.h @@ -3,7 +3,7 @@ * All rights reserved. * Copyright (c) 2013-2015 Intel, Inc. All rights reserved * Copyright (c) 2014 Cisco Systems, Inc. All rights reserved. - * Copyright (c) 2014 Research Organization for Information Science + * Copyright (c) 2014-2016 Research Organization for Information Science * and Technology (RIST). All rights reserved. * Copyright (c) 2015 Intel, Inc. All rights reserved. * $COPYRIGHT$ @@ -59,6 +59,10 @@ typedef orte_ns_cmp_bitmask_t ompi_rte_cmp_bitmask_t; #define OMPI_RTE_CMP_JOBID ORTE_NS_CMP_JOBID #define OMPI_RTE_CMP_VPID ORTE_NS_CMP_VPID #define OMPI_RTE_CMP_ALL ORTE_NS_CMP_ALL +#define OMPI_LOCAL_JOBID(jobid) ORTE_LOCAL_JOBID(jobid) +#define OMPI_JOB_FAMILY(jobid) ORTE_JOB_FAMILY(jobid) +#define OMPI_CONSTRUCT_JOBID(family,local) ORTE_CONSTRUCT_JOBID(family,local) + /* This is the DSS tag to serialize a proc name */ #define OMPI_NAME ORTE_NAME #define OMPI_PROCESS_NAME_HTON ORTE_PROCESS_NAME_HTON diff --git a/ompi/proc/proc.h b/ompi/proc/proc.h index e03999666ff..eac16cf51f0 100644 --- a/ompi/proc/proc.h +++ b/ompi/proc/proc.h @@ -369,15 +369,43 @@ static inline bool ompi_proc_is_sentinel (ompi_proc_t *proc) return (intptr_t) proc & 0x1; } +/* + * we assume an ompi_proc_t is at least aligned on two bytes, + * so if the LSB of a pointer to an ompi_proc_t is 1, we have to handle + * this pointer as a sentinel instead of a pointer. + * a sentinel can be seen as an uint64_t with the following format : + * - bit 0 : 1 + * - bits 1-15 : local jobid + * - bits 16-31 : job family + * - bits 32-63 : vpid + */ static inline uintptr_t ompi_proc_name_to_sentinel (opal_process_name_t name) { - return (*((uintptr_t *) &name) << 1) | 0x1; + uintptr_t tmp, sentinel = 0; + /* local jobid must fit in 15 bits */ + assert(! (OMPI_LOCAL_JOBID(name.jobid) & 0x8000)); + sentinel |= 0x1; + tmp = (uintptr_t)OMPI_LOCAL_JOBID(name.jobid); + sentinel |= ((tmp << 1) & 0xfffe); + tmp = (uintptr_t)OMPI_JOB_FAMILY(name.jobid); + sentinel |= ((tmp << 16) & 0xffff0000); + tmp = (uintptr_t)name.vpid; + sentinel |= ((tmp << 32) & 0xffffffff00000000); + return sentinel; } static inline opal_process_name_t ompi_proc_sentinel_to_name (uintptr_t sentinel) { - sentinel >>= 1; - return *((opal_process_name_t *) &sentinel); + opal_process_name_t name; + uint32_t local, family; + uint32_t vpid; + assert(sentinel & 0x1); + local = (sentinel >> 1) & 0x7fff; + family = (sentinel >> 16) & 0xffff; + vpid = (sentinel >> 32) & 0xffffffff; + name.jobid = OMPI_CONSTRUCT_JOBID(family,local); + name.vpid = vpid; + return name; } END_C_DECLS diff --git a/orte/util/name_fns.h b/orte/util/name_fns.h index 3003aa9b3d0..ecf3ca1d6af 100644 --- a/orte/util/name_fns.h +++ b/orte/util/name_fns.h @@ -97,6 +97,9 @@ ORTE_DECLSPEC char *orte_pretty_print_timing(int64_t secs, int64_t usecs); #define ORTE_CONSTRUCT_LOCAL_JOBID(local, job) \ ( ((local) & 0xffff0000) | ((job) & 0x0000ffff) ) +#define ORTE_CONSTRUCT_JOBID(family, local) \ + ORTE_CONSTRUCT_LOCAL_JOBID(ORTE_CONSTRUCT_JOB_FAMILY(family), local) + /* a macro for identifying that a proc is a daemon */ #define ORTE_JOBID_IS_DAEMON(n) \ !((n) & 0x0000ffff) From 96310f439bf99d057b4082162d45a6344e152e91 Mon Sep 17 00:00:00 2001 From: Gilles Gouaillardet Date: Mon, 8 Feb 2016 10:48:26 +0900 Subject: [PATCH 3/3] sentinel: fix 32 bits arch since a sentinel is only made from the current job, only store the first 31 bits of the vpid into the sentinel. --- ompi/proc/proc.h | 22 ++++++++++++++++++++++ opal/dss/dss_types.h | 3 ++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/ompi/proc/proc.h b/ompi/proc/proc.h index eac16cf51f0..62a720b92d7 100644 --- a/ompi/proc/proc.h +++ b/ompi/proc/proc.h @@ -369,6 +369,7 @@ static inline bool ompi_proc_is_sentinel (ompi_proc_t *proc) return (intptr_t) proc & 0x1; } +#if OPAL_SIZEOF_PROCESS_NAME_T == SIZEOF_VOID_P /* * we assume an ompi_proc_t is at least aligned on two bytes, * so if the LSB of a pointer to an ompi_proc_t is 1, we have to handle @@ -407,6 +408,27 @@ static inline opal_process_name_t ompi_proc_sentinel_to_name (uintptr_t sentinel name.vpid = vpid; return name; } +#elif 4 == SIZEOF_VOID_P +/* + * currently, a sentinel is only made from the current jobid aka OMPI_PROC_MY_NAME->jobid + * so we only store the first 31 bits of the vpid + */ +static inline uintptr_t ompi_proc_name_to_sentinel (opal_process_name_t name) +{ + assert(OMPI_PROC_MY_NAME->jobid == name.jobid); + return (uintptr_t)((name.vpid <<1) | 0x1); +} + +static inline opal_process_name_t ompi_proc_sentinel_to_name (uintptr_t sentinel) +{ + opal_process_name_t name; + name.jobid = OMPI_PROC_MY_NAME->jobid; + name.vpid = sentinel >> 1; + return name; +} +#else +#error unsupported pointer size +#endif END_C_DECLS diff --git a/opal/dss/dss_types.h b/opal/dss/dss_types.h index 8c1bad91efd..c2612231f35 100644 --- a/opal/dss/dss_types.h +++ b/opal/dss/dss_types.h @@ -14,7 +14,7 @@ * Copyright (c) 2012-2013 Los Alamos National Security, Inc. All rights * reserved. * Copyright (c) 2014 Intel, Inc. All rights reserved. - * Copyright (c) 2014 Research Organization for Information Science + * Copyright (c) 2014-2016 Research Organization for Information Science * and Technology (RIST). All rights reserved. * $COPYRIGHT$ * @@ -47,6 +47,7 @@ typedef struct { opal_jobid_t jobid; opal_vpid_t vpid; } opal_process_name_t; +#define OPAL_SIZEOF_PROCESS_NAME_T 8 BEGIN_C_DECLS