Re: [PATCH v2] mm: Fix memblock_free_late() when using deferred struct page

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Thu, 2026-02-19 at 12:16 +0200, Mike Rapoport wrote:
> 
> Let's split it. EFI does weird things with memory already, like mremapping
> normal memory for example.

Yup.

> Here's my take on the split. Lightly tested on qemu and recovered ~45M of
> ram with the OVMF version I have :)

Nice :-) I'll test this here.

> > 
> +struct efi_freeable_range {
> +	u64 start;
> +	u64 end;
> +};
> 

Haha, you went the blunt way :-) I was trying to avoid creating yet-
another structure with "start/end" :-) 

> +
> +static struct efi_freeable_range *ranges_to_free;
> +
>  void __init efi_free_boot_services(void)
>  {

I was going to call it efi_unmap_boot_services() to avoid having two
things with almost the same name.

>  	struct efi_memory_map_data data = { 0 };
>  	efi_memory_desc_t *md;
>  	int num_entries = 0;
> +	int idx = 0;
>  	void *new, *new_md;
>  
>  	/* Keep all regions for /sys/kernel/debug/efi */
>  	if (efi_enabled(EFI_DBG))
>  		return;
>  
> +	ranges_to_free = kzalloc(sizeof(*ranges_to_free) * efi.memmap.nr_map,
> +				 GFP_KERNEL);
> +	if (!ranges_to_free) {
> +		pr_err("Failed to allocate storage for freeable EFI regions\n");
> +		return;
> +	}

Do we still want to do the whole unmap dance in that case ? I mean, OOM
here means the system is pretty much a goner at that stage but ...

>  	for_each_efi_memory_desc(md) {
>  		unsigned long long start = md->phys_addr;
>  		unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
> @@ -471,7 +486,15 @@ void __init efi_free_boot_services(void)
>  			start = SZ_1M;
>  		}
>  
> -		memblock_free_late(start, size);
> +		/*
> +		 * With CONFIG_DEFERRED_STRUCT_PAGE_INIT parts of the memory
> +		 * map are still not initialized and we can't reliably free
> +		 * memory here.
> +		 * Queue the ranges to free at a later point.
> +		 */
> +		ranges_to_free[idx].start = start;
> +		ranges_to_free[idx].end = start + size;
> +		idx++;

Do we want to make this conditional to CONFIG_DEFERRED_STRUCT_PAGE_INIT
or we don't care ?

>  	}
>  
>  	if (!num_entries)
> @@ -512,6 +535,23 @@ void __init efi_free_boot_services(void)
>  	}
>  }
>  
> +static int __init efi_free_boot_services_memory(void)
> +{
> +	struct efi_freeable_range *range = ranges_to_free;
> +
> +	while (range->start) {
> +		void *start = phys_to_virt(range->start);
> +		void *end = phys_to_virt(range->end);
> +
> +		free_reserved_area(start, end, -1, NULL);

I assume here too the total_ram_page_inc stuff is taken care of ? I
haven't really looked. This feels like a fragile counter.

> +		range++;
> +	}
> +	kfree(ranges_to_free);
> +
> +	return 0;
> +}
> +late_initcall(efi_free_boot_services_memory);
> +
>  /*
>   * A number of config table entries get remapped to virtual addresses
>   * after entering EFI virtual mode. However, the kexec kernel requires
> 
> base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
> -- 
> 2.51.0
> 
>  
> > Cheers,
> > Ben.
> > 
>