Re: [PATCH v3 2/6] mm: Allow dynamically requesting additional memory regions

[Daniel Kiper]

On Thu, Sep 02, 2021 at 12:48:24AM +1000, Daniel Axtens wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > Currently, all platforms will set up their heap on initialization of the
> > platform code. While this works mostly fine, it poses some limitations
> > on memory management on us. Most notably, allocating big chunks of
> > memory in the gigabyte range would require us to pre-request this many
> > bytes from the firmware and add it to the heap from the beginning on
> > some platforms like EFI. As this isn't needed for most configurations,
> > it is inefficient and may even negatively impact some usecases when,
> > e.g., chainloading. Nonetheless, allocating big chunks of memory is
> > required sometimes, where one example is the upcoming support for the
> > Argon2 key derival function in LUKS2.
> >
> > In order to avoid pre-allocating big chunks of memory, this commit
> > implements a runtime mechanism to add more pages to the system. When a
> > given allocation cannot be currently satisfied, we'll call a given
> > callback set up by the platform's own memory management subsystem,
> > asking it to add a memory area with at least `n` bytes. If this
> > succeeds, we retry searching for a valid memory region, which should now
> > succeed.
> >
>
> I implemented this for ieee1275-powerpc. I set the initial memory claim
> to 1MB to match EFI and to exercise the code.
>
> Thoughts as I progressed:
>
>  - You probably need to think about how to satisfy requests with
>    particular alignments: currently there is no way to specify that with
>    the current interface, and I saw powerpc-ieee1275 return bunch of
>    allocations at e.g 0x2a561e which is not particularly well aligned!

I think at "firmware memory allocation" level we could always allocate
page aligned regions. Of course this may not satisfy allocations
aligned at larger values than a page. Though I think we can solve this
by allocating larger regions from firmware. Please look below for more
details...

>  - You haven't included in the calculations the extra space required for
>    mm housekeeping. For example, I'm seeing an allocation for 32kB be
>    requested via grub_mm_add_region_fn. I dutifully allocate 32kB, with
>    no alignment requirements, and pass that pointer to
>    grub_mm_init_region. grub_mm_init_region throws away bytes at the
>    start to get to GRUB_MM_ALIGN, then uses some bytes for the
>    grub_mm_header_t, then any actual allocation uses bytes for the
>    malloc metadata. So the actual underlying allocation cannot be
>    satisfied.
>
>    I think you get away with this on EFI because you use BYTES_TO_PAGES
>    and get page-aligned memory, but I think you should probably round up
>    to the next power of 2 for smaller allocations or to the next page or
>    so for larger allocations.

I think we could allocate at least e.g. 128 MiB from firmware if there is
not enough memory available in the GRUB mm. This way we would avoid frequent
calls to firmware and could satisfy requests for larger alignments.

>  - After fixing that in the ieee1275 code, all_functional_test
>    hangs trying to run the cmdline_cat test. I think this is from a slow
>    algorithm somewhere - the grub allocator isn't exactly optimised for
>    a proliferation of regions.

Could you try the solution proposed above? Maybe it will solve problem of
frequent additions of memory to the GRUB mm.

>  - I noticed that nearly all the allocations were under 1MB. This seems
>    inefficient for a trip out to firmware. So I made the ieee1275 code
>    allocate at least max(4MB, (size of allocation rounded up nearest
>    1MB) + 4kB). This makes the tests run with only the usual failures,
>    at least on pseries with debug on... still chasing some bugs beyond
>    that.

Yeah, this is similar to what I proposed above. Though I would want to see
larger numbers tested as I said earlier.

>  - The speed impact depends on the allocation size. I'll post something
>    on that tomorrow, hopefully, but larger minimum allocations work
>    noticably better.
>
>  - We only have 4GB max to play with because (at least) powerpc-ieee1275
>    is technically defined to be 32 bit. So I'm a bit nervous about
>    further large allocations unless we have a way to release them back
>    to _firmware_, not just to grub.

Ugh... This can be difficult. I am not sure the GRUB mm is smart enough
to release memory regions if they are not used anymore by it.

> I would think a better overall approach would be to allocate the 1/4 of
> ram when grub starts, and create a whole new interface for large slabs

I am not very happy with allocating 1/4 of memory at start of the day.
I think allocating larger chunks of memory from firmware should be
enough to make things working as expected.

> of memory that are directly allocated from, and directly returned to,
> the firmware.

Daniel