Skip to content

Commit 2b5f6c3

Browse files
markallemarkalle
committed
fixes for Dave's get/set info code
The expected sequence of events for processing info during object creation is that if there's an incoming info arg, it is opal_info_dup()ed into the obj at obj->s_info first. Then interested components register callbacks for keys they want to know about using opal_infosubscribe_infosubscribe(). Inside info_subscribe_subscribe() the specified callback() is called with whatever matching k/v is in the object's info, or with the default. The return string from the callback goes into the new k/v stored in info, and the input k/v is saved as __IN_<key>/<val>. It's saved the same way whether the input came from info or whether it was a default. A null return from the callback indicates an ignored key/val, and no k/v is stored for it, but an __IN_<key>/<val> is still kept so we still have access to the original. At MPI_*_set_info() time, opal_infosubscribe_change_info() is used. That function calls the registered callbacks for each item in the provided info. If the callback returns non-null, the info is updated with that k/v, or if the callback returns null, that key is deleted from info. An __IN_<key>/<val> is saved either way, and overwrites any previously saved value. When MPI_*_get_info() is called, opal_info_dup_mpistandard() is used, which allows relatively easy changes in interpretation of the standard, by looking at both the <key>/<val> and __IN_<key>/<val> in info. Right now it does 1. includes system extras, eg k/v defaults not expliclty set by the user 2. omits ignored keys 3. shows input values, not callback modifications, eg not the internal values Currently the callbacks are doing things like return some_condition ? "true" : "false" that is, returning static strings that are not to be freed. If the return strings start becoming more dynamic in the future I don't see how unallocated strings could support that, so I'd propose a change for the future that the callback()s registered with info_subscribe_subscribe() do a strdup on their return, and we change the callers of callback() to free the strings it returns (there are only two callers). Rough outline of the smaller changes spread over the less central files: comm.c initialize comm->super.s_info to NULL copy into comm->super.s_info in comm creation calls that provide info OBJ_RELEASE comm->super.s_info at free time comm_init.c initialize comm->super.s_info to NULL file.c copy into file->super.s_info if file creation provides info OBJ_RELEASE file->super.s_info at free time win.c copy into win->super.s_info if win creation provides info OBJ_RELEASE win->super.s_info at free time comm_get_info.c file_get_info.c win_get_info.c change_info() if there's no info attached (shouldn't happen if callbacks are registered) copy the info for the user The last category of change is generally addressing compiler warnings where ompi_info_t and opal_info_t were being used a little too interchangably. An ompi_info_t* contains an opal_info_t*, at &(ompi_info->super)
1 parent 996b406 commit 2b5f6c3

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+711
-157
lines changed

‎AUTHORS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,8 @@ Mangala Jyothi Bhaskar, University of Houston
198198
[email protected]
199199
Manjunath Gorentla Venkata, Oak Ridge National Laboratory
200200
[email protected]
201+
Mark Allen, IBM
202+
[email protected]
201203
Mark Santcroos, Rutgers University
202204
[email protected]
203205
[email protected]

‎ompi/communicator/comm.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ int ompi_comm_set_nb ( ompi_communicator_t **ncomm,
158158

159159
/* ompi_comm_allocate */
160160
newcomm = OBJ_NEW(ompi_communicator_t);
161+
newcomm->super.s_info = NULL;
161162
/* fill in the inscribing hyper-cube dimensions */
162163
newcomm->c_cube_dim = opal_cube_dim(local_size);
163164
newcomm->c_id_available = MPI_UNDEFINED;
@@ -918,6 +919,12 @@ int ompi_comm_split_type (ompi_communicator_t *comm, int split_type, int key,
918919
break;
919920
}
920921

922+
// Copy info if there is one.
923+
newcomp->super.s_info = OBJ_NEW(opal_info_t);
924+
if (info) {
925+
opal_info_dup(info, &(newcomp->super.s_info));
926+
}
927+
921928
/* Activate the communicator and init coll-component */
922929
rc = ompi_comm_activate (&newcomp, comm, NULL, NULL, NULL, false, mode);
923930
if (OPAL_UNLIKELY(OMPI_SUCCESS != rc)) {
@@ -1015,6 +1022,12 @@ int ompi_comm_dup_with_info ( ompi_communicator_t * comm, opal_info_t *info, omp
10151022
snprintf(newcomp->c_name, MPI_MAX_OBJECT_NAME, "MPI COMMUNICATOR %d DUP FROM %d",
10161023
newcomp->c_contextid, comm->c_contextid );
10171024

1025+
// Copy info if there is one.
1026+
newcomp->super.s_info = OBJ_NEW(opal_info_t);
1027+
if (info) {
1028+
opal_info_dup(info, &(newcomp->super.s_info));
1029+
}
1030+
10181031
/* activate communicator and init coll-module */
10191032
rc = ompi_comm_activate (&newcomp, comm, NULL, NULL, NULL, false, mode);
10201033
if ( OMPI_SUCCESS != rc ) {
@@ -1095,6 +1108,15 @@ static int ompi_comm_idup_internal (ompi_communicator_t *comm, ompi_group_t *gro
10951108
return rc;
10961109
}
10971110

1111+
// Copy info if there is one.
1112+
{
1113+
ompi_communicator_t *newcomp = context->newcomp;
1114+
newcomp->super.s_info = OBJ_NEW(opal_info_t);
1115+
if (info) {
1116+
opal_info_dup(info, &(newcomp->super.s_info));
1117+
}
1118+
}
1119+
10981120
ompi_comm_request_schedule_append (request, ompi_comm_idup_getcid, subreq, subreq[0] ? 1 : 0);
10991121

11001122
/* assign the newcomm now */
@@ -1472,6 +1494,10 @@ int ompi_comm_free( ompi_communicator_t **comm )
14721494
ompi_mpi_comm_parent = &ompi_mpi_comm_null.comm;
14731495
}
14741496

1497+
if (NULL != ((*comm)->super.s_info)) {
1498+
OBJ_RELEASE((*comm)->super.s_info);
1499+
}
1500+
14751501
/* Release the communicator */
14761502
if ( OMPI_COMM_IS_DYNAMIC (*comm) ) {
14771503
ompi_comm_num_dyncomm --;

‎ompi/communicator/comm_init.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ ompi_communicator_t *ompi_comm_allocate ( int local_size, int remote_size )
221221

222222
/* create new communicator element */
223223
new_comm = OBJ_NEW(ompi_communicator_t);
224+
new_comm->super.s_info = NULL;
224225
new_comm->c_local_group = ompi_group_allocate ( local_size );
225226
if ( 0 < remote_size ) {
226227
new_comm->c_remote_group = ompi_group_allocate (remote_size);

‎ompi/dpm/dpm.c

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
704704
if ( array_of_info != NULL && array_of_info[i] != MPI_INFO_NULL ) {
705705

706706
/* check for personality - this is a job-level key */
707-
opal_info_get (array_of_info[i], "personality", sizeof(host) - 1, host, &flag);
707+
ompi_info_get (array_of_info[i], "personality", sizeof(host) - 1, host, &flag);
708708
if ( flag ) {
709709
personality = true;
710710
info = OBJ_NEW(opal_value_t);
@@ -714,7 +714,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
714714
}
715715

716716
/* check for 'host' */
717-
opal_info_get (array_of_info[i], "host", sizeof(host) - 1, host, &flag);
717+
ompi_info_get (array_of_info[i], "host", sizeof(host) - 1, host, &flag);
718718
if ( flag ) {
719719
info = OBJ_NEW(opal_value_t);
720720
info->key = strdup(OPAL_PMIX_HOST);
@@ -723,7 +723,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
723723
}
724724

725725
/* check for 'hostfile' */
726-
opal_info_get (array_of_info[i], "hostfile", sizeof(host) - 1, host, &flag);
726+
ompi_info_get (array_of_info[i], "hostfile", sizeof(host) - 1, host, &flag);
727727
if ( flag ) {
728728
info = OBJ_NEW(opal_value_t);
729729
info->key = strdup(OPAL_PMIX_HOSTFILE);
@@ -732,7 +732,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
732732
}
733733

734734
/* check for 'add-hostfile' */
735-
opal_info_get (array_of_info[i], "add-hostfile", sizeof(host) - 1, host, &flag);
735+
ompi_info_get (array_of_info[i], "add-hostfile", sizeof(host) - 1, host, &flag);
736736
if ( flag ) {
737737
info = OBJ_NEW(opal_value_t);
738738
info->key = strdup(OPAL_PMIX_ADD_HOSTFILE);
@@ -741,7 +741,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
741741
}
742742

743743
/* check for 'add-host' */
744-
opal_info_get (array_of_info[i], "add-host", sizeof(host) - 1, host, &flag);
744+
ompi_info_get (array_of_info[i], "add-host", sizeof(host) - 1, host, &flag);
745745
if ( flag ) {
746746
info = OBJ_NEW(opal_value_t);
747747
info->key = strdup(OPAL_PMIX_ADD_HOST);
@@ -750,7 +750,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
750750
}
751751

752752
/* check for env */
753-
opal_info_get (array_of_info[i], "env", sizeof(host)-1, host, &flag);
753+
ompi_info_get (array_of_info[i], "env", sizeof(host)-1, host, &flag);
754754
if ( flag ) {
755755
envars = opal_argv_split(host, '\n');
756756
for (j=0; NULL != envars[j]; j++) {
@@ -766,7 +766,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
766766
*
767767
* This is a job-level key
768768
*/
769-
opal_info_get (array_of_info[i], "ompi_prefix", sizeof(prefix) - 1, prefix, &flag);
769+
ompi_info_get (array_of_info[i], "ompi_prefix", sizeof(prefix) - 1, prefix, &flag);
770770
if ( flag ) {
771771
info = OBJ_NEW(opal_value_t);
772772
info->key = strdup(OPAL_PMIX_PREFIX);
@@ -775,7 +775,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
775775
}
776776

777777
/* check for 'wdir' */
778-
opal_info_get (array_of_info[i], "wdir", sizeof(cwd) - 1, cwd, &flag);
778+
ompi_info_get (array_of_info[i], "wdir", sizeof(cwd) - 1, cwd, &flag);
779779
if ( flag ) {
780780
info = OBJ_NEW(opal_value_t);
781781
info->key = strdup(OPAL_PMIX_WDIR);
@@ -785,7 +785,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
785785
}
786786

787787
/* check for 'mapper' - a job-level key */
788-
opal_info_get(array_of_info[i], "mapper", sizeof(mapper) - 1, mapper, &flag);
788+
ompi_info_get(array_of_info[i], "mapper", sizeof(mapper) - 1, mapper, &flag);
789789
if ( flag ) {
790790
info = OBJ_NEW(opal_value_t);
791791
info->key = strdup(OPAL_PMIX_MAPPER);
@@ -794,7 +794,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
794794
}
795795

796796
/* check for 'display_map' - a job-level key */
797-
opal_info_get_bool(array_of_info[i], "display_map", &local_spawn, &flag);
797+
ompi_info_get_bool(array_of_info[i], "display_map", &local_spawn, &flag);
798798
if ( flag ) {
799799
info = OBJ_NEW(opal_value_t);
800800
info->key = strdup(OPAL_PMIX_DISPLAY_MAP);
@@ -803,22 +803,22 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
803803
}
804804

805805
/* check for 'npernode' and 'ppr' - job-level key */
806-
opal_info_get (array_of_info[i], "npernode", sizeof(slot_list) - 1, slot_list, &flag);
806+
ompi_info_get (array_of_info[i], "npernode", sizeof(slot_list) - 1, slot_list, &flag);
807807
if ( flag ) {
808808
info = OBJ_NEW(opal_value_t);
809809
info->key = strdup(OPAL_PMIX_PPR);
810810
info->type = OPAL_STRING;
811811
(void)asprintf(&(info->data.string), "%s:n", slot_list);
812812
opal_list_append(&job_info, &info->super);
813813
}
814-
opal_info_get (array_of_info[i], "pernode", sizeof(slot_list) - 1, slot_list, &flag);
814+
ompi_info_get (array_of_info[i], "pernode", sizeof(slot_list) - 1, slot_list, &flag);
815815
if ( flag ) {
816816
info = OBJ_NEW(opal_value_t);
817817
info->key = strdup(OPAL_PMIX_PPR);
818818
opal_value_load(info, "1:n", OPAL_STRING);
819819
opal_list_append(&job_info, &info->super);
820820
}
821-
opal_info_get (array_of_info[i], "ppr", sizeof(slot_list) - 1, slot_list, &flag);
821+
ompi_info_get (array_of_info[i], "ppr", sizeof(slot_list) - 1, slot_list, &flag);
822822
if ( flag ) {
823823
info = OBJ_NEW(opal_value_t);
824824
info->key = strdup(OPAL_PMIX_PPR);
@@ -827,7 +827,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
827827
}
828828

829829
/* check for 'map_by' - job-level key */
830-
opal_info_get(array_of_info[i], "map_by", sizeof(slot_list) - 1, slot_list, &flag);
830+
ompi_info_get(array_of_info[i], "map_by", sizeof(slot_list) - 1, slot_list, &flag);
831831
if ( flag ) {
832832
info = OBJ_NEW(opal_value_t);
833833
info->key = strdup(OPAL_PMIX_MAPBY);
@@ -836,7 +836,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
836836
}
837837

838838
/* check for 'rank_by' - job-level key */
839-
opal_info_get(array_of_info[i], "rank_by", sizeof(slot_list) - 1, slot_list, &flag);
839+
ompi_info_get(array_of_info[i], "rank_by", sizeof(slot_list) - 1, slot_list, &flag);
840840
if ( flag ) {
841841
info = OBJ_NEW(opal_value_t);
842842
info->key = strdup(OPAL_PMIX_RANKBY);
@@ -845,7 +845,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
845845
}
846846

847847
/* check for 'bind_to' - job-level key */
848-
opal_info_get(array_of_info[i], "bind_to", sizeof(slot_list) - 1, slot_list, &flag);
848+
ompi_info_get(array_of_info[i], "bind_to", sizeof(slot_list) - 1, slot_list, &flag);
849849
if ( flag ) {
850850
info = OBJ_NEW(opal_value_t);
851851
info->key = strdup(OPAL_PMIX_BINDTO);
@@ -854,7 +854,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
854854
}
855855

856856
/* check for 'preload_binary' - job-level key */
857-
opal_info_get_bool(array_of_info[i], "ompi_preload_binary", &local_spawn, &flag);
857+
ompi_info_get_bool(array_of_info[i], "ompi_preload_binary", &local_spawn, &flag);
858858
if ( flag ) {
859859
info = OBJ_NEW(opal_value_t);
860860
info->key = strdup(OPAL_PMIX_PRELOAD_BIN);
@@ -863,7 +863,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
863863
}
864864

865865
/* check for 'preload_files' - job-level key */
866-
opal_info_get (array_of_info[i], "ompi_preload_files", sizeof(cwd) - 1, cwd, &flag);
866+
ompi_info_get (array_of_info[i], "ompi_preload_files", sizeof(cwd) - 1, cwd, &flag);
867867
if ( flag ) {
868868
info = OBJ_NEW(opal_value_t);
869869
info->key = strdup(OPAL_PMIX_PRELOAD_FILES);
@@ -874,7 +874,7 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
874874
/* see if this is a non-mpi job - if so, then set the flag so ORTE
875875
* knows what to do - job-level key
876876
*/
877-
opal_info_get_bool(array_of_info[i], "ompi_non_mpi", &non_mpi, &flag);
877+
ompi_info_get_bool(array_of_info[i], "ompi_non_mpi", &non_mpi, &flag);
878878
if (flag && non_mpi) {
879879
info = OBJ_NEW(opal_value_t);
880880
info->key = strdup(OPAL_PMIX_NON_PMI);
@@ -883,15 +883,15 @@ int ompi_dpm_spawn(int count, const char *array_of_commands[],
883883
}
884884

885885
/* see if this is an MCA param that the user wants applied to the child job */
886-
opal_info_get (array_of_info[i], "ompi_param", sizeof(params) - 1, params, &flag);
886+
ompi_info_get (array_of_info[i], "ompi_param", sizeof(params) - 1, params, &flag);
887887
if ( flag ) {
888888
opal_argv_append_unique_nosize(&app->env, params, true);
889889
}
890890

891891
/* see if user specified what to do with stdin - defaults to
892892
* not forwarding stdin to child processes - job-level key
893893
*/
894-
opal_info_get (array_of_info[i], "ompi_stdin_target", sizeof(stdin_target) - 1, stdin_target, &flag);
894+
ompi_info_get (array_of_info[i], "ompi_stdin_target", sizeof(stdin_target) - 1, stdin_target, &flag);
895895
if ( flag ) {
896896
if (0 == strcmp(stdin_target, "all")) {
897897
ui32 = ORTE_VPID_WILDCARD;

‎ompi/file/file.c

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,10 @@ int ompi_file_open(struct ompi_communicator_t *comm, const char *filename,
114114
file->f_comm = comm;
115115
OBJ_RETAIN(comm);
116116

117-
/* Present the info to the info layer */
118-
119-
if (OPAL_SUCCESS != opal_infosubscribe_change_info(&file->super, info)) {
120-
OBJ_RELEASE(file);
121-
return ret;
117+
/* Copy the info for the info layer */
118+
file->super.s_info = OBJ_NEW(opal_info_t);
119+
if (info) {
120+
opal_info_dup(info, &(file->super.s_info));
122121
}
123122

124123
file->f_amode = amode;
@@ -310,6 +309,13 @@ static void file_destructor(ompi_file_t *file)
310309
#endif
311310
}
312311

312+
if (NULL != file->super.s_info) {
313+
OBJ_RELEASE(file->super.s_info);
314+
#if OPAL_ENABLE_DEBUG
315+
file->super.s_info = NULL;
316+
#endif
317+
}
318+
313319
/* Reset the f_to_c table entry */
314320

315321
if (MPI_UNDEFINED != file->f_f_to_c_index &&

‎ompi/info/info.c

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,57 @@ int ompi_mpiinfo_init(void)
196196
return OMPI_SUCCESS;
197197
}
198198

199+
// Generally ompi_info_t processing is handled by opal_info_t now.
200+
// But to avoid compiler warnings and to avoid having to constantly
201+
// change code to mpiinfo->super to make MPI code use the opal_info_t
202+
// it's convenient to have ompi_info_t wrappers for some of the opal_info_t
203+
// related calls:
204+
205+
int ompi_info_dup (ompi_info_t *info, ompi_info_t **newinfo) {
206+
return opal_info_dup (&(info->super), (opal_info_t **)newinfo);
207+
}
208+
int ompi_info_dup_mpistandard (ompi_info_t *info, ompi_info_t **newinfo) {
209+
return opal_info_dup_mpistandard (&(info->super), (opal_info_t **)newinfo);
210+
}
211+
int ompi_info_set (ompi_info_t *info, const char *key, const char *value) {
212+
return opal_info_set (&(info->super), key, value);
213+
}
214+
int ompi_info_set_value_enum (ompi_info_t *info, const char *key, int value,
215+
mca_base_var_enum_t *var_enum)
216+
{
217+
return opal_info_set_value_enum (&(info->super), key, value, var_enum);
218+
}
219+
int ompi_info_get (ompi_info_t *info, const char *key, int valuelen,
220+
char *value, int *flag)
221+
{
222+
return opal_info_get (&(info->super), key, valuelen, value, flag);
223+
}
224+
int ompi_info_get_value_enum (ompi_info_t *info, const char *key, int *value,
225+
int default_value, mca_base_var_enum_t *var_enum,
226+
int *flag)
227+
{
228+
return opal_info_get_value_enum (&(info->super), key, value,
229+
default_value, var_enum, flag);
230+
}
231+
int ompi_info_get_bool(ompi_info_t *info, char *key, bool *value, int *flag) {
232+
return opal_info_get_bool(&(info->super), key, value, flag);
233+
}
234+
int ompi_info_delete (ompi_info_t *info, const char *key) {
235+
return opal_info_delete (&(info->super), key);
236+
}
237+
int ompi_info_get_valuelen (ompi_info_t *info, const char *key, int *valuelen,
238+
int *flag)
239+
{
240+
return opal_info_get_valuelen (&(info->super), key, valuelen, flag);
241+
}
242+
int ompi_info_get_nthkey (ompi_info_t *info, int n, char *key) {
243+
return opal_info_get_nthkey (&(info->super), n, key);
244+
}
245+
int ompi_info_get_nkeys(ompi_info_t *info, int *nkeys)
246+
{
247+
return opal_info_get_nkeys (&(info->super), nkeys);
248+
}
249+
199250

200251
/*
201252
* Shut down MPI_Info handling

0 commit comments

Comments
 (0)