Re: [PATCH 5/9] userfaultfd: add minor fault registration mode
From: Peter Xu <peterx@redhat.com>
Date: 2021-01-21 19:33:47
Also in:
linux-mm, lkml
Hi, Axel, On Fri, Jan 15, 2021 at 11:04:47AM -0800, Axel Rasmussen wrote:
quoted hunk ↗ jump to hunk
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? [...]
quoted hunk ↗ jump to hunk
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...
quoted hunk ↗ jump to hunk
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