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

["Pratik R. Sampat" <prsampat@amd.com>]

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;
+       }

        /*
         * 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!