On Thu, Jan 30, 2020 at 04:05:10PM -0500, Josef Bacik wrote:
> On 1/14/20 10:41 PM, Qu Wenruo wrote:
> > [BUG]
> > When there are a lot of metadata space reserved, e.g. after balancing a
> > data block with many extents, vanilla df would report 0 available space.
> >
> > [CAUSE]
> > btrfs_statfs() would report 0 available space if its metadata space is
> > exhausted.
> > And the calculation is based on currently reserved space vs on-disk
> > available space, with a small headroom as buffer.
> > When there is not enough headroom, btrfs_statfs() will report 0
> > available space.
> >
> > The problem is, since commit ef1317a1b9a3 ("btrfs: do not allow
> > reservations if we have pending tickets"), we allow btrfs to over commit
> > metadata space, as long as we have enough space to allocate new metadata
> > chunks.
> >
> > This makes old calculation unreliable and report false 0 available space.
> >
> > [FIX]
> > Don't do such naive check anymore for btrfs_statfs().
> > Also remove the comment about "0 available space when metadata is
> > exhausted".
> >
> > Please note that, this is a just a quick fix. There are still a lot of
> > things to be improved.
> >
> > Fixes: ef1317a1b9a3 ("btrfs: do not allow reservations if we have pending tickets")
>
> This isn't the patch that broke it. The patch that broke it is the patch that
> introduced this code in the first place.
>
> And this isn't the proper fix either, because technically we have 0 available if
> we don't have enough space for our global reserve _and_ we don't have any
> unallocated space. So for now the best "quick" fix would be to make the
> condition something like
>
> if (!mixed && block-rsv->space_info->full &&
> total_free_meta - thresh < block_rsv->size)
Yes, that seems to be the missing part of my patch that added the above
check. In the testcase there was no remaining metadata space so the
->full == 1 was implied.
