Skip to content

Commit fb3b68f

Browse files
committed
fbtl/posix: fix data-sieving calculations
as part of introducing atomicity support for ompi v5.0, we also tried to improve the robustness in some file I/O routines. Unfortunately, this also introduced a bug since ret_code returned by a function does not necessarily contain the number of bytes read or written, but could contain the last value (e.g. 0). The value was however used in a subsequent calculation and we ended not copying data out of the temporary buffer used in the data sieving at all. This commit also simplifies some of the logic in the while loop, no need to retry to read past the end of the file multiple times. Fixes issue #11917 Code was tested with the reproducer provided as part of the issue, our internal testsuite, and the hdf5-1.4.2 testsuite, all tests pass. Signed-off-by: Edgar Gabriel <[email protected]>
1 parent d6d175e commit fb3b68f

File tree

2 files changed

+6
-30
lines changed

2 files changed

+6
-30
lines changed

ompi/mca/fbtl/posix/fbtl_posix_preadv.c

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ static ssize_t mca_fbtl_posix_preadv_datasieving (ompio_file_t *fh, struct flock
3333
static ssize_t mca_fbtl_posix_preadv_generic (ompio_file_t *fh, struct flock *lock, int *lock_counter);
3434
static ssize_t mca_fbtl_posix_preadv_single (ompio_file_t *fh, struct flock *lock, int *lock_counter);
3535

36-
#define MAX_RETRIES 10
3736

3837
ssize_t mca_fbtl_posix_preadv (ompio_file_t *fh )
3938
{
@@ -108,7 +107,6 @@ ssize_t mca_fbtl_posix_preadv_single (ompio_file_t *fh, struct flock *lock, int
108107
return OMPI_ERROR;
109108
}
110109

111-
int retries = 0;
112110
size_t len = fh->f_io_array[0].length;
113111
while ( total_bytes < len ) {
114112
ret_code = pread(fh->fd, (char*)fh->f_io_array[0].memory_address+total_bytes,
@@ -121,13 +119,7 @@ ssize_t mca_fbtl_posix_preadv_single (ompio_file_t *fh, struct flock *lock, int
121119
}
122120
if ( ret_code == 0 ) {
123121
// end of file
124-
retries++;
125-
if ( retries == MAX_RETRIES ) {
126-
break;
127-
}
128-
else {
129-
continue;
130-
}
122+
break;
131123
}
132124
total_bytes += ret_code;
133125
}
@@ -206,7 +198,6 @@ ssize_t mca_fbtl_posix_preadv_datasieving (ompio_file_t *fh, struct flock *lock,
206198
return OMPI_ERROR;
207199
}
208200
size_t total_bytes = 0;
209-
int retries = 0;
210201

211202
while ( total_bytes < len ) {
212203
ret_code = pread (fh->fd, temp_buf+total_bytes, len-total_bytes, start+total_bytes);
@@ -218,13 +209,7 @@ ssize_t mca_fbtl_posix_preadv_datasieving (ompio_file_t *fh, struct flock *lock,
218209
}
219210
if ( ret_code == 0 ) {
220211
// end of file
221-
retries++;
222-
if ( retries == MAX_RETRIES ) {
223-
break;
224-
}
225-
else {
226-
continue;
227-
}
212+
break;
228213
}
229214
total_bytes += ret_code;
230215
}
@@ -236,12 +221,12 @@ ssize_t mca_fbtl_posix_preadv_datasieving (ompio_file_t *fh, struct flock *lock,
236221
size_t start_offset = (size_t) fh->f_io_array[startindex].offset;
237222
for ( i = startindex ; i < endindex ; i++) {
238223
pos = (size_t) fh->f_io_array[i].offset - start_offset;
239-
if ( (ssize_t) pos > ret_code ) {
224+
if ( (ssize_t) pos > total_bytes ) {
240225
break;
241226
}
242227
num_bytes = fh->f_io_array[i].length;
243-
if ( ((ssize_t) pos + (ssize_t)num_bytes) > ret_code ) {
244-
num_bytes = ret_code - (ssize_t)pos;
228+
if ( ((ssize_t) pos + (ssize_t)num_bytes) > total_bytes ) {
229+
num_bytes = total_bytes - (ssize_t)pos;
245230
}
246231

247232
memcpy (fh->f_io_array[i].memory_address, temp_buf + pos, num_bytes);

ompi/mca/fbtl/posix/fbtl_posix_pwritev.c

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ static ssize_t mca_fbtl_posix_pwritev_datasieving (ompio_file_t *fh, struct floc
3434
static ssize_t mca_fbtl_posix_pwritev_generic (ompio_file_t *fh, struct flock *lock, int *lock_counter );
3535
static ssize_t mca_fbtl_posix_pwritev_single (ompio_file_t *fh, struct flock *lock, int *lock_counter );
3636

37-
#define MAX_RETRIES 10
38-
3937
ssize_t mca_fbtl_posix_pwritev(ompio_file_t *fh )
4038
{
4139
ssize_t bytes_written=0;
@@ -192,7 +190,6 @@ ssize_t mca_fbtl_posix_pwritev_datasieving (ompio_file_t *fh, struct flock *lock
192190
return OMPI_ERROR;
193191
}
194192

195-
int retries=0;
196193
while ( total_bytes < len ) {
197194
ret_code = pread (fh->fd, temp_buf, len, start);
198195
if ( ret_code == -1 ) {
@@ -203,13 +200,7 @@ ssize_t mca_fbtl_posix_pwritev_datasieving (ompio_file_t *fh, struct flock *lock
203200
}
204201
if ( ret_code == 0 ) {
205202
// end of file
206-
retries++;
207-
if ( retries == MAX_RETRIES ) {
208-
break;
209-
}
210-
else {
211-
continue;
212-
}
203+
break;
213204
}
214205
total_bytes += ret_code;
215206
}

0 commit comments

Comments
 (0)