Hi, Axel,
On Fri, Jan 15, 2021 at 11:04:47AM -0800, Axel Rasmussen wrote:
> diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> index c63ccdae3eab..7aa1461e1a8b 100644
> --- a/include/linux/userfaultfd_k.h
> +++ b/include/linux/userfaultfd_k.h
> @@ -71,6 +71,11 @@ static inline bool userfaultfd_wp(struct vm_area_struct *vma)
> return vma->vm_flags & VM_UFFD_WP;
> }
>
> +static inline bool userfaultfd_minor(struct vm_area_struct *vma)
> +{
> + return vma->vm_flags & VM_UFFD_MINOR;
> +}
> +
> static inline bool userfaultfd_pte_wp(struct vm_area_struct *vma,
> pte_t pte)
> {
> @@ -85,7 +90,7 @@ static inline bool userfaultfd_huge_pmd_wp(struct vm_area_struct *vma,
>
> static inline bool userfaultfd_armed(struct vm_area_struct *vma)
> {
> - return vma->vm_flags & (VM_UFFD_MISSING | VM_UFFD_WP);
> + return vma->vm_flags & (VM_UFFD_MISSING | VM_UFFD_WP | VM_UFFD_MINOR);
> }
Maybe move the __VM_UFFD_FLAGS into this header so use it too here?
[...]
> diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h
> index 5f2d88212f7c..1cc2cd8a5279 100644
> --- a/include/uapi/linux/userfaultfd.h
> +++ b/include/uapi/linux/userfaultfd.h
> @@ -19,15 +19,19 @@
> * means the userland is reading).
> */
> #define UFFD_API ((__u64)0xAA)
> +#define UFFD_API_REGISTER_MODES (UFFDIO_REGISTER_MODE_MISSING | \
> + UFFDIO_REGISTER_MODE_WP | \
> + UFFDIO_REGISTER_MODE_MINOR)
> #define UFFD_API_FEATURES (UFFD_FEATURE_PAGEFAULT_FLAG_WP | \
> UFFD_FEATURE_EVENT_FORK | \
> UFFD_FEATURE_EVENT_REMAP | \
> - UFFD_FEATURE_EVENT_REMOVE | \
> + UFFD_FEATURE_EVENT_REMOVE | \
> UFFD_FEATURE_EVENT_UNMAP | \
> UFFD_FEATURE_MISSING_HUGETLBFS | \
> UFFD_FEATURE_MISSING_SHMEM | \
> UFFD_FEATURE_SIGBUS | \
> - UFFD_FEATURE_THREAD_ID)
> + UFFD_FEATURE_THREAD_ID | \
> + UFFD_FEATURE_MINOR_FAULT_HUGETLBFS)
I'd remove the "_FAULT" to align with the missing features...
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 61d6346ed009..2b3741d6130c 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4377,6 +4377,37 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm,
> }
> }
>
> + /* Check for page in userfault range. */
> + if (!new_page && userfaultfd_minor(vma)) {
> + u32 hash;
> + struct vm_fault vmf = {
> + .vma = vma,
> + .address = haddr,
> + .flags = flags,
> + /*
> + * 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
> + */
> + };
> +
> + unlock_page(page);
> +
> + /*
> + * hugetlb_fault_mutex and i_mmap_rwsem must be dropped before
> + * handling userfault. Reacquire after handling fault to make
> + * calling code simpler.
> + */
> +
> + hash = hugetlb_fault_mutex_hash(mapping, idx);
> + mutex_unlock(&hugetlb_fault_mutex_table[hash]);
> + i_mmap_unlock_read(mapping);
> + ret = handle_userfault(&vmf, VM_UFFD_MINOR);
> + i_mmap_lock_read(mapping);
> + mutex_lock(&hugetlb_fault_mutex_table[hash]);
> + goto out;
I figured it easier if the whole chunk be put into the else block right after
find_lock_page(); will that work the same?
It's just not obviously clear on when we'll go into this block otherwise,
basically the dependency of new_page variable and when it's unset.
Thanks,
--
Peter Xu
