Re: [PATCH 34/46] hugetlb: userfaultfd: when using MADV_SPLIT, round addresses to PAGE_SIZE

From: Peter Xu
Date: Fri Jan 06 2023 - 10:14:25 EST


On Thu, Jan 05, 2023 at 10:18:32AM +0000, James Houghton wrote:
> MADV_SPLIT enables HugeTLB HGM which allows for UFFDIO_CONTINUE in
> PAGE_SIZE chunks. If a huge-page-aligned address were to be provided,
> userspace would be completely unable to take advantage of HGM. That
> would then require userspace to know to provide
> UFFD_FEATURE_EXACT_ADDRESS.
>
> This patch would make it harder to make a mistake. Instead of requiring
> userspace to provide UFFD_FEATURE_EXACT_ADDRESS, always provide a usable
> address.
>
> Signed-off-by: James Houghton <jthoughton@xxxxxxxxxx>
> ---
> mm/hugetlb.c | 31 +++++++++++++++----------------
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 5af6db52f34e..5b6215e03fe1 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -5936,28 +5936,27 @@ static inline vm_fault_t hugetlb_handle_userfault(struct vm_area_struct *vma,
> unsigned long addr,
> unsigned long reason)
> {
> + u32 hash;
> + struct vm_fault vmf;
> +
> /*
> * Don't use the hpage-aligned address if the user has explicitly
> * enabled HGM.
> */
> if (hugetlb_hgm_advised(vma) && reason == VM_UFFD_MINOR)
> - haddr = address & PAGE_MASK;
> -
> - u32 hash;
> - struct vm_fault vmf = {
> - .vma = vma,
> - .address = haddr,
> - .real_address = addr,
> - .flags = flags,
> + haddr = addr & PAGE_MASK;
>
> - /*
> - * Hard to debug if it ends up being
> - * used by a callee that assumes
> - * something about the other
> - * uninitialized fields... same as in
> - * memory.c
> - */
> - };
> + vmf.vma = vma;
> + vmf.address = haddr;
> + vmf.real_address = addr;
> + vmf.flags = flags;

Const fields here:

mm/hugetlb.c: In function ‘hugetlb_handle_userfault’:
mm/hugetlb.c:5961:17: error: assignment of member ‘vma’ in read-only object
5961 | vmf.vma = vma;
| ^
mm/hugetlb.c:5962:21: error: assignment of member ‘address’ in read-only object
5962 | vmf.address = haddr;
| ^
mm/hugetlb.c:5963:26: error: assignment of member ‘real_address’ in read-only object
5963 | vmf.real_address = addr;

> + /*
> + * Hard to debug if it ends up being
> + * used by a callee that assumes
> + * something about the other
> + * uninitialized fields... same as in
> + * memory.c
> + */

PS: I think we can drop this along the way.

>
> /*
> * vma_lock and hugetlb_fault_mutex must be dropped before handling
> --
> 2.39.0.314.g84b9a713c41-goog
>

--
Peter Xu