[PATCHv3 5/6] efi_mem_sort: use list_for_each_entry_safe instead

[Ilias Apalodimas]

On Mon, 13 Apr 2026 at 23:36, <rs at ti.com> wrote:
>
> From: Randolph Sapp <rs at ti.com>
>
> Use list_for_each_entry_safe and comparisons against the current and
> next efi_mem_desc. This reduces the computation required for merging
> regions, prevents unnecessary additional iterations of the list, and
> requires less temporary values.
>
> Signed-off-by: Randolph Sapp <rs at ti.com>
> ---

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>

>  lib/efi_loader/efi_memory.c | 48 +++++++++++++------------------------
>  1 file changed, 17 insertions(+), 31 deletions(-)
>
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index b77c2f980cc..b3b292ebf56 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -128,44 +128,30 @@ static uint64_t desc_get_end(struct efi_mem_desc *desc)
>   */
>  static void efi_mem_sort(void)
>  {
> -       struct efi_mem_list *lmem;
> -       struct efi_mem_list *prevmem = NULL;
> -       bool merge_again = true;
> +       struct efi_mem_list *curmem, *nextmem = NULL;
>
>         list_sort(NULL, &efi_mem, efi_mem_cmp);
>
>         /* Now merge entries that can be merged */
> -       while (merge_again) {
> -               merge_again = false;
> -               list_for_each_entry(lmem, &efi_mem, link) {
> -                       struct efi_mem_desc *prev;
> -                       struct efi_mem_desc *cur;
> -                       uint64_t pages;
> +       list_for_each_entry_safe(curmem, nextmem, &efi_mem, link) {
> +               struct efi_mem_desc *cur;
> +               struct efi_mem_desc *next;
>
> -                       if (!prevmem) {
> -                               prevmem = lmem;
> -                               continue;
> -                       }
> +               /* Exit when we've got nothing to compare with */
> +               if (&nextmem->link == &efi_mem) {
> +                       break;
> +               }
>
> -                       cur = &lmem->desc;
> -                       prev = &prevmem->desc;
> -
> -                       if ((desc_get_end(cur) == prev->physical_start) &&
> -                           (prev->type == cur->type) &&
> -                           (prev->attribute == cur->attribute)) {
> -                               /* There is an existing map before, reuse it */
> -                               pages = cur->num_pages;
> -                               prev->num_pages += pages;
> -                               prev->physical_start -= pages << EFI_PAGE_SHIFT;
> -                               prev->virtual_start -= pages << EFI_PAGE_SHIFT;
> -                               list_del(&lmem->link);
> -                               free(lmem);
> -
> -                               merge_again = true;
> -                               break;
> -                       }
> +               cur = &curmem->desc;
> +               next = &nextmem->desc;
>
> -                       prevmem = lmem;
> +               if ((cur->physical_start == desc_get_end(next)) &&
> +                   (cur->type == next->type) &&
> +                   (cur->attribute == next->attribute)) {
> +                       /* There is another similar map coming up, reuse it */
> +                       next->num_pages += cur->num_pages;
> +                       list_del(&curmem->link);
> +                       free(curmem);
>                 }
>         }
>  }
> --
> 2.53.0
>