Skip to content

Commit c814914

Browse files
wkliaoggouaillardet
wkliao
authored and
ggouaillardet
committed
[mpich romio bbb5210] make ADIOI_GEN_WriteStrided not step on itself
> Pulled in from mpich romio, branch "main". > Their commit message is below. > > This is part of a batch of commits from the > following set of PRs: > * pmodels/mpich#4943 > -- darray fix which contains a flatten fix > 73a3eba > c4b5762 > * pmodels/mpich#4995 > -- write strided fix > bbb5210 > * pmodels/mpich#5100 > -- build fix for -Wpedantic > ad0e435 > * pmodels/mpich#5099 > -- build fix, they had let file-system=...gpfs bit rot > e1d42af > 313289a > 83bbb82 > * pmodels/mpich#5150 > -- build fix, configure-related _GNU_SOURCE > a712b56 > 5a036e7 > * pmodels/mpich#5184 > -- build fix, continuation of _GNU_SOURCE fix > d97c4ee The ADIOI_GEN_WriteStrided funcion uses data sieving on non-contiguous types. That is, if it wants to write data at locations [W...X...Y...Z...] it reads the whole buffer [dddddddddddddddd] changes the locations it wants to write to [WdddXdddYdddZddd] then writes the whole thing back. It uses locks to make this safe, but the problem is this only protects against other parts of the product that are using locks. And without this PR a peer who is simultaneously making a simple non-contiguous write wouldn't have locked. A testcase to demonstrate the original problem is here: https://gist.github.com/markalle/d7da240c19e57f095c5d1b13240dae24 % mpicc -o x romio_write_timing.c % mpirun -np 4 ./x Note: you need to use a filesystem that uses ADIOI_GEN_WriteStrided to hit the bug. I was using GPFS. This commit is pulled from wkliao after discussing where to put the new lock. It adds locks to contiguous writes in independent write functions when data sieving write is not disabled Signed-off-by: Mark Allen <[email protected]>
1 parent 3e7f46b commit c814914

File tree

2 files changed

+36
-22
lines changed

2 files changed

+36
-22
lines changed

