Re: [PATCH v7 6/7] mm/memory_hotplug: Embed vmem_altmap details in memory block

[Aneesh Kumar K V <aneesh.kumar@linux.ibm.com>]

On 8/2/23 4:40 AM, Verma, Vishal L wrote:
> On Tue, 2023-08-01 at 10:11 +0530, Aneesh Kumar K.V wrote:
>> With memmap on memory, some architecture needs more details w.r.t altmap
>> such as base_pfn, end_pfn, etc to unmap vmemmap memory. Instead of
>> computing them again when we remove a memory block, embed vmem_altmap
>> details in struct memory_block if we are using memmap on memory block
>> feature.
>>
>> Acked-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>>  drivers/base/memory.c  | 27 +++++++++++++--------
>>  include/linux/memory.h |  8 ++----
>>  mm/memory_hotplug.c    | 55 ++++++++++++++++++++++++++----------------
>>  3 files changed, 53 insertions(+), 37 deletions(-)
>>
> <snip>
> 
>> @@ -2136,10 +2148,10 @@ EXPORT_SYMBOL(try_offline_node);
>>  
>>  static int __ref try_remove_memory(u64 start, u64 size)
>>  {
>> -       struct vmem_altmap mhp_altmap = {};
>> -       struct vmem_altmap *altmap = NULL;
>> -       unsigned long nr_vmemmap_pages;
>> +       int ret;
> 
> Minor nit - there is already an 'int rc' below - just use that, or
> rename it to 'ret' if that's better for consistency.
> 


I reused the existing rc variable. 

>> +       struct memory_block *mem;
>>         int rc = 0, nid = NUMA_NO_NODE;
>> +       struct vmem_altmap *altmap = NULL;
>>  
>>         BUG_ON(check_hotplug_memory_range(start, size));
>>  
>> @@ -2161,25 +2173,20 @@ static int __ref try_remove_memory(u64 start, u64 size)
>>          * the same granularity it was added - a single memory block.
>>          */
>>         if (mhp_memmap_on_memory()) {
>> -               nr_vmemmap_pages = walk_memory_blocks(start, size, NULL,
>> -                                                     get_nr_vmemmap_pages_cb);
>> -               if (nr_vmemmap_pages) {
>> +               ret = walk_memory_blocks(start, size, &mem, test_has_altmap_cb);
>> +               if (ret) {
>>                         if (size != memory_block_size_bytes()) {
>>                                 pr_warn("Refuse to remove %#llx - %#llx,"
>>                                         "wrong granularity\n",
>>                                         start, start + size);
>>                                 return -EINVAL;
>>                         }
>> -
>> +                       altmap = mem->altmap;
>>                         /*
>> -                        * Let remove_pmd_table->free_hugepage_table do the
>> -                        * right thing if we used vmem_altmap when hot-adding
>> -                        * the range.
>> +                        * Mark altmap NULL so that we can add a debug
>> +                        * check on memblock free.
>>                          */
>> -                       mhp_altmap.base_pfn = PHYS_PFN(start);
>> -                       mhp_altmap.free = nr_vmemmap_pages;
>> -                       mhp_altmap.alloc = nr_vmemmap_pages;
>> -                       altmap = &mhp_altmap;
>> +                       mem->altmap = NULL;
>>                 }
>>         }
>>  


Thank you for taking the time to review the patch.

-aneesh