Re: [PATCH] btrfs: don't update the block group item if used bytes are the same

[Josef Bacik <josef@toxicpanda.com>]

On Mon, Jul 11, 2022 at 02:37:52PM +0800, Qu Wenruo wrote:
> When committing a transaction, we will update block group items for all
> dirty block groups.
> 
> But in fact, dirty block groups don't always need to update their block
> group items.
> It's pretty common to have a metadata block group which experienced
> several CoW operations, but still have the same amount of used bytes.
> 
> In that case, we may unnecessarily CoW a tree block doing nothing.
> 
> This patch will introduce btrfs_block_group::commit_used member to
> remember the last used bytes, and use that new member to skip
> unnecessary block group item update.
> 
> This would be more common for large fs, which metadata block group can
> be as large as 1GiB, containing at most 64K metadata items.
> 
> In that case, if CoW added and the deleted one metadata item near the end
> of the block group, then it's completely possible we don't need to touch
> the block group item at all.
> 
> I don't have any benchmark to prove this, but this should not cause any
> hurt either.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

I've been seeing random btrfs check failures on our overnight testing since this
patch was merged.  I can't blame it directly yet, I've mostly seen it on
TEST_DEV, and once while running generic/648.  I'm running it in a loop now to
reproduce and then fix it.

We can start updating block groups before we're in the critical section, so we
can update block_group->bytes_used while we're updating the block group item in
a different thread.  So if we set the block_group item to some value of
bytes_used, then update it in another thread, and then set ->commit_used to the
new value we'll fail to update the block group item with the correct value
later.

We need to wrap this bit in the block_group->lock to avoid this particular
problem.  Once I reproduce and validate the fix I'll send that, but I wanted to
reply in case that takes longer than I expect.  Thanks,

Josef