[RFC PATCH 8/8] HACK: mm: memory_hotplug: Drop memblock_phys_free() call in try_remove_memory()

[Jonathan Cameron]

On Thu, 30 May 2024 12:07:37 +0200
Oscar Salvador <osalvador at suse.com> wrote:

> On Wed, May 29, 2024 at 06:12:36PM +0100, Jonathan Cameron wrote:
> > I'm not sure what this is balancing, but it if is necessary then the reserved
> > memblock approach can't be used to stash NUMA node assignments as after the
> > first add / remove cycle the entry is dropped so not available if memory is
> > re-added at the same HPA.  
> 
> It is balancing previously allocated memory which was allocated via
> memblock_phys_alloc{_range,try_nid}.
> memblock_phys_alloc_try_nid() is who does the heavy-lifting, and also calls
> kmemleak_alloc_phys() to make kmemleak aware of that memory.

Thanks Oscar,

I'm struggling to find the call path that does that for the particular range being
released.

There are some calls to allocate node data but for small amounts of data
whereas I think this call is removing the full range of the hot removed memory.
Is the intent just to get rid of the node data related reserved memblock if
it happens to be in the memory removed?  If so feels like the call should
really be targetting just that.

> 
> A quick idea that came to me is:
> I think that it should be possible 1) create a new memory_block flag
> (check 'enum memblock_flags') and 2) flag the range you want with this
> range (check memblock_setclr_flag()) with a function like
> memblock_reserved_mark_{yourflag}.
> 
> Then, in memblock_phys_free() (or down the path) we could check for that flag,
> and refuse to proceed if it is set.

I had tried a flag in the main memblock rather than using the reserved memblock
a while back and that was horribly invasive so I got a bit scared of touching
those flags. One used only in the reserved memblock might work
to bypass the removal of the reserved memblocks but carry out everything
else that call is intended to do. I'm still sketchy on what I'm bypassing
though!

Thanks,

Jonathan

> 
> Would that work?
> I am not sure, but you might need to 
> 
>