Should we still go __GFP_NOFAIL? (Was Re: [PATCH] btrfs: refactor alloc_extent_buffer() to allocate-then-attach method)

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

On 2023/11/23 06:33, Qu Wenruo wrote:
[...]
>> I wonder if we still can keep the __GFP_NOFAIL for the fallback
>> allocation, it's there right now and seems to work on sysmtems under
>> stress and does not cause random failures due to ENOMEM.
>>
> Oh, I forgot the __NOFAIL gfp flags, that's not hard to fix, just
> re-introduce the gfp flags to btrfs_alloc_page_array().

BTW, I think it's a good time to start a new discussion on whether we
should go __GFP_NOFAIL.
(Although I have updated the patch to keep the GFP_NOFAIL behavior)

I totally understand that we need some memory for tree block during
transaction commitment and other critical sections.

And it's not that uncommon to see __GFP_NOFAIL usage in other mainstream
filesystems.

But my concern is, we also have a lot of memory allocation which can
lead to a lot of problems either, like btrfs_csum_one_bio() or even
join_transaction().

I doubt if btrfs (or any other filesystems) would be to blamed if we're
really running out of memory.
Should the memory hungry user space programs to be firstly killed far
before we failed to allocate memory?


Furthermore, at least for btrfs, I don't think we would hit a situation
where memory allocation failure for metadata would lead to any data
corruption.
The worst case is we hit transaction abort, and the fs flips RO.

Thus I'm wondering if we really need __NOFAIL for btrfs?

Thanks,
Qu
>
> Thanks,
> Qu
>