Re: [PATCH mm-unstable v18 11/14] mm/khugepaged: Introduce mTHP collapse support

On 2026/6/2 18:58, Nico Pache wrote:

On Sun, May 31, 2026 at 1:19 AM Lance Yang [off-list ref] wrote:

quoted

On Fri, May 22, 2026 at 09:00:06AM -0600, Nico Pache wrote:
[...]

quoted

@@ -1587,10 +1749,11 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
       if (result == SCAN_SUCCEED) {
               /* collapse_huge_page expects the lock to be dropped before calling */
               mmap_read_unlock(mm);
-              result = collapse_huge_page(mm, start_addr, referenced,
-                                          unmapped, cc, HPAGE_PMD_ORDER);
-              /* collapse_huge_page will return with the mmap_lock released */
+              nr_collapsed = mthp_collapse(mm, vma, start_addr, referenced,
+                                           unmapped, cc, enabled_orders);
+              /* mmap_lock was released above, set lock_dropped */
               *lock_dropped = true;
+              result = nr_collapsed ? SCAN_SUCCEED : SCAN_FAIL;

Hmm ... don't we lose the allocation-failure result here?

Previously collapse_scan_pmd() propagated SCAN_ALLOC_HUGE_PAGE_FAIL from
collapse_huge_page(), so khugepaged would call khugepaged_alloc_sleep()
in khugepaged_do_scan().

Now if allocation fails and nr_collapsed stays 0, we just return
SCAN_FAIL. So we won't back off via khugepaged_alloc_sleep() anymore?

Ok I did the error propagation! I think I handled both of these cases
you brought up pretty easily.

Thanks.

However I don't know what to do in the following case: We successfully
collapsed some portion of the PMD, but during that process, we also
hit an allocation failure. Is it best to back off entirely? or can we
treat some forward progress as a sign we can continue trying collapses
without sleeping.

Basically, do we prioritize SCAN_ALLOC_HUGE_PAGE_FAIL or the
successful collapses as the returned value?

Thinking out loud, forward progress should win here, the allocation
failure only matter if we made no progress at all?

This is what I currently have:
done:
     if (collapsed)
         return SCAN_SUCCEED;
     if (alloc_failed)
         return SCAN_ALLOC_HUGE_PAGE_FAIL;

I'd go with this ordering :)

Cheers, Lance