‎3rd-party/romio341/adio/ad_lustre/ad_lustre_wrstr.c

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
ADIO_WriteContig(fd, writebuf, writebuf_len, MPI_BYTE, \
1414
ADIO_EXPLICIT_OFFSET, writebuf_off, \
1515
&status1, error_code); \
16-
if (!(fd->atomicity)) \
16+
if (!fd->atomicity && fd->hints->ds_write == ADIOI_HINT_DISABLE) \
1717
ADIOI_UNLOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
1818
if (*error_code != MPI_SUCCESS) { \
1919
*error_code = MPIO_Err_create_code(*error_code, \
@@ -30,7 +30,7 @@
3030
writebuf_len = (unsigned) MPL_MIN(end_offset - writebuf_off + 1, \
3131
(writebuf_off / stripe_size + 1) * \
3232
stripe_size - writebuf_off); \
33-
if (!(fd->atomicity)) \
33+
if (!fd->atomicity && fd->hints->ds_write == ADIOI_HINT_DISABLE) \
3434
ADIOI_WRITE_LOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
3535
ADIO_ReadContig(fd, writebuf, writebuf_len, MPI_BYTE, \
3636
ADIO_EXPLICIT_OFFSET, \
@@ -53,7 +53,7 @@
5353
while (write_sz != req_len) { \
5454
ADIO_WriteContig(fd, writebuf, writebuf_len, MPI_BYTE, \
5555
ADIO_EXPLICIT_OFFSET, writebuf_off, &status1, error_code); \
56-
if (!(fd->atomicity)) \
56+
if (!fd->atomicity && fd->hints->ds_write == ADIOI_HINT_DISABLE) \
5757
ADIOI_UNLOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
5858
if (*error_code != MPI_SUCCESS) { \
5959
*error_code = MPIO_Err_create_code(*error_code, \
@@ -70,7 +70,7 @@
7070
writebuf_len = (unsigned) MPL_MIN(end_offset - writebuf_off + 1, \
7171
(writebuf_off / stripe_size + 1) * \
7272
stripe_size - writebuf_off); \
73-
if (!(fd->atomicity)) \
73+
if (!fd->atomicity && fd->hints->ds_write == ADIOI_HINT_DISABLE) \
7474
ADIOI_WRITE_LOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
7575
ADIO_ReadContig(fd, writebuf, writebuf_len, MPI_BYTE, \
7676
ADIO_EXPLICIT_OFFSET, \
@@ -213,8 +213,9 @@ void ADIOI_LUSTRE_WriteStrided(ADIO_File fd, const void *buf, int count,
213213
writebuf_off = 0;
214214
writebuf_len = 0;
215215

216-
/* if atomicity is true, lock the region to be accessed */
217-
if (fd->atomicity)
216+
/* if atomicity is true or data sieving is not disable, lock the region
217+
* to be accessed */
218+
if (fd->atomicity || fd->hints->ds_write != ADIOI_HINT_DISABLE)
218219
ADIOI_WRITE_LOCK(fd, start_off, SEEK_SET, bufsize);
219220

220221
for (j = 0; j < count; j++) {
@@ -231,7 +232,7 @@ void ADIOI_LUSTRE_WriteStrided(ADIO_File fd, const void *buf, int count,
231232
ADIO_WriteContig(fd, writebuf, writebuf_len, MPI_BYTE,
232233
ADIO_EXPLICIT_OFFSET, writebuf_off, &status1, error_code);
233234

234-
if (fd->atomicity)
235+
if (fd->atomicity || fd->hints->ds_write != ADIOI_HINT_DISABLE)
235236
ADIOI_UNLOCK(fd, start_off, SEEK_SET, bufsize);
236237
if (*error_code != MPI_SUCCESS) {
237238
ADIOI_Free(writebuf);
@@ -311,8 +312,12 @@ void ADIOI_LUSTRE_WriteStrided(ADIO_File fd, const void *buf, int count,
311312
userbuf_off = 0;
312313
ADIOI_BUFFERED_WRITE_WITHOUT_READ;
313314
/* write the buffer out finally */
315+
if (fd->hints->ds_write != ADIOI_HINT_DISABLE)
316+
ADIOI_WRITE_LOCK(fd, writebuf_off, SEEK_SET, writebuf_len);
314317
ADIO_WriteContig(fd, writebuf, writebuf_len, MPI_BYTE,
315318
ADIO_EXPLICIT_OFFSET, writebuf_off, &status1, error_code);
319+
if (fd->hints->ds_write != ADIOI_HINT_DISABLE)
320+
ADIOI_UNLOCK(fd, writebuf_off, SEEK_SET, writebuf_len);
316321

317322
if (file_ptr_type == ADIO_INDIVIDUAL) {
318323
/* update MPI-IO file pointer to point to the first byte
@@ -362,8 +367,9 @@ void ADIOI_LUSTRE_WriteStrided(ADIO_File fd, const void *buf, int count,
362367
fwr_size = MPL_MIN(flat_file->blocklens[j], bufsize - i_offset);
363368
}
364369

365-
/* if atomicity is true, lock the region to be accessed */
366-
if (fd->atomicity)
370+
/* if atomicity is true or data sieving is not disable, lock the region
371+
* to be accessed */
372+
if (fd->atomicity || fd->hints->ds_write != ADIOI_HINT_DISABLE)
367373
ADIOI_WRITE_LOCK(fd, start_off, SEEK_SET, end_offset - start_off + 1);
368374

369375
writebuf_off = 0;
@@ -481,12 +487,12 @@ void ADIOI_LUSTRE_WriteStrided(ADIO_File fd, const void *buf, int count,
481487
if (writebuf_len) {
482488
ADIO_WriteContig(fd, writebuf, writebuf_len, MPI_BYTE,
483489
ADIO_EXPLICIT_OFFSET, writebuf_off, &status1, error_code);
484-
if (!(fd->atomicity))
490+
if (!fd->atomicity && fd->hints->ds_write == ADIOI_HINT_DISABLE)
485491
ADIOI_UNLOCK(fd, writebuf_off, SEEK_SET, writebuf_len);
486492
if (*error_code != MPI_SUCCESS)
487493
return;
488494
}
489-
if (fd->atomicity)
495+
if (fd->atomicity || fd->hints->ds_write != ADIOI_HINT_DISABLE)
490496
ADIOI_UNLOCK(fd, start_off, SEEK_SET, end_offset - start_off + 1);
491497

492498
ADIOI_Free(writebuf);

‎3rd-party/romio341/adio/common/ad_write_str.c

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
if (writebuf_len) { \
1313
ADIO_WriteContig(fd, writebuf, writebuf_len, MPI_BYTE, \
1414
ADIO_EXPLICIT_OFFSET, writebuf_off, &status1, error_code); \
15-
if (!(fd->atomicity)) ADIOI_UNLOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
15+
if (!fd->atomicity && fd->hints->ds_write == ADIOI_HINT_DISABLE) \
16+
ADIOI_UNLOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
1617
if (*error_code != MPI_SUCCESS) { \
1718
*error_code = MPIO_Err_create_code(*error_code, \
1819
MPIR_ERR_RECOVERABLE, myname, \
@@ -23,7 +24,8 @@
2324
} \
2425
writebuf_off = req_off; \
2526
writebuf_len = (unsigned) (MPL_MIN(max_bufsize,end_offset-writebuf_off+1)); \
26-
if (!(fd->atomicity)) ADIOI_WRITE_LOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
27+
if (!fd->atomicity && fd->hints->ds_write == ADIOI_HINT_DISABLE) \
28+
ADIOI_WRITE_LOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
2729
ADIO_ReadContig(fd, writebuf, writebuf_len, MPI_BYTE, \
2830
ADIO_EXPLICIT_OFFSET, writebuf_off, &status1, error_code); \
2931
if (*error_code != MPI_SUCCESS) { \
@@ -40,7 +42,8 @@
4042
while (write_sz != req_len) { \
4143
ADIO_WriteContig(fd, writebuf, writebuf_len, MPI_BYTE, \
4244
ADIO_EXPLICIT_OFFSET, writebuf_off, &status1, error_code); \
43-
if (!(fd->atomicity)) ADIOI_UNLOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
45+
if (!fd->atomicity && fd->hints->ds_write == ADIOI_HINT_DISABLE) \
46+
ADIOI_UNLOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
4447
if (*error_code != MPI_SUCCESS) { \
4548
*error_code = MPIO_Err_create_code(*error_code, \
4649
MPIR_ERR_RECOVERABLE, myname, \
@@ -52,7 +55,8 @@
5255
userbuf_off += write_sz; \
5356
writebuf_off += writebuf_len; \
5457
writebuf_len = (unsigned) (MPL_MIN(max_bufsize,end_offset-writebuf_off+1)); \
55-
if (!(fd->atomicity)) ADIOI_WRITE_LOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
58+
if (!fd->atomicity && fd->hints->ds_write == ADIOI_HINT_DISABLE) \
59+
ADIOI_WRITE_LOCK(fd, writebuf_off, SEEK_SET, writebuf_len); \
5660
ADIO_ReadContig(fd, writebuf, writebuf_len, MPI_BYTE, \
5761
ADIO_EXPLICIT_OFFSET, writebuf_off, &status1, error_code); \
5862
if (*error_code != MPI_SUCCESS) { \
@@ -184,8 +188,9 @@ void ADIOI_GEN_WriteStrided(ADIO_File fd, const void *buf, int count,
184188
writebuf = (char *) ADIOI_Malloc(max_bufsize);
185189
writebuf_len = (unsigned) (MPL_MIN(max_bufsize, end_offset - writebuf_off + 1));
186190

187-
/* if atomicity is true, lock the region to be accessed */
188-
if (fd->atomicity)
191+
/* if atomicity is true or data sieving is not disable, lock the region
192+
* to be accessed */
193+
if (fd->atomicity || fd->hints->ds_write != ADIOI_HINT_DISABLE)
189194
ADIOI_WRITE_LOCK(fd, start_off, SEEK_SET, end_offset - start_off + 1);
190195

191196
for (j = 0; j < count; j++) {
@@ -204,7 +209,7 @@ void ADIOI_GEN_WriteStrided(ADIO_File fd, const void *buf, int count,
204209
writebuf_off, &status1, error_code);
205210
}
206211

207-
if (fd->atomicity)
212+
if (fd->atomicity || fd->hints->ds_write != ADIOI_HINT_DISABLE)
208213
ADIOI_UNLOCK(fd, start_off, SEEK_SET, end_offset - start_off + 1);
209214

210215
if (*error_code != MPI_SUCCESS)
@@ -280,8 +285,10 @@ void ADIOI_GEN_WriteStrided(ADIO_File fd, const void *buf, int count,
280285
* datatypes, instead of a count of bytes (which might overflow)
281286
* Other WriteContig calls in this path are operating on data
282287
* sieving buffer */
288+
ADIOI_WRITE_LOCK(fd, offset, SEEK_SET, bufsize);
283289
ADIO_WriteContig(fd, buf, count, datatype, ADIO_EXPLICIT_OFFSET,
284290
offset, status, error_code);
291+
ADIOI_UNLOCK(fd, offset, SEEK_SET, bufsize);
285292

286293
if (file_ptr_type == ADIO_INDIVIDUAL) {
287294
/* update MPI-IO file pointer to point to the first byte
@@ -330,8 +337,9 @@ void ADIOI_GEN_WriteStrided(ADIO_File fd, const void *buf, int count,
330337
fwr_size = MPL_MIN(flat_file->blocklens[j], bufsize - i_offset);
331338
}
332339

333-
/* if atomicity is true, lock the region to be accessed */
334-
if (fd->atomicity)
340+
/* if atomicity is true or data sieving is not disable, lock the region
341+
* to be accessed */
342+
if (fd->atomicity || fd->hints->ds_write != ADIOI_HINT_DISABLE)
335343
ADIOI_WRITE_LOCK(fd, start_off, SEEK_SET, end_offset - start_off + 1);
336344

337345
writebuf_off = 0;
@@ -451,12 +459,12 @@ void ADIOI_GEN_WriteStrided(ADIO_File fd, const void *buf, int count,
451459
if (writebuf_len) {
452460
ADIO_WriteContig(fd, writebuf, writebuf_len, MPI_BYTE, ADIO_EXPLICIT_OFFSET,
453461
writebuf_off, &status1, error_code);
454-
if (!(fd->atomicity))
462+
if (!fd->atomicity && fd->hints->ds_write == ADIOI_HINT_DISABLE)
455463
ADIOI_UNLOCK(fd, writebuf_off, SEEK_SET, writebuf_len);
456464
if (*error_code != MPI_SUCCESS)
457465
goto fn_exit;
458466
}
459-
if (fd->atomicity)
467+
if (fd->atomicity || fd->hints->ds_write != ADIOI_HINT_DISABLE)
460468
ADIOI_UNLOCK(fd, start_off, SEEK_SET, end_offset - start_off + 1);
461469

462470
if (file_ptr_type == ADIO_INDIVIDUAL)

0 commit comments

Comments
 (0)