On Wed, Dec 04, 2024 at 07:17:45AM -0500, Brian Foster wrote: > > Coming back to our current issue, during writeback mapping, we sample > > the inode size to determine if the ioend is within EOF and attempt to > > trim io_size. Concurrent truncate operations may update the inode size, > > causing the pos of write back beyond EOF. In such cases, we simply don't > > trim io_size, which seems like a viable approach. > > > > Perhaps. I'm not claiming it isn't functional. But to Dave's (more > elaborated) point and in light of the racy i_size issue you've > uncovered, what bugs me also about this is that this creates an internal > inconsistency in the submission codepath. > > I.e., the top level code does one thing based on one value of i_size, > then the ioend construction does another, and the logic is not directly > correlated so there is no real guarantee changes in one area correlate > to the other. IME, this increases potential for future bugs and adds > maintenance burden. > > A simple example to consider might be.. suppose sometime in the future > we determine there is a selective case where we do want to allow a > post-eof writeback. As of right now, all that really requires is > adjustment to the "handle_eof()" logic and the rest of the codepath does > the right thing agnostic to outside operations like truncate. I think > there's value if we can preserve that invariant going forward. > > FWIW, I'm not objecting to the alternative if something in the above > reasoning is wrong. I'm just trying to prioritize keeping things simple > and maintainable, particularly since truncate is kind of a complicated > beast as it is. > > Brian > Yes, I agree with you, thanks for the detailed explanation. Thanks, Long Li
