On Fri, Mar 26, 2021 at 09:57:43AM +0100, Oscar Salvador wrote:

On Fri, Mar 26, 2021 at 09:52:58AM +0100, David Hildenbrand wrote:

quoted

Might have to set fully spanned section online. (vmemmap >= SECTION_SIZE)

Bleh, this morning I was in a rush and I could not think straight.
I got what you mean now.

Yes, if vmemmap pages fully span a section, we need to online that
section before calling online_pages(), otherwise we would left that
section offline.

quoted

We'll not call the memory notifier (e.g., MEM_ONLINE) for the vmemmap. The

Well, since it is not actual memory we might do not need to, but

quoted

1. We won't allocate extended struct pages for the range. Don't think this
is really problematic (pages are never allocated/freed, so I guess we don't
care - like ZONE_DEVICE code).

Probably not worth for vmemmap

quoted

2. We won't allocate kasan shadow memory. We most probably have to do it
explicitly via kasan_add_zero_shadow()/kasan_remove_zero_shadow(), see
mm/memremap.c:pagemap_range()

we should be calling those kasan_{add,remove}.
We can stuff that into the vmemmap helpers, so everything remains
contained.

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?
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?"
        

-- 
Oscar Salvador
SUSE L3