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

Anand Jain
2020-01-30T23:14:25Z

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