From c0d7b578beb4c5e8b27f7d8385e8578c9d210585 Mon Sep 17 00:00:00 2001 From: Edgar Gabriel Date: Thu, 11 Oct 2018 14:07:45 -0500 Subject: [PATCH 1/5] io/ompio: fix seek position calculation for SEEK_CUR This commit fixes the calculation of the position where to seek to, in case SEEK_CUR is used. Signed-off-by: Edgar Gabriel --- ompi/mca/io/ompio/io_ompio_file_open.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ompi/mca/io/ompio/io_ompio_file_open.c b/ompi/mca/io/ompio/io_ompio_file_open.c index 37f7b308b7..cf9b545fa5 100644 --- a/ompi/mca/io/ompio/io_ompio_file_open.c +++ b/ompi/mca/io/ompio/io_ompio_file_open.c @@ -400,8 +400,9 @@ int mca_io_ompio_file_seek (ompi_file_t *fh, } break; case MPI_SEEK_CUR: - offset += data->ompio_fh.f_position_in_file_view; - offset += data->ompio_fh.f_disp; + ret = mca_common_ompio_file_get_position (&data->ompio_fh, + &temp_offset); + offset += temp_offset; if (offset < 0) { OPAL_THREAD_UNLOCK(&fh->f_lock); return OMPI_ERROR; From 05d25383c2c57a1df9a59675fe46f868587e8e3f Mon Sep 17 00:00:00 2001 From: Edgar Gabriel Date: Thu, 11 Oct 2018 14:41:58 -0500 Subject: [PATCH 2/5] common/ompio: return correct error code for improper access return MPI_ERR_ACCESS if the user tries to read from a file that was opened using MPI_MODE_WRONLY return MPI_ERR_READ_ONLY if the user tries to write a file that was opened using MPI_MODE_RDONLY Signed-off-by: Edgar Gabriel --- ompi/mca/common/ompio/common_ompio_file_read.c | 17 ++++++++++++----- ompi/mca/common/ompio/common_ompio_file_write.c | 13 +++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/ompi/mca/common/ompio/common_ompio_file_read.c b/ompi/mca/common/ompio/common_ompio_file_read.c index 6d6d112eb3..3203e2a697 100644 --- a/ompi/mca/common/ompio/common_ompio_file_read.c +++ b/ompi/mca/common/ompio/common_ompio_file_read.c @@ -77,6 +77,12 @@ int mca_common_ompio_file_read (ompio_file_t *fh, int i = 0; /* index into the decoded iovec of the buffer */ int j = 0; /* index into the file vie iovec */ + if (fh->f_amode & MPI_MODE_WRONLY){ +// opal_output(10, "Improper use of FILE Mode, Using WRONLY for Read!\n"); + ret = MPI_ERR_ACCESS; + return ret; + } + if ( 0 == count ) { if ( MPI_STATUS_IGNORE != status ) { status->_ucount = 0; @@ -84,11 +90,6 @@ int mca_common_ompio_file_read (ompio_file_t *fh, return ret; } - if (fh->f_amode & MPI_MODE_WRONLY){ - printf("Improper use of FILE Mode, Using WRONLY for Read!\n"); - ret = OMPI_ERROR; - return ret; - } #if OPAL_CUDA_SUPPORT int is_gpu, is_managed; @@ -226,6 +227,12 @@ int mca_common_ompio_file_iread (ompio_file_t *fh, mca_ompio_request_t *ompio_req=NULL; size_t spc=0; + if (fh->f_amode & MPI_MODE_WRONLY){ +// opal_output(10, "Improper use of FILE Mode, Using WRONLY for Read!\n"); + ret = MPI_ERR_ACCESS; + return ret; + } + mca_common_ompio_request_alloc ( &ompio_req, MCA_OMPIO_REQUEST_READ); if ( 0 == count ) { diff --git a/ompi/mca/common/ompio/common_ompio_file_write.c b/ompi/mca/common/ompio/common_ompio_file_write.c index fb62edf2d9..e53a1d080b 100644 --- a/ompi/mca/common/ompio/common_ompio_file_write.c +++ b/ompi/mca/common/ompio/common_ompio_file_write.c @@ -58,6 +58,13 @@ int mca_common_ompio_file_write (ompio_file_t *fh, int i = 0; /* index into the decoded iovec of the buffer */ int j = 0; /* index into the file view iovec */ + if (fh->f_amode & MPI_MODE_RDONLY){ +// opal_output(10, "Improper use of FILE Mode, Using RDONLY for write!\n"); + ret = MPI_ERR_READ_ONLY; + return ret; + } + + if ( 0 == count ) { if ( MPI_STATUS_IGNORE != status ) { status->_ucount = 0; @@ -194,6 +201,12 @@ int mca_common_ompio_file_iwrite (ompio_file_t *fh, mca_ompio_request_t *ompio_req=NULL; size_t spc=0; + if (fh->f_amode & MPI_MODE_RDONLY){ +// opal_output(10, "Improper use of FILE Mode, Using RDONLY for write!\n"); + ret = MPI_ERR_READ_ONLY; + return ret; + } + mca_common_ompio_request_alloc ( &ompio_req, MCA_OMPIO_REQUEST_WRITE); if ( 0 == count ) { From bf058ca6b0aa739c1ddfae522d1bfd0015e9c0b1 Mon Sep 17 00:00:00 2001 From: Edgar Gabriel Date: Thu, 11 Oct 2018 14:43:32 -0500 Subject: [PATCH 3/5] common/ompio: check datatypes when setting file view return MPI_ERR_ARG if the size of the fileview is not a multiple of the size of the etype provided. Signed-off-by: Edgar Gabriel --- ompi/mca/common/ompio/common_ompio_file_view.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ompi/mca/common/ompio/common_ompio_file_view.c b/ompi/mca/common/ompio/common_ompio_file_view.c index 71ba14ba02..bf8a25345b 100644 --- a/ompi/mca/common/ompio/common_ompio_file_view.c +++ b/ompi/mca/common/ompio/common_ompio_file_view.c @@ -141,6 +141,10 @@ int mca_common_ompio_set_view (ompio_file_t *fh, // in orig_file type, No need to set args on this one. ompi_datatype_duplicate (newfiletype, &fh->f_filetype); + if ( (fh->f_view_size % fh->f_etype_size) ) { + // File view is not a multiple of the etype. + return MPI_ERR_ARG; + } if( SIMPLE_PLUS == OMPIO_MCA_GET(fh, grouping_option) ) { fh->f_cc_size = get_contiguous_chunk_size (fh, 1); From 849d0452a07e74a0093cbb8f25425fd369dfa99a Mon Sep 17 00:00:00 2001 From: Edgar Gabriel Date: Thu, 11 Oct 2018 17:39:05 -0500 Subject: [PATCH 4/5] io/ompio: execute barrier before sync this ensures that all processes are done modifying a file before syncing. Fixes an error in the testmpio testsuite. Signed-off-by: Edgar Gabriel --- ompi/mca/io/ompio/io_ompio_file_open.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ompi/mca/io/ompio/io_ompio_file_open.c b/ompi/mca/io/ompio/io_ompio_file_open.c index cf9b545fa5..37bc8fea57 100644 --- a/ompi/mca/io/ompio/io_ompio_file_open.c +++ b/ompi/mca/io/ompio/io_ompio_file_open.c @@ -9,7 +9,7 @@ * University of Stuttgart. All rights reserved. * Copyright (c) 2004-2005 The Regents of the University of California. * All rights reserved. - * Copyright (c) 2008-2016 University of Houston. All rights reserved. + * Copyright (c) 2008-2018 University of Houston. All rights reserved. * Copyright (c) 2015-2018 Research Organization for Information Science * and Technology (RIST). All rights reserved. * Copyright (c) 2016 Cisco Systems, Inc. All rights reserved. @@ -372,6 +372,13 @@ int mca_io_ompio_file_sync (ompi_file_t *fh) OPAL_THREAD_UNLOCK(&fh->f_lock); return MPI_ERR_ACCESS; } + // Make sure all processes reach this point before syncing the file. + ret = data->ompio_fh.f_comm->c_coll->coll_barrier (data->ompio_fh.f_comm, + data->ompio_fh.f_comm->c_coll->coll_barrier_module); + if ( MPI_SUCCESS != ret ) { + OPAL_THREAD_UNLOCK(&fh->f_lock); + return ret; + } ret = data->ompio_fh.f_fs->fs_file_sync (&data->ompio_fh); OPAL_THREAD_UNLOCK(&fh->f_lock); From ba955883329cc37aba89dc6bf0012bce8349d814 Mon Sep 17 00:00:00 2001 From: Edgar Gabriel Date: Tue, 16 Oct 2018 12:45:33 -0500 Subject: [PATCH 5/5] io/ompio: add verification for data representations. check for providing a data representation that is actually supported by ompio. Add also one check for a non-NULL pointer in mpi/c/file_set_view for the data representation. Also fixes parts of issue #5643 Signed-off-by: Edgar Gabriel --- ompi/mca/io/ompio/io_ompio_file_set_view.c | 8 ++++++-- ompi/mpi/c/file_set_view.c | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/ompi/mca/io/ompio/io_ompio_file_set_view.c b/ompi/mca/io/ompio/io_ompio_file_set_view.c index ba18db8fe1..72671c3410 100644 --- a/ompi/mca/io/ompio/io_ompio_file_set_view.c +++ b/ompi/mca/io/ompio/io_ompio_file_set_view.c @@ -9,7 +9,7 @@ * University of Stuttgart. All rights reserved. * Copyright (c) 2004-2005 The Regents of the University of California. * All rights reserved. - * Copyright (c) 2008-2016 University of Houston. All rights reserved. + * Copyright (c) 2008-2018 University of Houston. All rights reserved. * Copyright (c) 2015-2018 Research Organization for Information Science * and Technology (RIST). All rights reserved. * Copyright (c) 2016-2017 IBM Corporation. All rights reserved. @@ -66,13 +66,17 @@ int mca_io_ompio_file_set_view (ompi_file_t *fp, mca_common_ompio_data_t *data; ompio_file_t *fh; + if ( (strcmp(datarep, "native") && strcmp(datarep, "NATIVE"))) { + return MPI_ERR_UNSUPPORTED_DATAREP; + } + data = (mca_common_ompio_data_t *) fp->f_io_selected_data; /* we need to call the internal file set view twice: once for the individual file pointer, once for the shared file pointer (if it is existent) */ fh = &data->ompio_fh; - + OPAL_THREAD_LOCK(&fp->f_lock); ret = mca_common_ompio_set_view(fh, disp, etype, filetype, datarep, info); OPAL_THREAD_UNLOCK(&fp->f_lock); diff --git a/ompi/mpi/c/file_set_view.c b/ompi/mpi/c/file_set_view.c index a49a80f29a..c62df489aa 100644 --- a/ompi/mpi/c/file_set_view.c +++ b/ompi/mpi/c/file_set_view.c @@ -64,6 +64,10 @@ int MPI_File_set_view(MPI_File fh, MPI_Offset disp, MPI_Datatype etype, OMPI_CHECK_DATATYPE_FOR_VIEW(rc, filetype, 0); } } + if ( NULL == datarep) { + rc = MPI_ERR_UNSUPPORTED_DATAREP; + fh = MPI_FILE_NULL; + } OMPI_ERRHANDLER_CHECK(rc, fh, rc, FUNC_NAME); }