Re: [PATCH RFC v2 06/19] Add a vmalloc_node_user function

[Dave Chinner <david@fromorbit.com>]

On Mon, Jun 03, 2024 at 09:20:10PM -0700, Christoph Hellwig wrote:
> On Mon, Jun 03, 2024 at 07:24:03PM +0000, Bernd Schubert wrote:
> > void *vmalloc_node(unsigned long size, int node)
> > {
> >         return __vmalloc_node(size, 1, GFP_KERNEL, node,
> >                         __builtin_return_address(0));
> > }
> > 
> > 
> > 
> > 
> > If we wanted to avoid another export, shouldn't we better rename
> > vmalloc_user to vmalloc_node_user, add the node argument and change
> > all callers?
> > 
> > Anyway, I will send the current patch separately to linux-mm and will ask
> > if it can get merged before the fuse patches.
> 
> Well, the GFP flags exist to avoid needing a gazillion of variants of
> everything build around the page allocator.  For vmalloc we can't, as
> Kent rightly said, support GFP_NOFS and GFP_NOIO and need to use the
> scopes instead, and we should warn about that (which __vmalloc doesn't
> and could use some fixes for).

Perhaps before going any further here, we should refresh our
memories on what the vmalloc code actually does these days?
__vmalloc_area_node() does this when mapping the pages:


	/*
         * page tables allocations ignore external gfp mask, enforce it
         * by the scope API
         */
        if ((gfp_mask & (__GFP_FS | __GFP_IO)) == __GFP_IO)
                flags = memalloc_nofs_save();
        else if ((gfp_mask & (__GFP_FS | __GFP_IO)) == 0)
                flags = memalloc_noio_save();

        do {
                ret = vmap_pages_range(addr, addr + size, prot, area->pages,
                        page_shift);
                if (nofail && (ret < 0))
                        schedule_timeout_uninterruptible(1);
        } while (nofail && (ret < 0));

        if ((gfp_mask & (__GFP_FS | __GFP_IO)) == __GFP_IO)
                memalloc_nofs_restore(flags);
        else if ((gfp_mask & (__GFP_FS | __GFP_IO)) == 0)
                memalloc_noio_restore(flags);


IOWs, vmalloc() has obeyed GFP_NOFS/GFP_NOIO constraints properly
for since early 2022 and there isn't a need to wrap it with scopes
just to do a single constrained allocation:

commit 451769ebb7e792c3404db53b3c2a422990de654e
Author: Michal Hocko <mhocko@suse.com>
Date:   Fri Jan 14 14:06:57 2022 -0800

    mm/vmalloc: alloc GFP_NO{FS,IO} for vmalloc
    
    Patch series "extend vmalloc support for constrained allocations", v2.
    
    Based on a recent discussion with Dave and Neil [1] I have tried to
    implement NOFS, NOIO, NOFAIL support for the vmalloc to make life of
    kvmalloc users easier.

.....

    Add support for GFP_NOFS and GFP_NOIO to vmalloc directly.  All internal
    allocations already comply with the given gfp_mask.  The only current
    exception is vmap_pages_range which maps kernel page tables.  Infer the
    proper scope API based on the given gfp mask.
.....

-Dave.

-- 
Dave Chinner
david@fromorbit.com