Re: [PATCH 1/3] xfs: simplify extent allocation alignment

Dave Chinner
2024-04-02T23:44:54Z

On Tue, Apr 02, 2024 at 08:49:09AM +0100, John Garry wrote:
>
> > > > the problem should go away and the
> > > > extent gets trimmed to 76 blocks.
> > > ..if so, then, yes, it does. We end up with this:
> > >
> > > 0: [0..14079]: 42432..56511 0 (42432..56511) 14080
> > > 1: [14080..14687]: 177344..177951 0 (177344..177951) 608
> > > 2: [14688..14719]: 350720..350751 1 (171520..171551) 32
> > Good, that's how it should work. 🙂
> >
> > I'll update the patchset I have with these fixes.
>
> ok, thanks
>
> Update:
> So I have some more patches from trying to support both truncate and
> fallocate + punch/insert/collapse for forcealign.
>
> I seem to have at least 2x problems:
> - unexpected -ENOSPC in some case
> - sometimes misaligned extents from ordered combo of punch, truncate, write

Punch and truncate do not enforce extent alignment and never have.
They will trim extents to whatever the new EOF block is supposed to
be. This is by design - they are intended to be able to remove
extents beyond EOF that may have been done due to extent size hints
and/or speculative delayed allocation to minimise wasted space.

Again, forced alignment is introducing new constraints, so anything
that is truncating EOF blocks (punch, truncate, eof block freeing
during inactivation or blockgc) is going to need to take forced
extent alignment constraints into account.

This is likely something that needs to be done in
xfs_itruncate_extents_flags() for the truncate/inactivation/blockgc
cases (i.e. correct calculation of first_unmap_block). Punching and
insert/collapse are a bit more complex - they'll need their
start/end offsets to be aligned, the chunk sizes they work in to be
aligned, and the rounding in xfs_flush_unmap_range() to be forced
alignment aware.

> I don't know if it is a good use of time for me to try to debug, as I guess
> you could spot problems in the changes almost immediately.
>
> Next steps:
> I would like to send out a new series for XFS support for atomic writes
> soon, which so far included forcealign support.
>
> Please advise on your preference for what I should do, like wait for your
> forcealign update and then combine into the XFS support for atomic write
> series. Or just post the doubtful patches now, as above, and go from there?

I just sent out the forced allocation alignment series for review.
Forced truncate/punch extent alignment will need to be implemented
and reviewed as a separate patch set...

-Dave.
--
Dave Chinner
david@fromorbit.com