Re: [PATCH v3 1/5] efi: Always try to allocate heap size of 1.6GB

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2009 bytes --]

On Fri, Mar 13, 2020 at 12:55:08PM +0000, Leif Lindholm wrote:
> My idea was something along the lines of (pseudocode - coding style,
> macro use and naming may or may not be suitable):
> 
>   target_heap_pages = MIN_HEAP_PAGES;
> #ifdef GRUB_LUKS2_ARGON2_ENABLED
>   target_heap_pages += GRUB_LUKS2_ARGON2_PAGES;
> #endif
> 
> and then
> 
>   if (target_heap_pages > (total_pages >> 2)
>     target_heap_pages = total_pages >> 2;
> 
> and then possibly
> 
>   if (target_heap_pages > MAX_HEAP_PAGES)
>     target_heap_pages = MAX_HEAP_PAGES;

What do you think about the following algorithm:

  /* Aim to satisfy requirements posed by both GRUB and other modules.  */
  target_pages = MIN_HEAP_PAGES;
#ifdef GRUB_LUKS2_ARGON2_ENABLED
  target_pages += GRUB_LUKS2_ARGON2_PAGES;
#endif

  total_pages = get_total_pages (filtered_memory_map, desc_size,
				 filtered_memory_map_end);

  /* By default, request a quarter of the available memory.  */
  required_pages = (total_pages >> 2);
  /* But try to satisfy requirements set forth by modules.  */
  required_pages = grub_max (required_pages, target_pages);
  /* And clamp to MIN_HEAP_PAGES <= required_pages <= MAX_HEAP_PAGES.  */
  required_pages = grub_max (required_pages, MIN_HEAP_PAGES);
  required_pages = grub_min (required_pages, MAX_HEAP_PAGES);

In the general case where GRUB_LUKS2_ARGON2_ENABLED is not set, it will
behave exactly the same as before: we request a quarter of available
pages and clamp to MIN_HEAP_PAGES <= required_pages <= MAX_HEAP_PAGES.
If GRUB_LUKS2_ARGON2_ENABLED is set, it will also behave that way,
except for the case where `required_pages < target_pages`, where we just
set `required_pages = target_pages`. As we're still doing the clamping,
we will end up with at most MAX_HEAP_PAGES (1.6GB), which should be
sufficient.

I think this is the least invasive way this can be implemented with
minimal risk: in the general case it is exactly the same as before.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]