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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2443 bytes --]



On 2020/1/31 上午8:35, Qu Wenruo wrote:
> 
> 
> On 2020/1/31 上午5:05, 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)
> 
> block-rsv->space_info->full is not reliable afaik.

My bad, I should double check the code before writing bullsh**.

That full is set as long as we can't allocate a new chunk for that profile.

So that fix is pretty good.

I'll update the patch to use Josef's credit.

Thanks,
Qu

> 
> For metadata we will never really reach full space info.
> 
> Anyway, the patch is already discarded since it doesn't make sense if
> the patch can't reach v5.5.
> 
> The proper fix in the per-profile will be the fix.
> 
> Thanks,
> Qu
> 
>>
>> Thanks,
>>
>> Josef
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]