Re: [PATCH] btrfs: statfs: Don't reset f_bavail if we're over committing metadata space

[Anand Jain <anand.jain@oracle.com>]

On 1/31/20 5:05 AM, 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 &&

  Makes sense. I didn't realize we have space_info::full I was
  experimenting create btrfs_calc_avail_data_space() (which we use
  few lines above in this function) for type metadata.

  This issue is easy to reproduce as below.

  mkfs.btrfs -fq -n64K -msingle /dev/sdb && mount /dev/sdb /btrfs && df 
-k /btrfs

  Filesystem     1K-blocks  Used Available Use% Mounted on
/dev/sdb         3072000 13824         0 100% /btrfs

  So xfstests using _get_available_space() with MKFS_OPTS="-m single"
  option is failing.

  Unfortunately, revert of patch 0096420adb03 (btrfs: do not account
  global reserve in can_overcommit) also fixes this, I don't know yet
  how and why yet. Any idea?

  As because whats happening for the test case above, is
  %found->disk_total is 8388608 (for single) but the %block_rsv->size is
  13631488 (for nodesize 64K). So the condition fails. The
  %block_rev->size scales with nodesize, where as  %found->disk_total
  doesn't, so with default nodesize 16K this problem isn't reproducible.

Thanks, Anand


>      total_free_meta - thresh < block_rsv->size)

> Thanks,
> 
> Josef