On Fri, Nov 08, 2024 at 06:55:23AM -0800, Christoph Hellwig wrote: > On Fri, Nov 08, 2024 at 08:27:38PM +0800, Long Li wrote: > > After reboot, file has zero padding in range [A+B, A+B+C]: > > > > |< Block Size (BS) >| > > |DDDDDDDDDDDDDDDD0000000000000000| > > ^ ^ ^ > > A A+B A+B+C (EOF) > > > > D = Valid Data > > 0 = Zero Padding > > > > The issue stems from disk size being set to min(io_offset + io_size, > > inode->i_size) at I/O completion. Since io_offset+io_size is block > > size granularity, it may exceed the actual valid file data size. In > > the case of concurrent append writes, inode->i_size may be larger > > than the actual range of valid file data written to disk, leading to > > inaccurate disk size updates. > > Oh, interesting one. Do you have a reproducer we could wire up > to xfstests? > Yes, I have a simple reproducer, but it would require significant work to incorporate it into xfstestis. > > This patch introduce ioend->io_end to trace the end position of the > > valid data in ioend, rather than solely relying on ioend->io_size. > > It ensures more precise disk size updates and avoids the zero padding > > issue. Another benefit is that it makes the xfs_ioend_is_append() > > check more accurate, which can reduce unnecessary end bio callbacks > > of xfs_end_bio() in certain scenarios, such as repeated writes at the > > file tail without extending the file size. > > Hmm. Can we do away with two members for the size by just rounding > up the block size for the block based operations? > If we only use one size record, we can remove io_size and keep only io_end to record the tail end of valid file data in ioend. Meanwhile, we can add a wrapper function iomep_ioend_iosize() to get the extent size of ioend, replacing the existing ioend->io_size. Would this work? Thanks, Long Li
