Re: [PATCH v2 1/2] iomap: fix zero padding data issue in concurrent append writes

[Christoph Hellwig <hch@infradead.org>]

On Wed, Nov 20, 2024 at 08:50:45AM -0500, Brian Foster wrote:
> On Wed, Nov 20, 2024 at 01:05:16AM -0800, Christoph Hellwig wrote:
> > On Mon, Nov 18, 2024 at 09:26:45AM -0500, Brian Foster wrote:
> > > IOW following the train of thought in the other subthread, would any
> > > practical workload be affected if we just trimmed io_size when needed by
> > > i_size and left it at that?
> > 
> > Can you explain what you mean with that? 
> > 
> 
> I mean what happens if we trimmed io_size just as this patch already
> does, but left off the rounding stuff?

Ah.

> 
> The rounding is only used for adding to or attempting to merge ioends. I
> don't see when it would ever really make a difference in adding folios
> to an ioend, since we don't writeback folios beyond i_size and a size
> extending operation is unlikely to write back between the time an ioend
> is being constructed/trimmed and submitted.

True.

> After discussion, it seems there are some scenarios where the rounding
> allows i_size trimmed ioends to merge, but again I'm not seeing how that
> can occur frequently enough such that just skipping the rounding and
> letting trimmed ioends fail to merge would have any noticeable
> performance impact.

Agreed.

> But anyways, I think we've reached compromise that pulling the rounding
> helper into buffered-io.c and documenting it well preserves logic and
> addresses my concerns by making sure it doesn't proliferate for the time
> being.

I mean if we can do away without the rounding I'm fine with it.

Looking at fs/iomap/buffered-io.c I think we should be fine with
it, and in XFS everything but the size updaste itself converts to
FSBs first, so it should be fine as well.