On Wed, May 4, 2022 at 12:42 AM Linus Torvalds <torva...@linux-foundation.org> wrote: > On Tue, May 3, 2022 at 2:35 PM Andreas Gruenbacher <agrue...@redhat.com> > wrote: > > > > More testing still ongoing, but the following patch seems to fix the > > data corruption. > > Fingers crossed.
It turns out that crossing fingers wasn't enough and we still get corruption, but less frequently than before. We're going in the right direction. My working theory is that this is due to a subtle bug in the hole punching done by gfs2_iomap_end() to get rid of unused blocks. With the test case that fails, gfs2_iomap_end() is punching holes way more often than I was expecting, and way more often than it should. Remember that the test case is doing 32-MiB writes of a user buffer that usually isn't entirely in memory. The first iomap_file_buffered_write() allocates filesystem blocks for the entire buffer, and when it finds that it could only do a partial write, it frees a large part of those blocks. It will then call fault_in_iov_iter_readable() for the next chunk, and the next call to iomap_file_buffered_write() will then usually be able to write that chunk entirely. So it seems that we should always call fault_in_iov_iter_readable() before calling into iomap_file_buffered_write(). This will probably hide whatever is going wrong in punch_hole(), but we'll get to that later ... (Side note: the chunk size should be aligned to the page cache, not to the iov_iter as in the current code.) > > + truncate_pagecache_range(inode, hstart, hend - 1); > > + if (hstart < hend) > > + punch_hole(ip, hstart, hend - hstart); > > Why doesn't that "hstart < hend" condition cover both the truncate and > the hole punch? That was a leftover from a previous experiment in which I did the truncate_pagecache_range() on the unaligned boundaries. Which turned out to be pointless. I'll clean that up. Thanks, Andreas
