Re: [PATCH v4 1/2] mm/memory_hotplug: Add support to accept memory during hot-add

[Kiryl Shutsemau <kas@kernel.org>]

On Wed, Feb 04, 2026 at 09:50:09PM -0600, Pratik R. Sampat wrote:
> 
> 
> On 2/4/26 2:00 PM, David Hildenbrand (arm) wrote:
> >>   #endif
> >>     static inline bool pfn_is_unaccepted_memory(unsigned long pfn)
> >> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> >> index a63ec679d861..549ccfd190ee 100644
> >> --- a/mm/memory_hotplug.c
> >> +++ b/mm/memory_hotplug.c
> >> @@ -1567,6 +1567,8 @@ int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
> >>       if (!strcmp(res->name, "System RAM"))
> >>           firmware_map_add_hotplug(start, start + size, "System RAM");
> >>   +    accept_hotplug_memory(start, size);
> >> +
> >>       /* device_online() will take the lock when calling online_pages() */
> >>       mem_hotplug_done();
> >>   
> > 
> > I really hate that accepting (and un-accepting) hotplugged memory is different to accepting ordinary boot memory.
> > 
> > Is there really no way we can get a reasonable implementation where we just call a generic accept_memory() and it will know what to do?
> > 
> 
> Sure, that shouldn't be impossible.
> 
> The only reason I initially kept them separate is because we accept and update
> the bitmap unconditionally. This mainly applies to cold-plugged memory since
> their bitmap state after remove shouldn't matter. However, as we are now
> correctly setting the bits in the hot-remove path we should be fine accepting
> from the for_each_set_bitrange_from() logic within accept_memory(), I think.
> 
> Something like so?
> 
> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
> index d11e7836200a..e56adfd382f8 100644
> --- a/drivers/firmware/efi/unaccepted_memory.c
> +++ b/drivers/firmware/efi/unaccepted_memory.c
> @@ -36,6 +36,7 @@ void accept_memory(phys_addr_t start, unsigned long size)
>         unsigned long range_start, range_end;
>         struct accept_range range, *entry;
>         phys_addr_t end = start + size;
> +       phys_addr_t bitmap_end;
>         unsigned long flags;
>         u64 unit_size;
> 
> @@ -44,6 +45,21 @@ void accept_memory(phys_addr_t start, unsigned long size)
>                 return;
> 
>         unit_size = unaccepted->unit_size;
> +       bitmap_end = unaccepted->phys_base + unaccepted->size * unit_size * BITS_PER_BYTE;
> +
> +       /* Memory completely beyond bitmap: hotplug memory, accept unconditionally */
> +       if (start >= bitmap_end) {
> +               arch_accept_memory(start, end);
> +               return;
> +       }
> +
> +       /* Memory partially beyond bitmap */
> +       if (end > bitmap_end) {
> +               arch_accept_memory(bitmap_end, end);
> +               end = bitmap_end;
> +       }

You are calling arch_accept_memory() on every memory allocation if the
memory is not represented in the bitmap. Hard NAK.

> 
>         /*
>          * Only care for the part of the range that is represented
> 
> unaccept_hotplug_memory() truly doesn't do anything special for hotplug so I
> could just re-name it unaccept_memory().
> 
> Thanks!
> 

-- 
  Kiryl Shutsemau / Kirill A. Shutemov