This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "MPICH primary repository". The branch, master has been updated via 38f365b4950f1579e2c46686edfc2a49f783f6bd (commit) via bdc46c7e7da2ffa03d58cab4a6c6cd4ac35913e3 (commit) from c895bd9cc8bdb1701ac149d91742727ecd3cbca7 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- http://git.mpich.org/mpich.git/commitdiff/38f365b4950f1579e2c46686edfc2a49f7... commit 38f365b4950f1579e2c46686edfc2a49f783f6bd Author: Paul Coffman <[email protected]> Date: Thu Apr 23 11:11:42 2015 -0500 PAMI_Rput_typed / PAMI_Rget_typed add ref count to MPID_Datatype Because the PAMI_Rput_typed / PAMI_Rget_typed are used with user defined derived types that could be freed between the time of the MPID_Put / Get and when the pami context actually executes the put or get (say in the win_fence) add a ref count to the MPID_Datatype at the time of the MPID_Put / Get and release the ref in the MPIDI_Win_DoneCB callback once the get or put has actually executed and it is ok to free the datatype. Signed-off-by: Rob Latham <[email protected]> diff --git a/src/mpid/pamid/src/onesided/mpid_1s.c b/src/mpid/pamid/src/onesided/mpid_1s.c index fa91cd9..f46c0ba 100644 --- a/src/mpid/pamid/src/onesided/mpid_1s.c +++ b/src/mpid/pamid/src/onesided/mpid_1s.c @@ -82,6 +82,15 @@ MPIDI_Win_DoneCB(pami_context_t context, MPID_cc_set(req_handle->cc_ptr, 0); } } + if ((MPIDI_Process.typed_onesided == 1) && (!req->target.dt.contig || !req->origin.dt.contig)) { + /* We used the PAMI_Rput/Rget_typed call and added a ref so any MPI_Type_free before the context + * executes the put/get would not free the MPID_Datatype, which would also free the associated PAMI datatype + * which was still needed for communication -- that communication has completed, so now release the ref + * in the callback to allow the MPID_Datatype to be freed. + */ + MPID_Datatype_release(req->origin.dt.pointer); + MPID_Datatype_release(req->target.dt.pointer); + } MPIDI_Progress_signal(); } diff --git a/src/mpid/pamid/src/onesided/mpid_win_get.c b/src/mpid/pamid/src/onesided/mpid_win_get.c index ba318af..338c4ab 100644 --- a/src/mpid/pamid/src/onesided/mpid_win_get.c +++ b/src/mpid/pamid/src/onesided/mpid_win_get.c @@ -396,16 +396,25 @@ MPID_Get(void *origin_addr, #endif - MPIDI_Win_datatype_map(&req->target.dt); - if ((!req->target.dt.contig || !req->origin.dt.contig) && (MPIDI_Process.typed_onesided == 1)) /* If the datatype is non-contiguous and the PAMID typed_onesided optimization * is enabled then we will be using the typed interface and will only make 1 call. */ win->mpid.sync.total = 1; - else + else { + MPIDI_Win_datatype_map(&req->target.dt); win->mpid.sync.total += req->target.dt.num_contig; + } + if ((MPIDI_Process.typed_onesided == 1) && (!req->target.dt.contig || !req->origin.dt.contig)) { + /* We will use the PAMI_Rget_typed call so we need to make sure any MPI_Type_free before the context + * executes the get does not free the MPID_Datatype, which would also free the associated PAMI datatype which + * is still needed for communication -- decrement the ref in the callback to allow the MPID_Datatype + * to be freed once the PAMI communication has completed. + */ + MPID_Datatype_add_ref(req->origin.dt.pointer); + MPID_Datatype_add_ref(req->target.dt.pointer); + } /* The pamid one-sided design requires context post in order to handle the * case where the number of pending rma operation exceeds the * 'PAMID_RMA_PENDING' threshold. When there are too many pending requests the diff --git a/src/mpid/pamid/src/onesided/mpid_win_put.c b/src/mpid/pamid/src/onesided/mpid_win_put.c index a7e7ce5..625676e 100644 --- a/src/mpid/pamid/src/onesided/mpid_win_put.c +++ b/src/mpid/pamid/src/onesided/mpid_win_put.c @@ -397,15 +397,24 @@ MPID_Put(const void *origin_addr, #endif - MPIDI_Win_datatype_map(&req->target.dt); if ((!req->target.dt.contig || !req->origin.dt.contig) && (MPIDI_Process.typed_onesided == 1)) /* If the datatype is non-contiguous and the PAMID typed_onesided optimization * is enabled then we will be using the typed interface and will only make 1 call. */ win->mpid.sync.total = 1; - else + else { + MPIDI_Win_datatype_map(&req->target.dt); win->mpid.sync.total += req->target.dt.num_contig; - + } + if ((MPIDI_Process.typed_onesided == 1) && (!req->target.dt.contig || !req->origin.dt.contig)) { + /* We will use the PAMI_Rput_typed call so we need to make sure any MPI_Type_free before the context + * executes the put does not free the MPID_Datatype, which would also free the associated PAMI datatype which + * is still needed for communication -- decrement the ref in the callback to allow the MPID_Datatype + * to be freed once the PAMI communication has completed. + */ + MPID_Datatype_add_ref(req->origin.dt.pointer); + MPID_Datatype_add_ref(req->target.dt.pointer); + } /* The pamid one-sided design requires context post in order to handle the * case where the number of pending rma operation exceeds the * 'PAMID_RMA_PENDING' threshold. When there are too many pending requests the http://git.mpich.org/mpich.git/commitdiff/bdc46c7e7da2ffa03d58cab4a6c6cd4ac3... commit bdc46c7e7da2ffa03d58cab4a6c6cd4ac35913e3 Author: Rob Latham <[email protected]> Date: Wed Apr 22 14:05:51 2015 -0500 tweak hint inspection approach Recent changes to MPI_INFO_GET will report a hard error if the info value passed in is too short. If we want to determine if a key is set or not, and don't care what it is, we'll use MPI_INFO_GET_VALUELEN. Took the opportunity to add a system-hints test to the ROMIO tests Signed-off-by: Paul Coffman <[email protected]> Signed-off-by: Wei-keng Liao <[email protected]> diff --git a/src/mpi/romio/adio/common/system_hints.c b/src/mpi/romio/adio/common/system_hints.c index fd6cba5..1b049b1 100644 --- a/src/mpi/romio/adio/common/system_hints.c +++ b/src/mpi/romio/adio/common/system_hints.c @@ -38,8 +38,8 @@ void ADIOI_Info_print_keyvals(MPI_Info info) { int i, nkeys, flag; - char key[MPI_MAX_INFO_KEY]; - char value[MPI_MAX_INFO_VAL]; + char key[MPI_MAX_INFO_KEY+1]; + char value[MPI_MAX_INFO_VAL+1]; if (info == MPI_INFO_NULL) return; @@ -48,7 +48,7 @@ void ADIOI_Info_print_keyvals(MPI_Info info) for (i=0; i<nkeys; i++) { MPI_Info_get_nthkey(info, i, key); - ADIOI_Info_get(info, key, MPI_MAX_INFO_VAL-1, value, &flag); + ADIOI_Info_get(info, key, MPI_MAX_INFO_VAL, value, &flag); printf("key = %-25s value = %-10s\n", key, value); } return; @@ -91,7 +91,7 @@ static int file_to_info_all(int fd, MPI_Info info, int rank, MPI_Comm comm) char *pos1=NULL, *pos2=NULL; int flag; ssize_t ret; - char dummy; + int valuelen; /* assumption: config files will be small */ #define HINTFILE_MAX_SIZE 1024*4 @@ -128,7 +128,7 @@ static int file_to_info_all(int fd, MPI_Info info, int rank, MPI_Comm comm) #endif /* don't actually care what the value is. only want to know if key * exists: we leave it alone if so*/ - ADIOI_Info_get(info, key, 1, &dummy, &flag); + ADIOI_Info_get_valuelen(info, key, &valuelen, &flag); if (flag == 1) continue; ADIOI_Info_set(info, key, val); } while ((token = strtok_r(NULL, "\n", &pos1)) != NULL); @@ -159,9 +159,10 @@ void ADIOI_incorporate_system_hints(MPI_Info info, MPI_Info sysinfo, MPI_Info *new_info) { - int i, nkeys_sysinfo, flag=0; /* must initialize flag to 0 */ + int i, nkeys_sysinfo, nkeys_info=0, flag=0; /* must initialize flag to 0 */ + int valuelen; - char val[MPI_MAX_INFO_VAL], key[MPI_MAX_INFO_KEY]; + char val[MPI_MAX_INFO_VAL+1], key[MPI_MAX_INFO_KEY+1]; if (sysinfo == MPI_INFO_NULL) nkeys_sysinfo = 0; @@ -176,15 +177,21 @@ void ADIOI_incorporate_system_hints(MPI_Info info, if (info == MPI_INFO_NULL) MPI_Info_create(new_info); - else + else { + /* tiny optimization: if 'info' has no keys, we can skip the check if a + * hint is set: no keys means nothing has been set, and there's nothing + * we might step on */ + MPI_Info_get_nkeys(info, &nkeys_info); MPI_Info_dup(info, new_info); + } for (i=0; i<nkeys_sysinfo; i++) { MPI_Info_get_nthkey(sysinfo, i, key); /* don't care about the value, just want to know if hint set already*/ - if (info != MPI_INFO_NULL) ADIOI_Info_get(info, key, 1, val, &flag); + if (info != MPI_INFO_NULL && nkeys_info) + ADIOI_Info_get_valuelen(info, key, &valuelen, &flag); if (flag == 1) continue; /* skip any hints already set by user */ - ADIOI_Info_get(sysinfo, key, MPI_MAX_INFO_VAL-1, val, &flag); + ADIOI_Info_get(sysinfo, key, MPI_MAX_INFO_VAL, val, &flag); ADIOI_Info_set(*new_info, key, val); flag = 0; } diff --git a/src/mpi/romio/test/Makefile.am b/src/mpi/romio/test/Makefile.am index caa5be9..cdddf64 100644 --- a/src/mpi/romio/test/Makefile.am +++ b/src/mpi/romio/test/Makefile.am @@ -25,7 +25,7 @@ AM_FFLAGS = $(USER_FFLAGS) CTESTS = simple perf async coll_test coll_perf misc file_info excl large_array \ atomicity noncontig i_noncontig noncontig_coll split_coll shared_fp \ large_file psimple error status noncontig_coll2 aggregation1 aggregation2 \ - async-multiple ordered_fp hindexed external32 types_with_zeros darray_read + async-multiple ordered_fp hindexed external32 types_with_zeros darray_read syshints FTESTS = fcoll_test fperf fmisc pfcoll_test diff --git a/src/mpi/romio/test/runtests.in b/src/mpi/romio/test/runtests.in index 2b8643e..0e5e58f 100644 --- a/src/mpi/romio/test/runtests.in +++ b/src/mpi/romio/test/runtests.in @@ -386,6 +386,11 @@ MakeExe darray_read echo '**** Testing darray_read ****' $mpirun -np 4 ./darray_read $FILENAME CleanExe darray_read +MakeExe syshints +echo '**** Testing syshints ****' +$mpirun -np 1 ./syshints $srcdir/test_hintfile +CleanExe syshints + # if [ @NOF77@ = 0 ] ; then echo "" diff --git a/src/mpi/romio/test/syshints.c b/src/mpi/romio/test/syshints.c new file mode 100755 index 0000000..e9952aa --- /dev/null +++ b/src/mpi/romio/test/syshints.c @@ -0,0 +1,53 @@ +#include <mpi.h> +#include <stdlib.h> +#include <stdio.h> +#include <string.h> + +static void handle_error(int errcode, const char *str) +{ + char msg[MPI_MAX_ERROR_STRING]; + int resultlen; + MPI_Error_string(errcode, msg, &resultlen); + fprintf(stderr, "%s: %s\n", str, msg); + MPI_Abort(MPI_COMM_WORLD, 1); +} + +#define CHECK(fn) {int errcode; errcode = (fn); if (errcode != MPI_SUCCESS) handle_error(errcode, #fn); } + +static int hint_check(MPI_Info info_used, char * key, char *expected) { + char value[MPI_MAX_INFO_VAL+1]; + int flag; + + CHECK(MPI_Info_get(info_used, key, MPI_MAX_INFO_VAL, value, &flag)); + if (strcmp(expected, value) ){ + fprintf(stderr, "expected value \"%s\" for key \"%s\" got \"%s\"\n", + expected, key, value); + return 1; + } + return 0; +} + +int main(int argc, char ** argv) +{ + setenv("ROMIO_HINTS", argv[1], 1); + MPI_File fh; + MPI_Info info_used, info_mine; + int nr_errors=0; + + MPI_Init(&argc, &argv); + MPI_Info_create(&info_mine); + MPI_Info_set(info_mine, "romio_cb_read", "disable"); + CHECK(MPI_File_open(MPI_COMM_WORLD, argv[1], MPI_MODE_RDONLY, info_mine, &fh)); + CHECK(MPI_File_get_info(fh, &info_used)); + + nr_errors += hint_check(info_used, "ind_rd_buffer_size", "49"); + nr_errors += hint_check(info_used, "romio_no_indep_rw", "true"); + + if (nr_errors == 0) printf(" No Errors\n"); + + CHECK(MPI_Info_free(&info_mine)); + CHECK(MPI_Info_free(&info_used)); + CHECK(MPI_File_close(&fh)); + MPI_Finalize(); + return nr_errors; +} diff --git a/src/mpi/romio/test/test_hintfile b/src/mpi/romio/test/test_hintfile new file mode 100644 index 0000000..edb72b0 --- /dev/null +++ b/src/mpi/romio/test/test_hintfile @@ -0,0 +1,6 @@ +romio_cb_read enable +# multiple info keys are strage but not an error. Note only the first one will +# be processed +ind_rd_buffer_size 49 +ind_rd_buffer_size 60 +romio_no_indep_rw true ----------------------------------------------------------------------- Summary of changes: src/mpi/romio/adio/common/system_hints.c | 27 +++++++++----- src/mpi/romio/test/Makefile.am | 2 +- src/mpi/romio/test/runtests.in | 5 +++ src/mpi/romio/test/syshints.c | 53 ++++++++++++++++++++++++++++ src/mpi/romio/test/test_hintfile | 6 +++ src/mpid/pamid/src/onesided/mpid_1s.c | 9 +++++ src/mpid/pamid/src/onesided/mpid_win_get.c | 15 ++++++-- src/mpid/pamid/src/onesided/mpid_win_put.c | 15 ++++++-- 8 files changed, 115 insertions(+), 17 deletions(-) create mode 100755 src/mpi/romio/test/syshints.c create mode 100644 src/mpi/romio/test/test_hintfile hooks/post-receive -- MPICH primary repository