Re: [PATCH] btrfs: refactor alloc_extent_buffer() to allocate-then-attach method

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

On 2023/11/23 01:08, David Sterba wrote:
> On Wed, Nov 22, 2023 at 10:05:04AM +1030, Qu Wenruo wrote:
>> -	for (i = 0; i < num_pages; i++, index++) {
>> -		p = find_or_create_page(mapping, index, GFP_NOFS|__GFP_NOFAIL);
>> -		if (!p) {
>> -			exists = ERR_PTR(-ENOMEM);
>> -			btrfs_free_subpage(prealloc);
>> -			goto free_eb;
>> +	/* Alloc all pages. */
>> +	ret = btrfs_alloc_page_array(num_pages, eb->pages);
>
> This looks promising.  Assuming everything else works, this can be
> changed to do:
>
> - alloc_pages(order), the optimistic fast path, contig pages right from
>    the allocator, can fail
> - fall back to btrfs_alloc_page_array(), this fills the array with
>    order-0 pages, similar what we have now, they could be contiguous

Yep, that's the next step planned.

Although I'd prefer to fully migrate to folio interfaces, e.g.
eb::pages[] -> eb::folios[]

But that's not really a big deal considering under most cases
folio/pages are interchangeable.

>
> 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().

Thanks,
Qu