Re: [RFC PATCH 1/5] mm: intorduce __GFP_UNMAPPED and unmapped_alloc()

[Song Liu <song@kernel.org>]

On Fri, May 19, 2023 at 1:30 AM Mike Rapoport <rppt@kernel.org> wrote:
>
> Hi Kent,
>
> On Thu, May 18, 2023 at 01:23:56PM -0400, Kent Overstreet wrote:
> > On Thu, May 18, 2023 at 10:00:39AM -0700, Song Liu wrote:
> > > On Thu, May 18, 2023 at 9:48 AM Kent Overstreet
> > > <kent.overstreet@linux.dev> wrote:
> > > >
> > > > On Thu, May 18, 2023 at 09:33:20AM -0700, Song Liu wrote:
> > > > > I am working on patches based on the discussion in [1]. I am planning to
> > > > > send v1 for review in a week or so.
> > > >
> > > > Hey Song, I was reviewing that thread too,
> > > >
> > > > Are you taking a different approach based on Thomas's feedback? I think
> > > > he had some fair points in that thread.
> > >
> > > Yes, the API is based on Thomas's suggestion, like 90% from the discussions.
> > >
> > > >
> > > > My own feeling is that the buddy allocator is our tool for allocating
> > > > larger variable sized physically contiguous allocations, so I'd like to
> > > > see something based on that - I think we could do a hybrid buddy/slab
> > > > allocator approach, like we have for regular memory allocations.
> > >
> > > I am planning to implement the allocator based on this (reuse
> > > vmap_area logic):
> >
> > Ah, you're still doing vmap_area approach.
> >
> > Mike's approach looks like it'll be _much_ lighter weight and higher
> > performance, to me. vmalloc is known to be slow compared to the buddy
> > allocator, and with Mike's approach we're only modifying mappings once
> > per 2 MB chunk.
> >
> > I don't see anything in your code for sub-page sized allocations too, so
> > perhaps I should keep going with my slab allocator.
>
> Your allocator implicitly relies on vmalloc because of module_alloc ;-)
>
> What I was thinking is that we can replace module_alloc() calls in your
> allocator with something based on my unmapped_alloc(). If we make the part
> that refills the cache also take care of creating the mapping in the
> module address space, that should cover everything.

Here are what I found as I work more on the code:

1. It takes quite some work to create a clean interface and make sure
    all the users of module_alloc can use the new allocator on all archs.
    (archs with text poke need to work with ROX memory from the
    allocator; archs without text poke need to have a clean fall back
    mechanism, etc.). Most of this work is independent of the actual
    allocator, so we can do this part and plug in whatever allocator we
    want (buddy+slab or vmap-based or any other solutions).

2. vmap_area based solution will work. And it will be one solution for
    both < PAGE_SIZE and > PAGE_SIZE allocations. Given
    module_alloc is not in any hot path (AFAIK), I don't see any
    practical issues with this solution. It will be a little tricky to place
    and name the code, as it uses vmalloc logic, but it is technically a
    module allocator.

I will prioritize building the interface, and migrating users to it. If we
do this part right, changing the underlying allocator should be
straightforward.

Thanks,
Song