Re: [PATCH] mm/hugetlb: restore reservation on error in hugetlb_mfill_atomic_pte() resubmission path

[Muchun Song <muchun.song@linux.dev>]

> On Mar 22, 2026, at 13:21, David Carlier <devnexen@gmail.com> wrote:
> 
> When the resubmission path in hugetlb_mfill_atomic_pte() allocates a new
> hugetlb folio via alloc_hugetlb_folio(), a VMA reservation is consumed. If
> copy_user_large_folio() subsequently fails, folio_put() restores the global
> hugetlb pool count through free_huge_folio(), but the per-VMA reservation
> map entry is left in an inconsistent state.
> 
> Add the missing restore_reserve_on_error() call before folio_put(), matching
> the first-attempt error path which already handles this correctly.
> 
> Fixes: 8cc5fcbb5be8 ("mm, hugetlb: fix racy resv_huge_pages underflow on UFFDIO_COPY")

Hi David,

Thanks for this fix. The patch looks good to me and clearly solves the
reservation leak in the resubmission path of hugetlb_mfill_atomic_pte().

However, I'm a bit curious about the Fixes tag. While commit 8cc5fcbb5be8
did introduce this code structure and the retry path, it seems the bug
wasn't actually introduced there. At that time, copy_huge_page() returned
void, so the failure path simply did not exist.

Instead, looking at the git history, the failure branch `if (ret)` was
added later by commit 1cb9dc4b475c ("mm: hwpoison: support recovery from
HugePage copy-on-write faults"). It modified copy_user_large_folio() to
return an int and introduced error handling paths that unfortunately
missed restoring the reservations. Should the Fixes tag perhaps point to
1cb9dc4b475c instead?

Furthermore, if commit 1cb9dc4b475c is indeed the root cause, I noticed
it also introduced similar error handling paths in other places. For
example, in copy_hugetlb_page_range():

        ret = copy_user_large_folio(new_folio, pte_folio, addr, dst_vma);
        folio_put(pte_folio);
        if (ret) {
                folio_put(new_folio);
                break;
        }

Here, new_folio was allocated with alloc_hugetlb_folio(), which consumes
reservations. But if the copy fails, new_folio is freed via folio_put()
without calling restore_reserve_on_error() first.

Does this imply we might have similar reservation leaks in other error
paths touched by 1cb9dc4b475c? I'd love to hear your thoughts on this.

Thanks,
Muchun

> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
> mm/hugetlb.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 88009cd2a846..d6ea11113f1d 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6295,6 +6295,7 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
> 	folio_put(*foliop);
> 	*foliop = NULL;
> 	if (ret) {
> + 		restore_reserve_on_error(h, dst_vma, dst_addr, folio);
> 		folio_put(folio);
> 		goto out;
> 	}
> -- 
> 2.53.0
>