This particular condition is obviously known to be problematic, as shown by numerous comments in the ext4 kernel code:

for example, commit e9e3bcecf44c0, which went into 2.6.38 added:
/*
 * This tests whether the IO in question is block-aligned or not.
 * Ext4 utilizes unwritten extents when hole-filling during direct IO, and they
 * are converted to written only after the IO is complete.  Until they are
 * mapped, these blocks appear as holes, so dio_zero_block() will assume that
 * it needs to zero out portions of the start and/or end block.  If 2 AIO
 * threads are at work on the same unwritten block, they must be synchronized
 * or one thread will zero the other's data, causing corruption.
 */

and upstream commit e142d05263a4b went into 4.6 added:

+        * Unaligned direct AIO must be serialized among each other as zeroing
+        * of partial blocks of two competing unaligned AIOs can result in data
+        * corruption.

however I've reproduced this with 4.19+, so this does not appear resolved.

Created attachment 1541975 [details]
updated/simplified reproducer

Here's a much-simplified reproducer

(In reply to Frank Sorenson from comment #15)
> Created attachment 1541975 [details]
> updated/simplified reproducer
> 
> Here's a much-simplified reproducer

Can someone help to explain why the reopen is needed after ftruncate?

        ftruncate(fd, WRITE0_SIZE);
        close(fd);
        ....
        fd = open(filename, O_RDWR|O_DIRECT);

I've tried, if no this reopen, can't reproduce this bug.


Thanks,
Zorro

Okay, the close()/open() can be eliminated if you initially open the file with 'O_DIRECT' as well (the unlink() can also be removed, since the following open() call includes O_TRUNC):

        if ((fd = open(filename, O_RDWR|O_CREAT|O_TRUNC|O_DIRECT, 0660)) == -1)

after changing the initial open options like this, the bug seems to reproduce consistently again.


However, also note that the file corruption is dependent on timing of the IOs, so there is a possibility that the test may return SUCCESS, rather than FAILURE; some light testing leads me to believe this may occur once about every 150-250 runs (on my test VMs).  Separating the ftruncate() from the io_submit() with the close()/open() appears to double the number of iterations between successful runs (failures to FAIL?).

Might be best to run the test several times, just in case it doesn't reproduce the bug the first time (though I suppose a very slow machine could potentially always return SUCCESS, even though the bug is still there).

I'm attaching an updated reproducer that tests multiple times, and eliminates the reopen.  With testing 5 times, I haven't had it incorrectly report SUCCESS in hundreds of thousands of runs.

Created attachment 1542966 [details]
updated updated reproducer

(In reply to Frank Sorenson from comment #17)
> Okay, the close()/open() can be eliminated if you initially open the file
> with 'O_DIRECT' as well (the unlink() can also be removed, since the
> following open() call includes O_TRUNC):
> 
>         if ((fd = open(filename, O_RDWR|O_CREAT|O_TRUNC|O_DIRECT, 0660)) ==
> -1)

Oh, sorry I didn't notice that the 1st open didn't use 'O_DIRECT'.

I've written a case for xfstests, but I still have one question which I don't know
how to comment it: why the ftruncate is necessary? And looks like must truncate
to the end of the 1st write operation (offset + 1st write size) to reproduce this bug.

Thanks,
Zorro

> 
> after changing the initial open options like this, the bug seems to
> reproduce consistently again.
> 
> 
> However, also note that the file corruption is dependent on timing of the
> IOs, so there is a possibility that the test may return SUCCESS, rather than
> FAILURE; some light testing leads me to believe this may occur once about
> every 150-250 runs (on my test VMs).  Separating the ftruncate() from the
> io_submit() with the close()/open() appears to double the number of
> iterations between successful runs (failures to FAIL?).
> 
> Might be best to run the test several times, just in case it doesn't
> reproduce the bug the first time (though I suppose a very slow machine could
> potentially always return SUCCESS, even though the bug is still there).
> 
> I'm attaching an updated reproducer that tests multiple times, and
> eliminates the reopen.  With testing 5 times, I haven't had it incorrectly
> report SUCCESS in hundreds of thousands of runs.

Hi Zorro,

the ftruncate is absolutely necessary because this bug is about writes at the i_size boundary, or rather in the same fs block as i_size (see the description of my ext4 patch).

https://patchwork.ozlabs.org/patch/1052277/

-Lukas

(In reply to Lukáš Czerner from comment #20)
> Hi Zorro,
> 
> the ftruncate is absolutely necessary because this bug is about writes at
> the i_size boundary, or rather in the same fs block as i_size (see the
> description of my ext4 patch).
> 
> https://patchwork.ozlabs.org/patch/1052277/

Thanks Lukas, finally I can understand all things by reading the patch:)
I've written a case to xfstests, please feel free to review it if you'd like.

> 
> -Lukas

Patch(es) committed on kernel-3.10.0-1031.el7

*** Bug 1699799 has been marked as a duplicate of this bug. ***

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2019:2029