Re: block allocations for the refcount btree

Darrick J. Wong
2016-03-02T05:24:47Z

On Tue, Mar 01, 2016 at 12:40:13PM -0800, Christoph Hellwig wrote:
> On Tue, Mar 01, 2016 at 10:18:09AM -0800, Darrick J. Wong wrote:
> > One side effect of the per-ag block reservation code is that it reserves all
> > the blocks that the refcountbt will ever need at mount time, which includes
> > decreasing the incore fdblocks counter at mount and putting it back at unmount
> > time. This /should/ eliminate the need for reserving blocks in truncate
> > transactions, though clearly this isn't being done correctly.
>
> We're still accouting these blocks in t_blk_res_used through
> xfs_alloc_vextent -> xfs_alloc_ag_vextent -> xfs_trans_mod_sb.
>
> I don't really see how the reservation code changes anything about
> that accounting. It just ensures the allocation will succeed through
> xfs_ag_resv_needed in xfs_alloc_ag_vextent, and then removes the
> allocated block from the reservation using xfs_ag_resv_alloc_block.
>
> Maybe we need to find a way to not account for these blocks.
>
> > So what I'm saying is that I think this problem was with the AGresv code not
> > doing accounting correctly, and that I've fixed it in a subsequent rewrite of
> > the AGresv code. I'll post it later, after I figure out why generic/333
> > regresses with the new code.
>
> Ok, let's see if the new version helps with the above issue.
>
> > However, there's one thing to be aware of -- if the AGresv uses up all the
> > blocks that were preallocated at mount time, the allocator will grab any free
> > blocks available and charge the blocks to the transaction, just like before.
> > If this ever happens (in theory we reserve enough blocks so that we can have a
> > refcount record for every block in the AG) then we'll still have this problem.
>
> It seems like we should simply avoid that this case ever happens.

I've rebased my trees and pushed them all to github.

The for-dave-for-4.6 kernel and progs branches are the giant piles of patches
against Dave's for-next integration trees which (I think) are being reviewed
for 4.6.

The for-dave branches are against upstream as they've always been.

New patches have been added on the end of the patchset.

I noticed that generic/139 crashes for-dave with a 1k block size due something
or other sending us bio->bi_bdev == NULL. This seems to be sorted out somehow
in for-next. Other than that I haven't seen any problems... but I've only
run against x64 on bare XFS. Will run other arches/NFS/etc tonight/tomorrow.

The transaction block reservation complaints should be fixed now, and I
think the transaction reservations have been fixed too... or at least they
don't show up on the tinydisk test setup. But all that means is that someone
else will find it, probably within the first 3 minutes of testing. :P

kernel:
for-dave-for-4.6: 31bb2360d45ae3920244a808caffd500ad85f2b6 -> b07b3e650891dc3bd439c5c296bfc22b90c1b9a5
for-dave: fc77dbd34c5c99bce46d40a2491937c3bcbd10af -> 163e86bdb2a2dc1ebb91867a247b4e671298fde7

xfsprogs:
for-dave-for-4.6: c062cfe7dbc568ec9ec308ef6a1bf4ef5cd9af2a -> b147a7ea86bf1f3697cea3bfa7909197ee374aba
for-dave: 1abecdabbc9190db2fa7c3dab15931b7b9ddbb0d -> cbda21d47a9ecde446660bb8ffd808e2ac008554

--D