[PATCH 14/21] Fix free/alloc reversal of btree block merge condition
Zach Brown
zab at zabbo.net
Fri Feb 14 12:20:47 PST 2025
On Tue, Feb 11, 2025 at 10:19:11PM +0100, Valerie Aurora wrote:
> To achieve the goal of merging two btree blocks when the merged block
> would be 80% full, we want to merge two blocks when their _used_ space
> is 40% or lower, not when their _free_ space is 40% or lower. Add some
> BUG_ON()'s to catch total_free wrapping around to negative, which was
> the visible symptom in this case.
[ ... ]
> -#define NGNFS_BTREE_MERGE_FREE_THRESH (NGNFS_BTREE_MAX_FREE * 40 / 100)
> +#define NGNFS_BTREE_MERGE_FREE_THRESH (NGNFS_BTREE_MAX_FREE * (100 - 40) / 100)
Yeah, this is great. Maintains the intent while also making the
used/free relationship clear.
> memmove_item_headers(tblk, bt, ind, 1);
> ngnfs_tblk_le16_add_cpu(tblk, &bt->tail_free, -bytes);
> + BUG_ON(le16_to_cpu(bt->tail_free) > NGNFS_BTREE_MAX_FREE);
> ngnfs_tblk_le16_add_cpu(tblk, &bt->total_free, -bytes);
> + BUG_ON(le16_to_cpu(bt->total_free) > NGNFS_BTREE_MAX_FREE);
But this drove me a little bonkers. Let's not sprinkle these amongst
the block changes.
If we're going to have these assertions, let's put them up at the
opening of the function that'd be verifying the caller's promises.
insert_item had the start of that. delete_item didn't but could.
I figured you'd want to tweak this instead of having my mangle the patch
-- but I'm happy to throw it together if you'd rather not. Whatever's
easiest!
- z
More information about the ngnfs-devel
mailing list