Re: [PATCH 4/9] btrfs: rework btrfs_space_info_add_old_bytes

[David Sterba <dsterba@suse.cz>]

On Fri, Aug 23, 2019 at 10:48:35AM +0300, Nikolay Borisov wrote:
> 
> 
> On 22.08.19 г. 22:10 ч., Josef Bacik wrote:
> > If there are pending tickets and we are overcommitted we will simply
> > return free'd reservations to space_info->bytes_may_use if we cannot
> > overcommit any more.  This is problematic because we assume any free
> > space would have been added to the tickets, and so if we go from an
> > overcommitted state to not overcommitted we could have plenty of space
> > in our space_info but be unable to make progress on our tickets because
> > we only refill tickets from previous reservations.
> > 
> > Consider the following example.  We were allowed to overcommit to
> > space_info->total_bytes + 2mib.  Now we've allocated all of our chunks
> > and are no longer allowed to overcommit those extra 2mib.  Assume there
> > is a 3mib reservation we are now freeing.  Because we can no longer
> > overcommit we do not partially refill the ticket with the 3mib, instead
> > we subtract it from space_info->bytes_may_use.  Now the total reserved
> > space is 1mib less than total_bytes, meaning we have 1mib of space we
> > could reserve.  Now assume that our ticket is 2mib, and we only have
> > 1mib of space to reclaim, so we have a partial refilling to 1mib.  We
> > keep trying to flush and eventually give up and ENOSPC the ticket, when
> > there was the remaining 1mib left in the space_info for usage.
> 
> The wording of this paragraph makes it a bit hard to understand. How
> about something like:

I got lost there too. It's hard too keep track of what changes,
something a bit more strucutred could help understanding it.

Also the subject is too generic, I'd suggest "update overcommit logic
when refilling tickets" or something like that. Using 'rework' or
'revamp' in the subject applies to very few patches.