On Sun, Nov 17, 2024 at 10:56:59PM -0800, Christoph Hellwig wrote:
> On Wed, Nov 13, 2024 at 11:13:49AM -0500, Brian Foster wrote:
> > > static bool
> > > iomap_ioend_can_merge(struct iomap_ioend *ioend, struct iomap_ioend *next)
> > > {
> > > + size_t size = iomap_ioend_extent_size(ioend);
> > > +
> >
> > The function name is kind of misleading IMO because this may not
> > necessarily reflect "extent size." Maybe something like
> > _ioend_size_aligned() would be more accurate..?
>
> Agreed. What also would be useful is a comment describing the
> function and why io_size is not aligned.
>
Ack.
> > 1. It kind of feels like a landmine in an area where block alignment is
> > typically expected. I wonder if a rename to something like io_bytes
> > would help at all with that.
>
> Fine with me.
>
> > Another randomish idea might be to define a flag like
> > IOMAP_F_EOF_TRIMMED for ioends that are trimmed to EOF. Then perhaps we
> > could make an explicit decision not to grow or merge such ioends, and
> > let the associated code use io_size as is.
>
> I don't think such a branch is any cheaper than the rounding..
>
True, but I figured it to be more informational/usable than performance
oriented.
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?
If not, then you could use a flag to be more deliberate/informative that
the ioend was slightly special in that it was modified on submission,
and then adjacent ioends would simply fail to merge on a flag comparison
rather than fail to merge on a contiguity check.
Of course if folks would rather just do the rounding helper thing and
leave it up to the fs to use it, then I don't see any fundamental
problem with that. I just find it kind of a subtle/quirky interface.
Brian
