On 2/19/21 2:14 AM, Oscar Salvador wrote:

On Fri, Feb 19, 2021 at 10:56:42AM +0100, Michal Hocko wrote:

quoted

OK, this should work but I am really wondering whether it wouldn't be
just simpler to replace the old page by a new one in the free list
directly. Or is there any reason we have to go through the generic
helpers path? I mean something like this

	new_page = alloc_fresh_huge_page();
	if (!new_page)
		goto fail;
	spin_lock(hugetlb_lock);
	if (!PageHuge(old_page)) {

Yes, something like this should work.  I'll let Oscar work out the details.
One thing to note is that you also need to check for old_page not on the
free list here.  It could have been allocated and in use.  In addition,
make sure to check the new flag HPageFreed to ensure page is on free list
before doing any type of update_and_free_page call.
-- 
Mike Kravetz

quoted

		/* freed from under us, nothing to do */ 
		__update_and_free_page(new_page);
		goto unlock;
	}
	list_del(&old_page->lru);
	__update_and_free_page(old_page);
	__enqueue_huge_page(new_page);
unlock:
	spin_unlock(hugetlb_lock);

This will require to split update_and_free_page and enqueue_huge_page to
counters independent parts but that shouldn't be a big deal. But it will
also protect from any races. Not an act of beauty but seems less hackish
to me.

Yes, I think this would to the trick, and it is race-free.
Let me play with it a bit and see what I can come up with.

Thanks for the valuable insight.