On Thu, Jan 14, 2021 at 11:38 PM Michal Hocko mhocko@suse.com wrote:
On Thu 14-01-21 21:47:36, Muchun Song wrote:
On Thu, Jan 14, 2021 at 9:20 PM Michal Hocko mhocko@suse.com wrote:
[...]
@@ -1770,6 +1789,28 @@ int dissolve_free_huge_page(struct page *page) int nid = page_to_nid(head); if (h->free_huge_pages - h->resv_huge_pages == 0) goto out;
/** We should make sure that the page is already on the free list* when it is dissolved.*/if (unlikely(!PageHugeFreed(head))) {spin_unlock(&hugetlb_lock);/** Theoretically, we should return -EBUSY when we* encounter this race. In fact, we have a chance* to successfully dissolve the page if we do a* retry. Because the race window is quite small.* If we seize this opportunity, it is an optimization* for increasing the success rate of dissolving page.*/while (PageHeadHuge(head) && !PageHugeFreed(head))cond_resched();Sorry, I should have raised that when replying to the previous version already but we have focused more on other things. Is there any special reason that you didn't simply if (!PageHugeFreed(head)) { spin_unlock(&hugetlb_lock); cond_resched(); goto retry; }
This would be less code and a very slight advantage would be that the waiter might get blocked on the spin lock while the concurrent freeing is happening. But maybe you wanted to avoid exactly this contention? Please put your thinking into the changelog.
I want to avoid the lock contention. I will add this reason to the changelog. Thanks.
Please also explain why it matters and whether an unintended contention is a real problem.
I have no idea about this, it is just my opinion. I will follow your suggestion.
-- Michal Hocko SUSE Labs