From cbe2c673bc6959af8db50a52058545369bd246b0 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Mon, 21 Mar 2016 15:20:56 -0600 Subject: [PATCH 1/7] mca: add support for flag enumerators This commit adds a new type of enumerator meant to support flag values. The enumerator parses comma-delimited strings and matches each string or value to a list of valid flags. Additionally, the enumerator does some basic checks to see if 1) a flag is valid in the enumerator, and 2) if any conflicting flags are specified. Signed-off-by: Nathan Hjelm (cherry picked from commit b15a45088c7e75aa10e04e85fe0f530b45b3ad5b) --- ompi/info/info.c | 6 +- opal/mca/base/mca_base_var.c | 12 +- opal/mca/base/mca_base_var_enum.c | 249 +++++++++++++++++- opal/mca/base/mca_base_var_enum.h | 78 +++++- opal/mca/btl/base/base.h | 3 + .../mpool/memkind/mpool_memkind_component.c | 5 +- 6 files changed, 320 insertions(+), 33 deletions(-) diff --git a/ompi/info/info.c b/ompi/info/info.c index fec7ad268f6..8f56311edfb 100644 --- a/ompi/info/info.c +++ b/ompi/info/info.c @@ -275,7 +275,7 @@ int ompi_info_set (ompi_info_t *info, const char *key, const char *value) int ompi_info_set_value_enum (ompi_info_t *info, const char *key, int value, mca_base_var_enum_t *var_enum) { - const char *string_value; + char *string_value; int ret; ret = var_enum->string_from_value (var_enum, value, &string_value); @@ -283,7 +283,9 @@ int ompi_info_set_value_enum (ompi_info_t *info, const char *key, int value, return ret; } - return ompi_info_set (info, key, string_value); + ret = ompi_info_set (info, key, string_value); + free (string_value); + return ret; } diff --git a/opal/mca/base/mca_base_var.c b/opal/mca/base/mca_base_var.c index e60c5b05681..5817efc6871 100644 --- a/opal/mca/base/mca_base_var.c +++ b/opal/mca/base/mca_base_var.c @@ -1526,7 +1526,7 @@ int mca_base_var_register (const char *project_name, const char *framework_name, mca_base_var_scope_t scope, void *storage) { /* Only integer variables can have enumerator */ - assert (NULL == enumerator || MCA_BASE_VAR_TYPE_INT == type); + assert (NULL == enumerator || (MCA_BASE_VAR_TYPE_INT == type || MCA_BASE_VAR_TYPE_UNSIGNED_INT == type)); return register_variable (project_name, framework_name, component_name, variable_name, description, type, enumerator, @@ -1927,7 +1927,6 @@ static char *source_name(mca_base_var_t *var) static int var_value_string (mca_base_var_t *var, char **value_string) { const mca_base_var_storage_t *value; - const char *tmp; int ret; assert (MCA_BASE_VAR_TYPE_MAX > var->mbv_type); @@ -1974,19 +1973,14 @@ static int var_value_string (mca_base_var_t *var, char **value_string) } else { /* we use an enumerator to handle string->bool and bool->string conversion */ if (MCA_BASE_VAR_TYPE_BOOL == var->mbv_type) { - ret = var->mbv_enumerator->string_from_value(var->mbv_enumerator, value->boolval, &tmp); + ret = var->mbv_enumerator->string_from_value(var->mbv_enumerator, value->boolval, value_string); } else { - ret = var->mbv_enumerator->string_from_value(var->mbv_enumerator, value->intval, &tmp); + ret = var->mbv_enumerator->string_from_value(var->mbv_enumerator, value->intval, value_string); } if (OPAL_SUCCESS != ret) { return ret; } - - *value_string = strdup (tmp); - if (NULL == *value_string) { - ret = OPAL_ERR_OUT_OF_RESOURCE; - } } return ret; diff --git a/opal/mca/base/mca_base_var_enum.c b/opal/mca/base/mca_base_var_enum.c index 2e74cc6fca2..8e0599c3f74 100644 --- a/opal/mca/base/mca_base_var_enum.c +++ b/opal/mca/base/mca_base_var_enum.c @@ -24,6 +24,7 @@ #include "opal/mca/base/mca_base_var_enum.h" #include "opal/mca/base/base.h" +#include "opal/util/argv.h" #include #include @@ -34,6 +35,11 @@ static void mca_base_var_enum_destructor (mca_base_var_enum_t *enumerator); OBJ_CLASS_INSTANCE(mca_base_var_enum_t, opal_object_t, mca_base_var_enum_constructor, mca_base_var_enum_destructor); +static void mca_base_var_enum_flag_constructor (mca_base_var_enum_flag_t *enumerator); +static void mca_base_var_enum_flag_destructor (mca_base_var_enum_flag_t *enumerator); +OBJ_CLASS_INSTANCE(mca_base_var_enum_flag_t, opal_object_t, mca_base_var_enum_flag_constructor, + mca_base_var_enum_flag_destructor); + static int enum_dump (mca_base_var_enum_t *self, char **out); static int enum_get_count (mca_base_var_enum_t *self, int *count); static int enum_get_value (mca_base_var_enum_t *self, int index, int *value, const char **string_value); @@ -85,10 +91,10 @@ static int mca_base_var_enum_bool_vfs (mca_base_var_enum_t *self, const char *st } static int mca_base_var_enum_bool_sfv (mca_base_var_enum_t *self, const int value, - const char **string_value) + char **string_value) { if (string_value) { - *string_value = value ? "true" : "false"; + *string_value = strdup (value ? "true" : "false"); } return OPAL_SUCCESS; @@ -155,9 +161,9 @@ static int mca_base_var_enum_verbose_vfs (mca_base_var_enum_t *self, const char } static int mca_base_var_enum_verbose_sfv (mca_base_var_enum_t *self, const int value, - const char **string_value) + char **string_value) { - static char buffer[4]; + int ret; if (value < 0 || value > 100) { return OPAL_ERR_VALUE_OUT_OF_BOUNDS; @@ -165,14 +171,16 @@ static int mca_base_var_enum_verbose_sfv (mca_base_var_enum_t *self, const int v for (int i = 0 ; verbose_values[i].string ; ++i) { if (verbose_values[i].value == value) { - *string_value = verbose_values[i].string; + *string_value = strdup (verbose_values[i].string); return OPAL_SUCCESS; } } - snprintf (buffer, 4, "%d", value); if (string_value) { - *string_value = buffer; + ret = asprintf (string_value, "%d", value); + if (0 > ret) { + return OPAL_ERR_OUT_OF_RESOURCE; + } } return OPAL_SUCCESS; @@ -251,6 +259,52 @@ int mca_base_var_enum_create (const char *name, const mca_base_var_enum_value_t return OPAL_SUCCESS; } +int mca_base_var_enum_create_flag (const char *name, const mca_base_var_enum_value_flag_t *flags, mca_base_var_enum_flag_t **enumerator) +{ + mca_base_var_enum_flag_t *new_enum; + int i; + + *enumerator = NULL; + + new_enum = OBJ_NEW(mca_base_var_enum_flag_t); + if (NULL == new_enum) { + return OPAL_ERR_OUT_OF_RESOURCE; + } + + new_enum->super.enum_name = strdup (name); + if (NULL == new_enum->super.enum_name) { + return OPAL_ERR_OUT_OF_RESOURCE; + } + + for (i = 0 ; flags[i].string ; ++i); + new_enum->super.enum_value_count = i; + + /* make a copy of the values */ + new_enum->enum_flags = calloc (new_enum->super.enum_value_count + 1, sizeof (*new_enum->enum_flags)); + if (NULL == new_enum->enum_flags) { + OBJ_RELEASE(new_enum); + return OPAL_ERR_OUT_OF_RESOURCE; + } + + int all_flags = 0; + for (i = 0 ; i < new_enum->super.enum_value_count ; ++i) { + new_enum->enum_flags[i].flag = flags[i].flag; + new_enum->enum_flags[i].string = strdup (flags[i].string); + new_enum->enum_flags[i].conflicting_flag = flags[i].conflicting_flag; + /* ensure flags are only set a single bit, doesn't conflict with itself, and + * hasn't already been specified. */ + assert (!(flags[i].flag & (flags[i].flag - 1))); + assert (!(flags[i].flag & flags[i].conflicting_flag)); + assert (!(all_flags & flags[i].flag)); + assert (flags[i].flag); + all_flags |= flags[i].flag; + } + + *enumerator = new_enum; + + return OPAL_SUCCESS; +} + static int enum_dump (mca_base_var_enum_t *self, char **out) { int i; @@ -301,7 +355,7 @@ static int enum_get_value (mca_base_var_enum_t *self, int index, int *value, con } if (string_value) { - *string_value = self->enum_values[index].string; + *string_value = strdup (self->enum_values[index].string); } return OPAL_SUCCESS; @@ -338,7 +392,7 @@ static int enum_value_from_string(mca_base_var_enum_t *self, const char *string_ return OPAL_SUCCESS; } -static int enum_string_from_value(mca_base_var_enum_t *self, const int value, const char **string_value) { +static int enum_string_from_value(mca_base_var_enum_t *self, const int value, char **string_value) { int count, ret, i; ret = self->get_count(self, &count); @@ -357,7 +411,7 @@ static int enum_string_from_value(mca_base_var_enum_t *self, const int value, co } if (string_value) { - *string_value = self->enum_values[i].string; + *string_value = strdup (self->enum_values[i].string); } return OPAL_SUCCESS; @@ -389,3 +443,178 @@ static void mca_base_var_enum_destructor (mca_base_var_enum_t *enumerator) free (enumerator->enum_values); } } + +static int enum_get_value_flag (mca_base_var_enum_t *self, int index, int *value, const char **string_value) +{ + mca_base_var_enum_flag_t *flag_enum = (mca_base_var_enum_flag_t *) self; + int count, ret; + + ret = self->get_count(self, &count); + if (OPAL_SUCCESS != ret) { + return ret; + } + + if (index >= count) { + return OPAL_ERR_VALUE_OUT_OF_BOUNDS; + } + + if (value) { + *value = flag_enum->enum_flags[index].flag; + } + + if (string_value) { + *string_value = strdup (flag_enum->enum_flags[index].string); + } + + return OPAL_SUCCESS; +} + +static int enum_value_from_string_flag (mca_base_var_enum_t *self, const char *string_value, int *value_out) { + mca_base_var_enum_flag_t *flag_enum = (mca_base_var_enum_flag_t *) self; + int value, count, ret, flag; + char **flags; + bool is_int; + char *tmp; + + ret = self->get_count(self, &count); + if (OPAL_SUCCESS != ret) { + return ret; + } + + flags = opal_argv_split (string_value, ','); + if (NULL == flags) { + return OPAL_ERR_BAD_PARAM; + } + + flag = 0; + + for (int i = 0 ; flags[i] ; ++i) { + value = strtol (flags[i], &tmp, 0); + is_int = tmp[0] == '\0'; + + bool found = false, conflict = false; + for (int j = 0 ; j < count ; ++j) { + if ((is_int && (value == flag_enum->enum_flags[i].flag)) || + 0 == strcasecmp (flags[i], flag_enum->enum_flags[i].string)) { + found = true; + + if (flag & flag_enum->enum_flags[i].conflicting_flag) { + conflict = true; + } else { + flag |= flag_enum->enum_flags[i].flag; + } + + break; + } + } + + if (!found || conflict) { + opal_argv_free (flags); + return !found ? OPAL_ERR_VALUE_OUT_OF_BOUNDS : OPAL_ERR_BAD_PARAM; + } + } + + *value_out = flag; + + return OPAL_SUCCESS; +} + +static int enum_string_from_value_flag (mca_base_var_enum_t *self, const int value, char **string_value) { + mca_base_var_enum_flag_t *flag_enum = (mca_base_var_enum_flag_t *) self; + int count, ret, current; + char *out = NULL, *tmp; + + ret = self->get_count(self, &count); + if (OPAL_SUCCESS != ret) { + return ret; + } + + current = value; + for (int i = 0 ; i < count ; ++i) { + if (!(flag_enum->enum_flags[i].flag & current)) { + continue; + } + + tmp = out; + + ret = asprintf (&out, "%s%s%s", tmp ? tmp : "", tmp ? "," : "", flag_enum->enum_flags[i].string); + free (tmp); + + if (0 > ret) { + return OPAL_ERR_OUT_OF_RESOURCE; + } + + if (value & flag_enum->enum_flags[i].conflicting_flag) { + free (out); + return OPAL_ERR_BAD_PARAM; + } + + current &= ~flag_enum->enum_flags[i].flag; + } + + if (current) { + free (out); + return OPAL_ERR_VALUE_OUT_OF_BOUNDS; + } + + if (string_value) { + *string_value = out ? out : strdup (""); + } else { + free (out); + } + + return OPAL_SUCCESS; +} + +static int enum_dump_flag (mca_base_var_enum_t *self, char **out) +{ + mca_base_var_enum_flag_t *flag_enum = (mca_base_var_enum_flag_t *) self; + char *tmp; + int ret; + + *out = NULL; + + if (NULL == self) { + return OPAL_ERROR; + } + + *out = strdup ("Comma-delimited list of: "); + if (NULL == *out) { + return OPAL_ERR_OUT_OF_RESOURCE; + } + + for (int i = 0; i < self->enum_value_count ; ++i) { + tmp = *out; + + ret = asprintf (out, "%s%s0x%x:\"%s\"", tmp, i ? ", " : " ", flag_enum->enum_flags[i].flag, + flag_enum->enum_flags[i].string); + free (tmp); + if (0 > ret) { + return OPAL_ERR_OUT_OF_RESOURCE; + } + } + + return OPAL_SUCCESS; +} + +static void mca_base_var_enum_flag_constructor (mca_base_var_enum_flag_t *enumerator) +{ + enumerator->enum_flags = NULL; + enumerator->super.get_value = enum_get_value_flag; + enumerator->super.get_count = enum_get_count; + enumerator->super.value_from_string = enum_value_from_string_flag; + enumerator->super.string_from_value = enum_string_from_value_flag; + enumerator->super.dump = enum_dump_flag; + enumerator->super.enum_is_static = false; +} + +static void mca_base_var_enum_flag_destructor (mca_base_var_enum_flag_t *enumerator) +{ + /* release the copy of the values */ + if (enumerator->enum_flags) { + for (int i = 0 ; i < enumerator->super.enum_value_count ; ++i) { + free ((void *) enumerator->enum_flags[i].string); + } + free (enumerator->enum_flags); + } +} diff --git a/opal/mca/base/mca_base_var_enum.h b/opal/mca/base/mca_base_var_enum.h index 67286424e64..a7fc90379bf 100644 --- a/opal/mca/base/mca_base_var_enum.h +++ b/opal/mca/base/mca_base_var_enum.h @@ -11,7 +11,7 @@ * Copyright (c) 2004-2005 The Regents of the University of California. * All rights reserved. * Copyright (c) 2008-2011 Cisco Systems, Inc. All rights reserved. - * Copyright (c) 2012-2015 Los Alamos National Security, LLC. All rights + * Copyright (c) 2012-2016 Los Alamos National Security, LLC. All rights * reserved. * $COPYRIGHT$ * @@ -85,10 +85,11 @@ typedef int (*mca_base_var_enum_dump_fn_t)(mca_base_var_enum_t *self, char **out * * @long This function returns the string value for a given interger value in the * {string_value} parameter. The {string_value} parameter may be NULL in which case - * no string is returned. + * no string is returned. If a string is returned in {string_value} the caller + * must free the string with free(). */ typedef int (*mca_base_var_enum_sfv_fn_t)(mca_base_var_enum_t *self, const int value, - const char **string_value); + char **string_value); /** * The default enumerator class takes in a list of integer-string pairs. If a @@ -102,7 +103,9 @@ struct mca_base_var_enum_value_t { typedef struct mca_base_var_enum_value_t mca_base_var_enum_value_t; -/* enumerator base class */ +/** + * enumerator base class + */ struct mca_base_var_enum_t { opal_object_t super; @@ -135,6 +138,36 @@ struct mca_base_var_enum_t { mca_base_var_enum_value_t *enum_values; }; + +/** + * The default flag enumerator class takes in a list of integer-string pairs. If a + * string is read from an environment variable or a file value the matching + * flag value is used for the MCA variable. The conflicting_flag is used to + * indicate any flags that should conflict. + */ +struct mca_base_var_enum_value_flag_t { + /** flag value (must be power-of-two) */ + int flag; + /** corresponding string name */ + const char *string; + /** conflicting flag(s) if any */ + int conflicting_flag; +}; + +typedef struct mca_base_var_enum_value_flag_t mca_base_var_enum_value_flag_t; + +/** + * flag enumerator base class + */ +struct mca_base_var_enum_flag_t { + /** use the existing enumerator interface */ + mca_base_var_enum_t super; + /** flag value(s) */ + mca_base_var_enum_value_flag_t *enum_flags; +}; + +typedef struct mca_base_var_enum_flag_t mca_base_var_enum_flag_t; + /** * Object declaration for mca_base_var_enum_t */ @@ -152,14 +185,12 @@ OPAL_DECLSPEC OBJ_CLASS_DECLARATION(mca_base_var_enum_t); * @retval opal error code On error * * This function creates a value enumerator for integer variables. The - * value array is stored by reference in the enumerator so it should - * not be allocated on the stack. The OUT enumerator value will be a - * newly OBJ_NEW'ed object that should be released by the caller via - * OBJ_RELEASE. + * OUT enumerator value will be a newly OBJ_NEW'ed object that should + * be released by the caller via OBJ_RELEASE. * * Note that the output enumerator can be OBJ_RELEASE'd after it has - * been used in a pvar registration, because variables that use the - * enumerator will OBJ_RETAIN it. + * been used in a cvar or pvar registration, because the variable + * registration functions will OBJ_RETAIN the enumberator. * * Note that all the strings in the values[] array are strdup'ed into * internal storage, meaning that the caller can free all of the @@ -169,6 +200,33 @@ OPAL_DECLSPEC OBJ_CLASS_DECLARATION(mca_base_var_enum_t); OPAL_DECLSPEC int mca_base_var_enum_create (const char *name, const mca_base_var_enum_value_t values[], mca_base_var_enum_t **enumerator); +/** + * Create a new default flag enumerator + * + * @param[in] name Name for this enumerator + * @param[in] flags List of flags terminated with a NULL .string + * member. + * @param[out] enumerator Newly created enumerator. + * + * @retval OPAL_SUCCESS On success + * @retval opal error code On error + * + * This function creates a flag enumerator for integer variables. The + * OUT enumerator value will be a newly OBJ_NEW'ed object that should + * be released by the caller via OBJ_RELEASE. + * + * Note that the output enumerator can be OBJ_RELEASE'd after it has + * been used in a cvar or pvar registration, because the variable + * registration functions will OBJ_RETAIN the enumberator. + * + * Note that all the strings in the values[] array are strdup'ed into + * internal storage, meaning that the caller can free all of the + * strings passed in values[] after mca_base_var_enum_create() + * returns. + */ +OPAL_DECLSPEC int mca_base_var_enum_create_flag (const char *name, const mca_base_var_enum_value_flag_t flags[], + mca_base_var_enum_flag_t **enumerator); + /* standard enumerators. it is invalid to call OBJ_RELEASE on any of these enumerators */ /** * Boolean enumerator diff --git a/opal/mca/btl/base/base.h b/opal/mca/btl/base/base.h index 0731a73f47a..5a759795d87 100644 --- a/opal/mca/btl/base/base.h +++ b/opal/mca/btl/base/base.h @@ -78,6 +78,9 @@ OPAL_DECLSPEC extern bool mca_btl_base_thread_multiple_override; OPAL_DECLSPEC extern mca_base_framework_t opal_btl_base_framework; +extern mca_base_var_enum_flag_t *mca_btl_base_flag_enum; +extern mca_base_var_enum_flag_t *mca_btl_base_atomic_enum; + END_C_DECLS #endif /* MCA_BTL_BASE_H */ diff --git a/opal/mca/mpool/memkind/mpool_memkind_component.c b/opal/mca/mpool/memkind/mpool_memkind_component.c index b3b01928ebf..9ac5690f6f7 100644 --- a/opal/mca/mpool/memkind/mpool_memkind_component.c +++ b/opal/mca/mpool/memkind/mpool_memkind_component.c @@ -12,7 +12,7 @@ * All rights reserved. * Copyright (c) 2007-2009 Sun Microsystems, Inc. All rights reserved. * Copyright (c) 2008-2009 Cisco Systems, Inc. All rights reserved. - * Copyright (c) 2010-2015 Los Alamos National Security, LLC. All rights + * Copyright (c) 2010-2016 Los Alamos National Security, LLC. All rights * reserved. * Copyright (c) 2014 NVIDIA Corporation. All rights reserved. * $COPYRIGHT$ @@ -148,13 +148,14 @@ static int mca_mpool_memkind_open (void) } if (memkind_check_available (default_kind)) { - const char *kind_string; + char *kind_string; mca_mpool_memkind_enum->string_from_value (mca_mpool_memkind_enum, mca_mpool_memkind_component.default_partition, &kind_string); opal_output_verbose (MCA_BASE_VERBOSE_WARN, mca_mpool_memkind_component.output, "default kind %s not available", kind_string); + free (kind_string); return OPAL_ERR_NOT_AVAILABLE; } From b8283e6bdf8e951ec4ecfae3b4461710938b5e74 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Wed, 30 Mar 2016 12:49:37 -0600 Subject: [PATCH 2/7] mca/base: fix coverity issue Fixes CID 1357979: Resource leak (RESOURCE_LEAK): flags array was being leaked. Fixed. Signed-off-by: Nathan Hjelm (cherry picked from commit 6df5a663b7bdd52c09b227b7d41cfa2255d6d334) --- opal/mca/base/mca_base_var_enum.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/opal/mca/base/mca_base_var_enum.c b/opal/mca/base/mca_base_var_enum.c index 8e0599c3f74..b261c50115a 100644 --- a/opal/mca/base/mca_base_var_enum.c +++ b/opal/mca/base/mca_base_var_enum.c @@ -514,6 +514,8 @@ static int enum_value_from_string_flag (mca_base_var_enum_t *self, const char *s } } + opal_argv_free (flags); + *value_out = flag; return OPAL_SUCCESS; From c94fd279b7abf537ec1f3e04ad27ad8607ae8343 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Mon, 23 May 2016 12:21:34 -0600 Subject: [PATCH 3/7] mca/base: fix typo in flag enumeration This commit fixes a typo in flag enumeration that can cause the parser to miss valid flags or crash. Signed-off-by: Nathan Hjelm (cherry picked from commit 37e9e2c660d949748f034bc0b4fcaade1913e355) --- opal/mca/base/mca_base_var.c | 2 +- opal/mca/base/mca_base_var_enum.c | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/opal/mca/base/mca_base_var.c b/opal/mca/base/mca_base_var.c index 5817efc6871..1757134234a 100644 --- a/opal/mca/base/mca_base_var.c +++ b/opal/mca/base/mca_base_var.c @@ -724,7 +724,7 @@ static int var_set_from_string (mca_base_var_t *var, char *src) case MCA_BASE_VAR_TYPE_BOOL: case MCA_BASE_VAR_TYPE_SIZE_T: ret = int_from_string(src, var->mbv_enumerator, &int_value); - if (OPAL_ERR_VALUE_OUT_OF_BOUNDS == ret || + if (OPAL_SUCCESS != ret || (MCA_BASE_VAR_TYPE_INT == var->mbv_type && ((int) int_value != (int64_t) int_value)) || (MCA_BASE_VAR_TYPE_UNSIGNED_INT == var->mbv_type && ((unsigned int) int_value != int_value))) { if (var->mbv_enumerator) { diff --git a/opal/mca/base/mca_base_var_enum.c b/opal/mca/base/mca_base_var_enum.c index b261c50115a..5f0d80ccbf2 100644 --- a/opal/mca/base/mca_base_var_enum.c +++ b/opal/mca/base/mca_base_var_enum.c @@ -494,21 +494,28 @@ static int enum_value_from_string_flag (mca_base_var_enum_t *self, const char *s bool found = false, conflict = false; for (int j = 0 ; j < count ; ++j) { - if ((is_int && (value == flag_enum->enum_flags[i].flag)) || - 0 == strcasecmp (flags[i], flag_enum->enum_flags[i].string)) { + if ((is_int && (value & flag_enum->enum_flags[j].flag)) || + 0 == strcasecmp (flags[i], flag_enum->enum_flags[j].string)) { found = true; - if (flag & flag_enum->enum_flags[i].conflicting_flag) { + if (flag & flag_enum->enum_flags[j].conflicting_flag) { conflict = true; } else { - flag |= flag_enum->enum_flags[i].flag; + flag |= flag_enum->enum_flags[j].flag; } - break; + if (is_int) { + value &= ~flag_enum->enum_flags[j].flag; + if (0 == value) { + break; + } + } else { + break; + } } } - if (!found || conflict) { + if (!found || conflict || (is_int && value)) { opal_argv_free (flags); return !found ? OPAL_ERR_VALUE_OUT_OF_BOUNDS : OPAL_ERR_BAD_PARAM; } From cd63bbf007ea50814b2b8a515968aa999321e5a1 Mon Sep 17 00:00:00 2001 From: Gilles Gouaillardet Date: Mon, 28 Mar 2016 14:57:31 +0900 Subject: [PATCH 4/7] win: silence a warning in alloc_window(...) (cherry picked from commit 1baed498b679bb5d2a6387ea083998defb26cc51) --- ompi/win/win.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ompi/win/win.c b/ompi/win/win.c index 8d2821b1f0f..7e48d11ec91 100644 --- a/ompi/win/win.c +++ b/ompi/win/win.c @@ -139,7 +139,7 @@ static int alloc_window(struct ompi_communicator_t *comm, ompi_info_t *info, int return ret; } - win->w_acc_ops = acc_ops; + win->w_acc_ops = (ompi_win_accumulate_ops_t)acc_ops; win->w_flavor = flavor; /* setup data that is independent of osc component */ From e32e65396c276e78a111a55d5c97b166b3ad1c17 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Tue, 24 May 2016 11:13:30 -0600 Subject: [PATCH 5/7] win: add support for accumulate_ordering info key This commit adds support for the MPI-3.1 accumulate_ordering info key. The default value is rar,war,raw,waw and is supported using an MCA variable flag enumerator. Signed-off-by: Nathan Hjelm (cherry picked from commit 5126da5377b3caa76571e0c7be6e1763a53e50f9) --- ompi/win/win.c | 31 ++++++++++++++++++++++++++++++- ompi/win/win.h | 21 +++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/ompi/win/win.c b/ompi/win/win.c index 7e48d11ec91..000ec9e8f29 100644 --- a/ompi/win/win.c +++ b/ompi/win/win.c @@ -46,6 +46,7 @@ opal_pointer_array_t ompi_mpi_windows = {{0}}; ompi_predefined_win_t ompi_mpi_win_null = {{{0}}}; ompi_predefined_win_t *ompi_mpi_win_null_addr = &ompi_mpi_win_null; mca_base_var_enum_t *ompi_win_accumulate_ops = NULL; +mca_base_var_enum_t *ompi_win_accumulate_order = NULL; static mca_base_var_enum_value_t accumulate_ops_values[] = { {.value = OMPI_WIN_ACCUMULATE_OPS_SAME_OP_NO_OP, .string = "same_op_no_op",}, @@ -53,6 +54,16 @@ static mca_base_var_enum_value_t accumulate_ops_values[] = { {.value = -1, .string = NULL}, }; +static mca_base_var_enum_value_flag_t accumulate_order_flags[] = { + {.flag = OMPI_WIN_ACC_ORDER_NONE, .string = "none", .conflicting_flag = OMPI_WIN_ACC_ORDER_RAR | + OMPI_WIN_ACC_ORDER_WAR | OMPI_WIN_ACC_ORDER_RAW | OMPI_WIN_ACC_ORDER_WAW}, + {.flag = OMPI_WIN_ACC_ORDER_RAR, .string = "rar", .conflicting_flag = OMPI_WIN_ACC_ORDER_NONE}, + {.flag = OMPI_WIN_ACC_ORDER_WAR, .string = "war", .conflicting_flag = OMPI_WIN_ACC_ORDER_NONE}, + {.flag = OMPI_WIN_ACC_ORDER_RAW, .string = "raw", .conflicting_flag = OMPI_WIN_ACC_ORDER_NONE}, + {.flag = OMPI_WIN_ACC_ORDER_WAW, .string = "waw", .conflicting_flag = OMPI_WIN_ACC_ORDER_NONE}, + {}, +}; + static void ompi_win_construct(ompi_win_t *win); static void ompi_win_destruct(ompi_win_t *win); @@ -86,6 +97,11 @@ ompi_win_init(void) return ret; } + ret = mca_base_var_enum_create_flag ("accumulate_order", accumulate_order_flags, &ompi_win_accumulate_order); + if (OPAL_SUCCESS != ret) { + return ret; + } + return OMPI_SUCCESS; } @@ -115,6 +131,7 @@ int ompi_win_finalize(void) OBJ_DESTRUCT(&ompi_mpi_win_null.win); OBJ_DESTRUCT(&ompi_mpi_windows); OBJ_RELEASE(ompi_win_accumulate_ops); + OBJ_RELEASE(ompi_win_accumulate_order); return OMPI_SUCCESS; } @@ -123,7 +140,7 @@ static int alloc_window(struct ompi_communicator_t *comm, ompi_info_t *info, int { ompi_win_t *win; ompi_group_t *group; - int acc_ops, flag, ret; + int acc_ops, acc_order, flag, ret; /* create the object */ win = OBJ_NEW(ompi_win_t); @@ -140,6 +157,18 @@ static int alloc_window(struct ompi_communicator_t *comm, ompi_info_t *info, int } win->w_acc_ops = (ompi_win_accumulate_ops_t)acc_ops; + + ret = ompi_info_get_value_enum (info, "accumulate_order", &acc_order, + OMPI_WIN_ACC_ORDER_RAR | OMPI_WIN_ACC_ORDER_WAR | + OMPI_WIN_ACC_ORDER_RAW | OMPI_WIN_ACC_ORDER_WAW, + ompi_win_accumulate_order, &flag); + if (OMPI_SUCCESS != ret) { + OBJ_RELEASE(win); + return ret; + } + + win->w_acc_order = acc_order; + win->w_flavor = flavor; /* setup data that is independent of osc component */ diff --git a/ompi/win/win.h b/ompi/win/win.h index 0cfc33d530d..07daab04295 100644 --- a/ompi/win/win.h +++ b/ompi/win/win.h @@ -50,7 +50,25 @@ enum ompi_win_accumulate_ops_t { }; typedef enum ompi_win_accumulate_ops_t ompi_win_accumulate_ops_t; +/** + * Accumulate ordering flags. The default accumulate ordering in + * MPI-3.1 is rar,war,raw,waw. + */ +enum ompi_win_accumulate_order_flags_t { + /** no accumulate ordering (may valid with any other flag) */ + OMPI_WIN_ACC_ORDER_NONE = 0x01, + /** read-after-read ordering */ + OMPI_WIN_ACC_ORDER_RAR = 0x02, + /** write-after-read ordering */ + OMPI_WIN_ACC_ORDER_WAR = 0x04, + /** read-after-write ordering */ + OMPI_WIN_ACC_ORDER_RAW = 0x08, + /** write-after-write ordering */ + OMPI_WIN_ACC_ORDER_WAW = 0x10, +}; + OMPI_DECLSPEC extern mca_base_var_enum_t *ompi_win_accumulate_ops; +OMPI_DECLSPEC extern mca_base_var_enum_t *ompi_win_accumulate_order; OMPI_DECLSPEC extern opal_pointer_array_t ompi_mpi_windows; @@ -87,6 +105,9 @@ struct ompi_win_t { /* one sided interface */ ompi_osc_base_module_t *w_osc_module; + + /** Accumulate ordering (see ompi_win_accumulate_order_flags_t above) */ + int32_t w_acc_order; }; typedef struct ompi_win_t ompi_win_t; OMPI_DECLSPEC OBJ_CLASS_DECLARATION(ompi_win_t); From 64e22527cb8b17ed54eb653efc02d127d4cc16e6 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Mon, 21 Mar 2016 15:23:37 -0600 Subject: [PATCH 6/7] btl: use flag enumerator for btl_*_flags and btl_*_atomic_flags This commit uses the new flag "enumerator" to support comma-delimited lists of flags for both the btl and btl atomic flags. After this commit is is valid to specify something like -mca btl_foo_flags self,put,get,in-place. All non-deprecated flags are supported by the enumerator. Signed-off-by: Nathan Hjelm (cherry picked from commit 7572c8b74f0ce9af7db31d8b349bdbbc6a7f29e1) --- opal/mca/btl/base/btl_base_frame.c | 47 ++++++++++++++++++++++++++++++ opal/mca/btl/base/btl_base_mca.c | 37 +++++------------------ 2 files changed, 55 insertions(+), 29 deletions(-) diff --git a/opal/mca/btl/base/btl_base_frame.c b/opal/mca/btl/base/btl_base_frame.c index 038f8f93b85..6cb49e5f49c 100644 --- a/opal/mca/btl/base/btl_base_frame.c +++ b/opal/mca/btl/base/btl_base_frame.c @@ -1,3 +1,4 @@ +/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */ /* * Copyright (c) 2004-2005 The Trustees of Indiana University and Indiana * University Research and Technology @@ -13,6 +14,8 @@ * Copyright (c) 2008-2013 Cisco Systems, Inc. All rights reserved. * Copyright (c) 2015 Research Organization for Information Science * and Technology (RIST). All rights reserved. + * Copyright (c) 2016 Los Alamos National Security, LLC. All rights + * reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -30,6 +33,39 @@ #include "opal/mca/btl/btl.h" #include "opal/mca/btl/base/base.h" +mca_base_var_enum_flag_t *mca_btl_base_flag_enum = NULL; +mca_base_var_enum_flag_t *mca_btl_base_atomic_enum = NULL; + +mca_base_var_enum_value_flag_t mca_btl_base_flag_enum_flags[] = { + {MCA_BTL_FLAGS_SEND, "send", 0}, + {MCA_BTL_FLAGS_PUT, "put", 0}, + {MCA_BTL_FLAGS_GET, "get", 0}, + {MCA_BTL_FLAGS_SEND_INPLACE, "inplace", 0}, + {MCA_BTL_FLAGS_SIGNALED, "signaled", 0}, + {MCA_BTL_FLAGS_ATOMIC_OPS, "atomics", 0}, + {MCA_BTL_FLAGS_ATOMIC_FOPS, "fetching-atomics", 0}, + {MCA_BTL_FLAGS_SINGLE_ADD_PROCS, "static", 0}, + {MCA_BTL_FLAGS_CUDA_PUT, "cuda-put", 0}, + {MCA_BTL_FLAGS_CUDA_GET, "cuda-get", 0}, + {MCA_BTL_FLAGS_CUDA_COPY_ASYNC_SEND, "cuda-async-send", 0}, + {MCA_BTL_FLAGS_CUDA_COPY_ASYNC_RECV, "cuda-async-recv", 0}, + {MCA_BTL_FLAGS_FAILOVER_SUPPORT, "failover", 0}, + {MCA_BTL_FLAGS_NEED_ACK, "need-ack", 0}, + {MCA_BTL_FLAGS_NEED_CSUM, "need-csum", 0}, + {MCA_BTL_FLAGS_HETEROGENEOUS_RDMA, "hetero-rdma", 0}, + {0, NULL, 0} +}; + +mca_base_var_enum_value_flag_t mca_btl_base_atomic_enum_flags[] = { + {MCA_BTL_ATOMIC_SUPPORTS_ADD, "add", 0}, + {MCA_BTL_ATOMIC_SUPPORTS_AND, "and", 0}, + {MCA_BTL_ATOMIC_SUPPORTS_OR, "or", 0}, + {MCA_BTL_ATOMIC_SUPPORTS_XOR, "xor", 0}, + {MCA_BTL_ATOMIC_SUPPORTS_CSWAP, "compare-and-swap", 0}, + {MCA_BTL_ATOMIC_SUPPORTS_GLOB, "global"}, + {0, NULL, 0} +}; + mca_btl_active_message_callback_t mca_btl_base_active_message_trigger[MCA_BTL_TAG_MAX] = {{0}}; /* @@ -104,6 +140,9 @@ static int mca_btl_base_register(mca_base_register_flag_t flags) MCA_BASE_VAR_SCOPE_READONLY, &mca_btl_base_warn_component_unused); + (void) mca_base_var_enum_create_flag ("btl_flags", mca_btl_base_flag_enum_flags, &mca_btl_base_flag_enum); + (void) mca_base_var_enum_create_flag ("btl_atomic_flags", mca_btl_base_atomic_enum_flags, &mca_btl_base_atomic_enum); + return OPAL_SUCCESS; } @@ -159,6 +198,14 @@ static int mca_btl_base_close(void) OBJ_DESTRUCT(&mca_btl_base_modules_initialized); + if (mca_btl_base_flag_enum) { + OBJ_RELEASE(mca_btl_base_flag_enum); + } + + if (mca_btl_base_atomic_enum) { + OBJ_RELEASE(mca_btl_base_atomic_enum); + } + #if 0 /* restore event processing */ opal_event_enable(); diff --git a/opal/mca/btl/base/btl_base_mca.c b/opal/mca/btl/base/btl_base_mca.c index 70de4ed45f2..7988096f957 100644 --- a/opal/mca/btl/base/btl_base_mca.c +++ b/opal/mca/btl/base/btl_base_mca.c @@ -14,7 +14,7 @@ * Copyright (c) 2007 Cisco Systems, Inc. All rights reserved. * Copyright (c) 2010 Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2013 NVIDIA Corporation. All rights reserved. - * Copyright (c) 2015 Los Alamos National Security, LLC. All rights + * Copyright (c) 2016 Los Alamos National Security, LLC. All rights * reserved. * * $COPYRIGHT$ @@ -37,8 +37,6 @@ int mca_btl_base_param_register(mca_base_component_t *version, mca_btl_base_module_t *module) { - char *msg; - /* If this is ever triggered change the uint32_ts in mca_btl_base_module_t to unsigned ints */ assert(sizeof(unsigned int) == sizeof(uint32_t)); @@ -49,33 +47,14 @@ int mca_btl_base_param_register(mca_base_component_t *version, MCA_BASE_VAR_SCOPE_READONLY, &module->btl_exclusivity); - asprintf(&msg, "BTL bit flags (general flags: SEND=%d, PUT=%d, GET=%d, SEND_INPLACE=%d, HETEROGENEOUS_RDMA=%d, " - "ATOMIC_OPS=%d; flags only used by the \"dr\" PML (ignored by others): ACK=%d, CHECKSUM=%d, " - "RDMA_COMPLETION=%d; flags only used by the \"bfo\" PML (ignored by others): FAILOVER_SUPPORT=%d)", - MCA_BTL_FLAGS_SEND, - MCA_BTL_FLAGS_PUT, - MCA_BTL_FLAGS_GET, - MCA_BTL_FLAGS_SEND_INPLACE, - MCA_BTL_FLAGS_HETEROGENEOUS_RDMA, - MCA_BTL_FLAGS_ATOMIC_OPS, - MCA_BTL_FLAGS_NEED_ACK, - MCA_BTL_FLAGS_NEED_CSUM, - MCA_BTL_FLAGS_RDMA_COMPLETION, - MCA_BTL_FLAGS_FAILOVER_SUPPORT); - (void) mca_base_component_var_register(version, "flags", msg, - MCA_BASE_VAR_TYPE_UNSIGNED_INT, NULL, 0, 0, - OPAL_INFO_LVL_5, - MCA_BASE_VAR_SCOPE_READONLY, - &module->btl_flags); - free(msg); - - asprintf (&msg, "BTL atomic bit flags (general flags: ADD=%d, AND=%d, OR=%d, XOR=%d", - MCA_BTL_ATOMIC_SUPPORTS_ADD, MCA_BTL_ATOMIC_SUPPORTS_AND, MCA_BTL_ATOMIC_SUPPORTS_OR, - MCA_BTL_ATOMIC_SUPPORTS_XOR); - (void) mca_base_component_var_register(version, "atomic_flags", msg, MCA_BASE_VAR_TYPE_UNSIGNED_INT, - NULL, 0, MCA_BASE_VAR_FLAG_DEFAULT_ONLY, OPAL_INFO_LVL_5, + (void) mca_base_component_var_register(version, "flags", "BTL bit flags (general flags: send, put, get, in-place, hetero-rdma, " + "atomics, fetching-atomics)", MCA_BASE_VAR_TYPE_UNSIGNED_INT, + &mca_btl_base_flag_enum->super, 0, 0, OPAL_INFO_LVL_5, + MCA_BASE_VAR_SCOPE_READONLY, &module->btl_flags); + + (void) mca_base_component_var_register(version, "atomic_flags", "BTL atomic support flags", MCA_BASE_VAR_TYPE_UNSIGNED_INT, + &mca_btl_base_atomic_enum->super, 0, MCA_BASE_VAR_FLAG_DEFAULT_ONLY, OPAL_INFO_LVL_5, MCA_BASE_VAR_SCOPE_CONSTANT, &module->btl_atomic_flags); - free(msg); (void) mca_base_component_var_register(version, "rndv_eager_limit", "Size (in bytes, including header) of \"phase 1\" fragment sent for all large messages (must be >= 0 and <= eager_limit)", MCA_BASE_VAR_TYPE_SIZE_T, NULL, 0, 0, From 8dc867c14341a7db82acdba68c7c07cc7ca1eb91 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Sun, 2 Oct 2016 22:04:49 -0600 Subject: [PATCH 7/7] btl/base: add missing include Signed-off-by: Nathan Hjelm --- opal/mca/btl/base/base.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/opal/mca/btl/base/base.h b/opal/mca/btl/base/base.h index 5a759795d87..c66f0c81bbd 100644 --- a/opal/mca/btl/base/base.h +++ b/opal/mca/btl/base/base.h @@ -1,3 +1,4 @@ +/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */ /* * Copyright (c) 2004-2005 The Trustees of Indiana University and Indiana * University Research and Technology @@ -11,7 +12,7 @@ * All rights reserved. * Copyright (c) 2006 Sun Microsystems, Inc. All rights reserved. * Copyright (c) 2008 Cisco Systems, Inc. All rights reserved. - * Copyright (c) 2015 Los Alamos National Security, LLC. All rights + * Copyright (c) 2015-2016 Los Alamos National Security, LLC. All rights * reserved. * $COPYRIGHT$ * @@ -30,6 +31,7 @@ #include "opal/mca/mca.h" #include "opal/mca/base/mca_base_framework.h" #include "opal/mca/btl/btl.h" +#include "opal/mca/base/mca_base_var_enum.h" BEGIN_C_DECLS