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@xxxxxxxxxx> >> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxx> >> --- >> 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