io/ompio: fix for CID 1645290

move the lock protection to include setting the flag indicating that a
split collective is already ongoing. Unify the handling for
write_all_begin, write_at_all_begin, read_all_begin, read_at_all_begin
(even if coverty only complained about one of them).

Signed-off-by: Edgar Gabriel <[email protected]>

Author: edgargabriel

ompi/mca/io/ompio/io_ompio_file_read.c

@@ -362,13 +362,22 @@ int mca_io_ompio_file_read_all_begin (ompi_file_t *fh,
 
     data = (mca_common_ompio_data_t *) fh->f_io_selected_data;
     fp = &data->ompio_fh;
-    if ( true == fp->f_split_coll_in_use ) {
-	printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
-	return MPI_ERR_OTHER;
+    if (true == fp->f_split_coll_in_use) {
+        printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
+        return MPI_ERR_OTHER;
+    }
+
+    OPAL_THREAD_LOCK(&fh->f_lock);
+    if (true == fp->f_split_coll_in_use) {
+        printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
+        OPAL_THREAD_UNLOCK(&fh->f_lock);
+        return MPI_ERR_OTHER;
     }
-    /* No need for locking fh->f_lock, that is done in file_iread_all */
-    ret = mca_io_ompio_file_iread_all ( fh, buf, count, datatype, &fp->f_split_coll_req );
     fp->f_split_coll_in_use = true;
+    OPAL_THREAD_UNLOCK(&fh->f_lock);
+
+    /* No need for locking fh->f_lock for the operation itself, that is done in io_ompio_file_iread_all */
+    ret = mca_io_ompio_file_iread_all ( fh, buf, count, datatype, &fp->f_split_coll_req );
 
     return ret;
 }
@@ -402,14 +411,22 @@ int mca_io_ompio_file_read_at_all_begin (ompi_file_t *fh,
     data = (mca_common_ompio_data_t *) fh->f_io_selected_data;
     fp = &data->ompio_fh;
 
-    if ( true == fp->f_split_coll_in_use ) {
-	printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
-	return MPI_ERR_REQUEST;
+    if (true == fp->f_split_coll_in_use) {
+        printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
+        return MPI_ERR_REQUEST;
     }
+
     OPAL_THREAD_LOCK(&fh->f_lock);
+    if (true == fp->f_split_coll_in_use) {
+        printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
+        OPAL_THREAD_UNLOCK(&fh->f_lock);
+        return MPI_ERR_REQUEST;
+    }
+
+    fp->f_split_coll_in_use = true;
     ret = mca_common_ompio_file_iread_at_all ( fp, offset, buf, count, datatype, &fp->f_split_coll_req );
     OPAL_THREAD_UNLOCK(&fh->f_lock);
-    fp->f_split_coll_in_use = true;
+
     return ret;
 }
 

ompi/mca/io/ompio/io_ompio_file_write.c

@@ -371,12 +371,21 @@ int mca_io_ompio_file_write_all_begin (ompi_file_t *fh,
     data = (mca_common_ompio_data_t *) fh->f_io_selected_data;
     fp = &data->ompio_fh;
     if ( true == fp->f_split_coll_in_use ) {
-	printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
-	return MPI_ERR_OTHER;
+        printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
+        return MPI_ERR_OTHER;
+    }
+
+    OPAL_THREAD_LOCK(&fh->f_lock);
+    if (true == fp->f_split_coll_in_use) {
+        printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
+        OPAL_THREAD_UNLOCK(&fh->f_lock);
+        return MPI_ERR_REQUEST;
     }
-    /* No need for locking fh->f_lock, that is done in file_iwrite_all */
-    ret = mca_io_ompio_file_iwrite_all ( fh, buf, count, datatype, &fp->f_split_coll_req );
     fp->f_split_coll_in_use = true;
+    OPAL_THREAD_UNLOCK(&fh->f_lock);
+
+    /* No need for locking fh->f_lock the operation itself, that is done in io_ompio_file_iwrite_all */
+    ret = mca_io_ompio_file_iwrite_all ( fh, buf, count, datatype, &fp->f_split_coll_req );
 
     return ret;
 }
@@ -412,14 +421,20 @@ int mca_io_ompio_file_write_at_all_begin (ompi_file_t *fh,
     data = (mca_common_ompio_data_t *) fh->f_io_selected_data;
     fp = &data->ompio_fh;
 
-    if ( true == fp->f_split_coll_in_use ) {
-	printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
-	return MPI_ERR_REQUEST;
+    if (true == fp->f_split_coll_in_use) {
+        printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
+        return MPI_ERR_REQUEST;
     }
+
     OPAL_THREAD_LOCK(&fh->f_lock);
+    if (true == fp->f_split_coll_in_use) {
+        printf("Only one split collective I/O operation allowed per file handle at any given point in time!\n");
+        OPAL_THREAD_UNLOCK(&fh->f_lock);
+        return MPI_ERR_REQUEST;
+    }
+    fp->f_split_coll_in_use = true;
     ret = mca_common_ompio_file_iwrite_at_all ( fp, offset, buf, count, datatype, &fp->f_split_coll_req );
     OPAL_THREAD_UNLOCK(&fh->f_lock);
-    fp->f_split_coll_in_use = true;
 
     return ret;
 }