Re: [PATCH 0/2] ->total_bytes_pinned fixes for early ENOSPC issues

[David Sterba <dsterba@suse.cz>]

On Wed, Oct 21, 2020 at 09:32:24AM -0400, Josef Bacik wrote:
> Hello,
> 
> Nikolay discovered a regression with generic/371 with my delayed refs patches
> applied.  This isn't actually a regression caused by those patches, but rather
> those patches uncovered a problem that has existed forever, we just have papered
> over it with our aggressive delayed ref flushing.  Enter these two patches.
> 
> The first patch is more of a refactoring and a simplification.  We've been
> passing in a &old_refs and a &new_refs into the delayed ref code, and
> duplicating the
> 
> if (old_refs >= 0 && new_refs < 0)
> 	->total_bytes_pinned += bytes;
> else if (old_refs < 0 && new_refs >= 0)
> 	->total_bytes_pinned -= bytes;
> 
> logic for data and metadata.  This was made even more confusing by the fact that
> we clean up this accounting when we drop the delayed ref, but also include it
> when we actually pin the extents down properly.  It took me quite a while to
> realize that we weren't mis-counting ->total_bytes_pinned because of how weirdly
> complicated the accounting was.
> 
> I've refactored this code to make the handling distinct.  We modify it in the
> delayed refs code itself, which allows us to clean up a bunch of function
> arguments and duplicated code.  It also unifies how we handle the delayed ref
> side of the ->total_bytes_pinned modification.  Now it's a little easier to see
> who is responsible for the modification and where.
> 
> The second patch is the actual fix for the problem.  Previously we had simply
> been assuming that ->total_ref_mod < 0 meant that we were freeing stuff.
> However if we allocate and free in the same transaction, ->total_ref_mod == 0
> also means we're freeing.  Adding that case is what fixes the problem Nikolay
> was seeing.  Thanks,
> 
> Josef
> 
> 
> Josef Bacik (2):
>   btrfs: handle ->total_bytes_pinned inside the delayed ref itself
>   btrfs: account for new extents being deleted in total_bytes_pinned

The 2nd patch did not get any review (1st one has from Nikolay), the
patchset hasn't been in any topic branch or for-next. As we're going to
have more space related changes in 5.12 cycle I'd like to get this one
in too, so either please resend or get the remaining review. Thanks.