[0/9,v3] Rework reserve ticket handling

Message

Josef Bacik Aug. 22, 2019, 7:10 p.m. UTC

This is the next round of my reserve ticket refinements.  Most of the changes
are just fixing issues brought up by review.  The updated diffstat is as follows

 fs/btrfs/block-group.c    |   5 +-
 fs/btrfs/block-rsv.c      |  10 +--
 fs/btrfs/delalloc-space.c |   4 --
 fs/btrfs/delayed-ref.c    |   2 +-
 fs/btrfs/extent-tree.c    |  13 +---
 fs/btrfs/space-info.c     | 171 +++++++++++++++++++---------------------------
 fs/btrfs/space-info.h     |  30 +++++---
 7 files changed, 98 insertions(+), 137 deletions(-)

v2->v3:
- added 9/9 to rename btrfs_space_info_add_old_bytes as per discussions with
  Nikolay.
- added a few comments.
- made the logic clearer in the may_commit_transaction patch.
- a few lockdep_assert_held()'s.
- added the reviewed-by's.

v1->v2:
- added "btrfs: fix may_commit_transaction to deal with no partial filling"
- fixed "btrfs: refactor the ticket wakeup code" to return true if we find a
  smaller ticket than our first ticket in the list.

- Original email -
While cleaning up some things around the global reserve and can_overcommit I
started getting ENOSPC's with plenty of space to make reservations.  The root
cause of the problem has to do with how we satisfy ticket reservations.

Previously we would add any space we were returning to the space info to the
first ticket we found.  The reason we did this was because new reservations just
check the counters to see if they can continue, so we didn't want them to get
reservations when we had waiters already queued up.  So instead of returning the
bytes to the space info, I'd add it to the ticket.  Then if we failed to satisfy
that ticket reservation we'd take any space we found and add it to the next guy
in case it satisfied the next ticket reservation.

This works generally well in practice, but there are several xfstests that run
ENOSPC tests against very small file systems.  These tests uncovered a corner
case when it comes to overcommitting.  If we overcommit the space, and then are
no longer allowed to overcommit, we won't actually give any returned space to
the tickets, because that would be really bad.  Instead we return that space to
the space_info and carry on.

What was biting us in these test cases was the fact that we had very small
metadata area, 8mib, and unlink asks for about 2mib of space.  If we had
overcommitted 8.1mib, we'd give back almost 2mib of space to the space_info,
which could have instead been used for the reservation.  This would result in an
early ENOSPC.

Since we are only doing this partial filling dance to avoid racing with new
reservations we just fix that race by checking if we have pending reservations
on the list, closing that race.  Then we are free to use the normal checks to
see if a ticket can be woken up.  This simplifies the code a bunch, we no longer
have to keep track of how much space the tickets were given and return those
bytes, and I could consolidate the wakeup code into one function instead of two.

The diffstat is as follows, this all passes xfstests, and sets us up nicely for
the upcoming changesets.  Thanks,

Josef

Comments

David Sterba Aug. 23, 2019, 12:55 p.m. UTC | #1

On Thu, Aug 22, 2019 at 03:10:53PM -0400, Josef Bacik wrote:
> This is the next round of my reserve ticket refinements.  Most of the changes
> are just fixing issues brought up by review.  The updated diffstat is as follows
> 
>  fs/btrfs/block-group.c    |   5 +-
>  fs/btrfs/block-rsv.c      |  10 +--
>  fs/btrfs/delalloc-space.c |   4 --
>  fs/btrfs/delayed-ref.c    |   2 +-
>  fs/btrfs/extent-tree.c    |  13 +---
>  fs/btrfs/space-info.c     | 171 +++++++++++++++++++---------------------------
>  fs/btrfs/space-info.h     |  30 +++++---
>  7 files changed, 98 insertions(+), 137 deletions(-)

I'll add the series to for-next as topic branch, the comments seem to be
more in the changelog or function names, that I'll updated and fold to
the patches (no need to resend the whole patchset).

We're running out of time before the merge window freeze, so the enospc
updates are probably the last big thing going in, I'll continue with the
remaining patchsets and will try to push them in for-next today.

David Sterba Aug. 28, 2019, 6:02 p.m. UTC | #2

On Fri, Aug 23, 2019 at 02:55:33PM +0200, David Sterba wrote:
> On Thu, Aug 22, 2019 at 03:10:53PM -0400, Josef Bacik wrote:
> > This is the next round of my reserve ticket refinements.  Most of the changes
> > are just fixing issues brought up by review.  The updated diffstat is as follows
> > 
> >  fs/btrfs/block-group.c    |   5 +-
> >  fs/btrfs/block-rsv.c      |  10 +--
> >  fs/btrfs/delalloc-space.c |   4 --
> >  fs/btrfs/delayed-ref.c    |   2 +-
> >  fs/btrfs/extent-tree.c    |  13 +---
> >  fs/btrfs/space-info.c     | 171 +++++++++++++++++++---------------------------
> >  fs/btrfs/space-info.h     |  30 +++++---
> >  7 files changed, 98 insertions(+), 137 deletions(-)
> 
> I'll add the series to for-next as topic branch, the comments seem to be
> more in the changelog or function names, that I'll updated and fold to
> the patches (no need to resend the whole patchset).

Updated and pushed to misc-next.