| Message ID | 20210420220804.486803-5-axelrasmussen@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | userfaultfd: add minor fault handling for shmem | expand |
On Tue, 20 Apr 2021, Axel Rasmussen wrote: > This patch allows shmem-backed VMAs to be registered for minor faults. > Minor faults are appropriately relayed to userspace in the fault path, > for VMAs with the relevant flag. > > This commit doesn't hook up the UFFDIO_CONTINUE ioctl for shmem-backed > minor faults, though, so userspace doesn't yet have a way to resolve > such faults. > > Acked-by: Peter Xu <peterx@redhat.com> > Signed-off-by: Axel Rasmussen <axelrasmussen@google.com> And if this "04/10" had been numbered 03/10, I would have said Acked-by: Hugh Dickins <hughd@google.com> Just read the comment above: "so userspace doesn't yet have a way to resolve such faults" - if it doesn't by this stage, we're in trouble. > --- > fs/userfaultfd.c | 6 +++--- > include/uapi/linux/userfaultfd.h | 7 ++++++- > mm/memory.c | 8 +++++--- > mm/shmem.c | 12 +++++++++++- > 4 files changed, 25 insertions(+), 8 deletions(-)
On Mon, Apr 26, 2021 at 07:23:57PM -0700, Hugh Dickins wrote: > On Tue, 20 Apr 2021, Axel Rasmussen wrote: > > > This patch allows shmem-backed VMAs to be registered for minor faults. > > Minor faults are appropriately relayed to userspace in the fault path, > > for VMAs with the relevant flag. > > > > This commit doesn't hook up the UFFDIO_CONTINUE ioctl for shmem-backed > > minor faults, though, so userspace doesn't yet have a way to resolve > > such faults. > > > > Acked-by: Peter Xu <peterx@redhat.com> > > Signed-off-by: Axel Rasmussen <axelrasmussen@google.com> > > And if this "04/10" had been numbered 03/10, I would have said > Acked-by: Hugh Dickins <hughd@google.com> > > Just read the comment above: "so userspace doesn't yet have a way to > resolve such faults" - if it doesn't by this stage, we're in trouble. Right, so merging the two patches might be easier. Even if we don't merge them, we'll need to touch up the commit message since at least above paragraph is not true anymore as we've already have UFFDIO_CONTINUE. Thanks,
Ah yes, I should have modified the commit message when I swapped them the first time - sorry for overlooking it. As I said in the other thread, unless someone strongly objects I'll just re-order them the other way around, minor faults first and then CONTINUE, which resolves this concern at least. I'm not too worried about leaving them split. Clearly we'll never release a kernel with one but not the other. So the only scenario I can imagine is, bisecting. But, bisecting across the range where UFFD shmem minor faults were introduced, if you're using that feature, won't really work out well no matter what we do. If you aren't using this feature explicitly, then any of the configurations we've talked about are fine. On Tue, Apr 27, 2021 at 8:57 AM Peter Xu <peterx@redhat.com> wrote: > > On Mon, Apr 26, 2021 at 07:23:57PM -0700, Hugh Dickins wrote: > > On Tue, 20 Apr 2021, Axel Rasmussen wrote: > > > > > This patch allows shmem-backed VMAs to be registered for minor faults. > > > Minor faults are appropriately relayed to userspace in the fault path, > > > for VMAs with the relevant flag. > > > > > > This commit doesn't hook up the UFFDIO_CONTINUE ioctl for shmem-backed > > > minor faults, though, so userspace doesn't yet have a way to resolve > > > such faults. > > > > > > Acked-by: Peter Xu <peterx@redhat.com> > > > Signed-off-by: Axel Rasmussen <axelrasmussen@google.com> > > > > And if this "04/10" had been numbered 03/10, I would have said > > Acked-by: Hugh Dickins <hughd@google.com> > > > > Just read the comment above: "so userspace doesn't yet have a way to > > resolve such faults" - if it doesn't by this stage, we're in trouble. > > Right, so merging the two patches might be easier. Even if we don't merge > them, we'll need to touch up the commit message since at least above paragraph > is not true anymore as we've already have UFFDIO_CONTINUE. Thanks, > > -- > Peter Xu >
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 14f92285d04f..9f3b8684cf3c 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -1267,8 +1267,7 @@ static inline bool vma_can_userfault(struct vm_area_struct *vma, } if (vm_flags & VM_UFFD_MINOR) { - /* FIXME: Add minor fault interception for shmem. */ - if (!is_vm_hugetlb_page(vma)) + if (!(is_vm_hugetlb_page(vma) || vma_is_shmem(vma))) return false; } @@ -1941,7 +1940,8 @@ static int userfaultfd_api(struct userfaultfd_ctx *ctx, /* report all available features and ioctls to userland */ uffdio_api.features = UFFD_API_FEATURES; #ifndef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR - uffdio_api.features &= ~UFFD_FEATURE_MINOR_HUGETLBFS; + uffdio_api.features &= + ~(UFFD_FEATURE_MINOR_HUGETLBFS | UFFD_FEATURE_MINOR_SHMEM); #endif uffdio_api.ioctls = UFFD_API_IOCTLS; ret = -EFAULT; diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h index bafbeb1a2624..159a74e9564f 100644 --- a/include/uapi/linux/userfaultfd.h +++ b/include/uapi/linux/userfaultfd.h @@ -31,7 +31,8 @@ UFFD_FEATURE_MISSING_SHMEM | \ UFFD_FEATURE_SIGBUS | \ UFFD_FEATURE_THREAD_ID | \ - UFFD_FEATURE_MINOR_HUGETLBFS) + UFFD_FEATURE_MINOR_HUGETLBFS | \ + UFFD_FEATURE_MINOR_SHMEM) #define UFFD_API_IOCTLS \ ((__u64)1 << _UFFDIO_REGISTER | \ (__u64)1 << _UFFDIO_UNREGISTER | \ @@ -185,6 +186,9 @@ struct uffdio_api { * UFFD_FEATURE_MINOR_HUGETLBFS indicates that minor faults * can be intercepted (via REGISTER_MODE_MINOR) for * hugetlbfs-backed pages. + * + * UFFD_FEATURE_MINOR_SHMEM indicates the same support as + * UFFD_FEATURE_MINOR_HUGETLBFS, but for shmem-backed pages instead. */ #define UFFD_FEATURE_PAGEFAULT_FLAG_WP (1<<0) #define UFFD_FEATURE_EVENT_FORK (1<<1) @@ -196,6 +200,7 @@ struct uffdio_api { #define UFFD_FEATURE_SIGBUS (1<<7) #define UFFD_FEATURE_THREAD_ID (1<<8) #define UFFD_FEATURE_MINOR_HUGETLBFS (1<<9) +#define UFFD_FEATURE_MINOR_SHMEM (1<<10) __u64 features; __u64 ioctls; diff --git a/mm/memory.c b/mm/memory.c index 4e358601c5d6..cc71a445c76c 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -3972,9 +3972,11 @@ static vm_fault_t do_read_fault(struct vm_fault *vmf) * something). */ if (vma->vm_ops->map_pages && fault_around_bytes >> PAGE_SHIFT > 1) { - ret = do_fault_around(vmf); - if (ret) - return ret; + if (likely(!userfaultfd_minor(vmf->vma))) { + ret = do_fault_around(vmf); + if (ret) + return ret; + } } ret = __do_fault(vmf); diff --git a/mm/shmem.c b/mm/shmem.c index b72c55aa07fc..30c0bb501dc9 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -1785,7 +1785,7 @@ static int shmem_swapin_page(struct inode *inode, pgoff_t index, * vm. If we swap it in we mark it dirty since we also free the swap * entry since a page cannot live in both the swap and page cache. * - * vmf and fault_type are only supplied by shmem_fault: + * vma, vmf, and fault_type are only supplied by shmem_fault: * otherwise they are NULL. */ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index, @@ -1820,6 +1820,16 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index, page = pagecache_get_page(mapping, index, FGP_ENTRY | FGP_HEAD | FGP_LOCK, 0); + + if (page && vma && userfaultfd_minor(vma)) { + if (!xa_is_value(page)) { + unlock_page(page); + put_page(page); + } + *fault_type = handle_userfault(vmf, VM_UFFD_MINOR); + return 0; + } + if (xa_is_value(page)) { error = shmem_swapin_page(inode, index, &page, sgp, gfp, vma, fault_type);