[PATCH 0/6] xfs: bunmapi needs updating for deferred freeing

Dave Chinner
2021-05-27T04:52:11Z

Hi folks,

I pulled on a loose thread when I started looking into the 64kB
directory block size assert failure I was seeing while trying to
test the bulk page allocation changes.

I posted the first patch in the series separately - it fixed the
immediate assert failure (5.13-rc1 regression) I was seeing, but in
fixing that it only then dropped back to the previous assert failure
that g/538 was triggering with 64kb directory block sizes. This can
only be reproduced on 5.12, because that's when the error injection
that g/538 uses was added. So I went looking deeper.

It turns out that xfs_bunmapi() has some code in it to avoid locking
AGFs in the wrong order and this is what was triggering. Many of the
xfs_bunmapi() callers can not/do not handle partial unmaps that
return success, and that's what the directory code is tripping over
trying to free badly fragmented directory blocks.

This AGF locking order constraint was added to xfs_bunmapu in 2017
to avoid a deadlock in g/299. Sad thing is that shortly after this,
we converted xfs-bunmapi to use deferred freeing, so it never
actually locks AGFs anymore. But the deadlock avoiding landmine
remained. And xfs_bmap_finish() went away, too, and we now only ever
put one extent in any EFI we log for deferred freeing.

That means we now only free one extent per transaction via deferred
freeing, and there are no limitations on what order xfs_bunmapi()
can unmap extents. 64kB directories on a 1kB block size filesystem
already unmap 64 extents in a single loop, so there's no real
limitation here.

This means that the limitations of how many extents we can unmap per
loop in xfs_itruncate_extents_flags() goes away for data device
extents (and will eventually go away for RT devices, too, when
Darrick's RT EFI stuff gets merged).

This "one data deveice extent free per transaction" change now means
that all of the transaction reservations that include
"xfs_bmap_finish" based freeing reservations are wrong. These extent
frees are now done by deferred freeing, and so they only need a
single extent free reservation instead of up to 4 (as truncate was
reserving).

This series fixes the btree fork regression, the bunmapi partial
unmap regression from 2017, extends xfs_itruncate_extents to unmap
64 extents at a time for data device (AG) resident extents, and
reworks the transaction reservations to use a consistent and correct
reservation for allocation and freeing extents. The size of some
transaction reservations drops dramatically as a result.

The first two patches are -rcX candidates, the rest are for the next
merge cycle....

Cheers,

Dave.