On Mon, 2023-07-24 at 13:54 +0800, Huang, Ying wrote:
> Vishal Verma <vishal.l.verma@xxxxxxxxx> writes:
>
> >
> > @@ -2035,12 +2056,38 @@ void try_offline_node(int nid)
> > }
> > EXPORT_SYMBOL(try_offline_node);
> >
> > -static int __ref try_remove_memory(u64 start, u64 size)
> > +static void __ref __try_remove_memory(int nid, u64 start, u64 size,
> > + struct vmem_altmap *altmap)
> > {
> > - struct vmem_altmap mhp_altmap = {};
> > - struct vmem_altmap *altmap = NULL;
> > - unsigned long nr_vmemmap_pages;
> > - int rc = 0, nid = NUMA_NO_NODE;
> > + /* remove memmap entry */
> > + firmware_map_remove(start, start + size, "System RAM");
>
> If mhp_supports_memmap_on_memory(), we will call
> firmware_map_add_hotplug() for whole range. But here we may call
> firmware_map_remove() for part of range. Is it OK?
>
Good point, this is a discrepancy in the add vs remove path. Can the
firmware memmap entries be moved up a bit in the add path, and is it
okay to create these for each memblock? Or should these be for the
whole range? I'm not familiar with the implications. (I've left it as
is for v3 for now, but depending on the direction I can update in a
future rev).
