Re: [PATCH 15/33] userfaultfd: hugetlbfs: add __mcopy_atomic_hugetlb for huge page UFFDIO_COPY

["Hillf Danton" <hillf.zj@alibaba-inc.com>]

On Tuesday, November 22, 2016 9:17 AM Mike Kravetz wrote:
> I am not sure if you are convinced ClearPagePrivate is an acceptable
> solution to this issue.  If you do, here is the simple patch to add
> it and an appropriate comment.
> 
Hi Mike and Andrea

Sorry for my jumping in.

In commit 07443a85ad
("mm, hugetlb: return a reserved page to a reserved pool if failed")
newly allocated huge page gets cleared for a successful COW.

I'm wondering if we can handle our error path along that way?

Obvious I could miss the points you are concerning.

thanks
Hillf
> 
> If __mcopy_atomic_hugetlb exits with an error, put_page will be called
> if a huge page was allocated and needs to be freed.  If a reservation
> was associated with the huge page, the PagePrivate flag will be set.
> Clear PagePrivate before calling put_page/free_huge_page so that the
> global reservation count is not incremented.
> 
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
>  mm/userfaultfd.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index b565481..d56ba83 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -303,8 +303,23 @@ static __always_inline ssize_t __mcopy_atomic_hugetlb(struct mm_struct *dst_mm,
>  out_unlock:
>  	up_read(&dst_mm->mmap_sem);
>  out:
> -	if (page)
> +	if (page) {
> +		/*
> +		 * We encountered an error and are about to free a newly
> +		 * allocated huge page.  It is possible that there was a
> +		 * reservation associated with the page that has been
> +		 * consumed.  See the routine restore_reserve_on_error
> +		 * for details.  Unfortunately, we can not call
> +		 * restore_reserve_on_error now as it would require holding
> +		 * mmap_sem.  Clear the PagePrivate flag so that the global
> +		 * reserve count will not be incremented in free_huge_page.
> +		 * The reservation map will still indicate the reservation
> +		 * was consumed and possibly prevent later page allocation.
> +		 * This is better than leaking a global reservation.
> +		 */
> +		ClearPagePrivate(page);
>  		put_page(page);
> +	}
>  	BUG_ON(copied < 0);
>  	BUG_ON(err > 0);
>  	BUG_ON(!copied && !err);
> --
> 2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>