quoted

quoted

Further a locking rework might be necessary. We hold the device hotplug
lock, but not the memory hotplug lock. E.g., for get_online_mems(). Might
have to move that out online_pages.

That is a good point.
I yet have to think about it further, but what about moving those
mem_hotplug_{begin,done} to memory_block_{offline,online}.

Something like:

  static int memory_block_online(...)
  {
          int ret;
  
          mem_hotplug_begin();
  
          if (nr_vmemmap_pages)
                  vmemmap_hotplug_init();
  
          ret = online_pages(...);
          if (ret)
  	/*
  	 * Cleanup stuff
  	 */
  
          mem_hotplug_done();
  }
	
As you said, you finished cleaning up those users who were calling
{online,offline}_pages() directly, but is this something that we will
forbidden in the future too?

Well, I cannot tell what will happen in the future. But at least as long 
as we have memory blocks, I doubt that there are valid use cases for 
online_pages()/offline_pages(). Especially once we have things like 
memmap_on_memory that need special care.

My question falls within "Are we sure we will not need that locking back
in those functions because that is something we will not allow to
happen?"

Who has access to online_pages()/offline_pages() also has access to 
mem_hotplug_begin()/mem_hotplug_done(). It would be nice if we could 
only use online_pages()/offline_pages() internally -- or at least add a 
comment that this is for internal purposes only / requires that locking.

-- 
Thanks,

David / dhildenb