On Wed, Nov 27, 2024 at 08:28:29AM -0800, Darrick J. Wong wrote:
> > @@ -1789,7 +1790,16 @@ static int iomap_add_to_ioend(struct iomap_writepage_ctx *wpc,
> >
> > if (ifs)
> > atomic_add(len, &ifs->write_bytes_pending);
> > +
> > + /*
> > + * If the ioend spans i_size, trim io_size to the former to provide
> > + * the fs with more accurate size information. This is useful for
> > + * completion time on-disk size updates.
>
> I think it's useful to preserve the diagram showing exactly what problem
> you're solving:
>
> /*
> * Clamp io_offset and io_size to the incore EOF so that ondisk
> * file size updates in the ioend completion are byte-accurate.
> * This avoids recovering files with zeroed tail regions when
> * writeback races with appending writes:
> *
> * Thread 1: Thread 2:
> * ------------ -----------
> * write [A, A+B]
> * update inode size to A+B
> * submit I/O [A, A+BS]
> * write [A+B, A+B+C]
> * update inode size to A+B+C
> * <I/O completes, updates disk size to min(A+B+C, A+BS)>
> * <power failure>
> *
> * After reboot:
> * 1) with A+B+C < A+BS, the file has zero padding in range
> * [A+B, A+B+C]
> *
> * |< Block Size (BS) >|
> * |DDDDDDDDDDDD00000000000000|
> * ^ ^ ^
> * A A+B A+B+C
> * (EOF)
> *
> * 2) with A+B+C > A+BS, the file has zero padding in range
> * [A+B, A+BS]
> *
> * |< Block Size (BS) >|< Block Size (BS) >|
> * |DDDDDDDDDDDD00000000000000|000000000000000000000000000|
> * ^ ^ ^ ^
> * A A+B A+BS A+B+C
> * (EOF)
> *
> * D = Valid Data
> * 0 = Zero Padding
> *
> * Note that this defeats the ability to chain the ioends of
> * appending writes.
> */
>
> (I reduced the blocksize a bit for wrapping purposes)
Ok, I will update it.
>
> The logic looks ok, but I'm curious about how you landed at 2.6.12-rc
> for the fixes tag.
>
> --D
I see that io_size was introduced in version 2.6. It's quite difficult
to determine the exact version where the issue was introduced, but I can
confirm it was before version 4.19, as I can reproduce the issue in 4.19.
It should before introduce iomap infrastructure, how about using the
following fix tag?
Fixes: ae259a9c8593 ("fs: introduce iomap infrastructure") # goes further back than this
Thanks,
Long Li
