Re: [PATCH RFC] btrfs: allow extent buffer helpers to skip cross-page handling

[David Sterba <dsterba@suse.cz>]

On Wed, Nov 22, 2023 at 07:07:10AM +1030, Qu Wenruo wrote:
> On 2023/11/22 02:05, David Sterba wrote:
> > On Tue, Nov 21, 2023 at 06:55:35AM +1030, Qu Wenruo wrote:
> >>>> @@ -3562,6 +3563,14 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
> > No, what I say is that alloc_pages would be the fast path and
> > optimization if there's enough memory, otherwise allocation page-by-page
> > would happen as a fallback in case of fragmentation.
> 
> That's also my understanding.
> 
> But the counter argument is still there, if you really think after some
> uptime there would be no contig pages, then the fast path will never
> trigger, and all fall back to page-by-page routine, thus defeating any
> changes to introduce any patch like this one.

Such state is not permanent and memory management tries to coalesce the
freed pages under the costly order back to bigger chunks. So in a system
under heavy load the fragmentation can become bad, we would be ready for
that. It would have to be very bad luck for a long time not to be able
to get any higher order allocation at all. The process stacks depend on
contig allocations 16K, slabs are backed by 2x4K pages, it wouldn't be
just us depending on that.

> > The idea is to try hard when allocating the extent buffers, with
> > fallbacks and potentially slower code but with the same guarantees as
> > now at least.
> >
> > But as it is now, alloc_pages can't be used as replacement due to how
> > the pages are obtained, find_or_create_page(). Currently I don't see a
> > way how to convince folios to allocate the full nodesize range (with a
> > given order) and then get the individual pages.
> 
> I'm doing a preparation to separate the memory allocation for metadata
> page allocation.
> 
> The current patch is to do the allocation first, then attach the new
> pages to the address space of btree inode.
> 
> By this, we can easily add new method of allocation, like trying higher
> order folio first, if failed then page by page allocation.

Right, that would work and probably be the easiest way how to switch to
folios.

> But unfortunately there is a big problem here, even if we can forget the
> argument on whether we can get contig pages after some uptime:
> 
> - My local tests can still sometimes lockup due to some phantom locked
>    pages

I don't know how your patches do it, it could be that the page is not
attached the same way as find_or_create_page/pagecache_get_page/__filemap_get_folio
would do.

So do you want this patch (adding the contig detection and eb::addr)
applied?