| Message ID | 20220619233449.181323-3-namit@vmware.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | userfaultfd: support access/write hints | expand |
On 20.06.22 01:34, Nadav Amit wrote: > From: Nadav Amit <namit@vmware.com> > > Using a PTE on x86 with cleared access-bit (aka young-bit) > takes ~600 cycles more than when the access bit is set. At the same > time, setting the access-bit for memory that is not used (e.g., > prefetched) can introduce greater overheads, as the prefetched memory is > reclaimed later than it should be. > > Userfaultfd currently does not set the access-bit (excluding the > huge-pages case). Arguably, it is best to let the user control whether > the access bit should be set or not. The expected use is to request > userfaultfd to set the access-bit when the copy/wp operation is done to > resolve a page-fault, and not to set the access-bit when the memory is > prefetched. > > Introduce UFFDIO_COPY_MODE_ACCESS_LIKELY and > UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY to enable userspace to request > the young bit to be set. Set for UFFDIO_CONTINUE and UFFDIO_ZEROPAGE the > bit unconditionally since the former is only used to resolve page-faults > and the latter would not benefit from not setting the access-bit. > > Cc: Mike Kravetz <mike.kravetz@oracle.com> > Cc: Hugh Dickins <hughd@google.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Axel Rasmussen <axelrasmussen@google.com> > Cc: Peter Xu <peterx@redhat.com> > Cc: David Hildenbrand <david@redhat.com> > Cc: Mike Rapoport <rppt@linux.ibm.com> > Signed-off-by: Nadav Amit <namit@vmware.com> > --- > fs/userfaultfd.c | 23 ++++++++++++++++------- > include/linux/userfaultfd_k.h | 1 + > include/uapi/linux/userfaultfd.h | 20 +++++++++++++++++++- > mm/userfaultfd.c | 18 ++++++++++++++++-- > 4 files changed, 52 insertions(+), 10 deletions(-) > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 5daafa54eb3f..35a8c4347c54 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -1700,7 +1700,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx, > struct uffdio_copy uffdio_copy; > struct uffdio_copy __user *user_uffdio_copy; > struct userfaultfd_wake_range range; > - bool mode_wp; > + bool mode_wp, mode_access_likely; > uffd_flags_t uffd_flags; > > user_uffdio_copy = (struct uffdio_copy __user *) arg; > @@ -1726,12 +1726,15 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx, > ret = -EINVAL; > if (uffdio_copy.src + uffdio_copy.len <= uffdio_copy.src) > goto out; > - if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP)) > + if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP| > + UFFDIO_COPY_MODE_ACCESS_LIKELY)) > goto out; > > mode_wp = uffdio_copy.mode & UFFDIO_COPY_MODE_WP; > + mode_access_likely = uffdio_copy.mode & UFFDIO_COPY_MODE_ACCESS_LIKELY; I *relly* prefer just if (uffdio_copy.mode & UFFDIO_COPY_MODE_ACCESS_LIKELY) uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY [...] > > - uffd_flags = (mode_wp ? UFFD_FLAGS_WP : 0); > + uffd_flags = (mode_wp ? UFFD_FLAGS_WP : 0) | > + (mode_access_likely ? UFFD_FLAGS_ACCESS_LIKELY : 0); > Dito. > if (mmget_not_zero(ctx->mm)) { > ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start, > @@ -1871,6 +1877,7 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg) > struct uffdio_continue uffdio_continue; > struct uffdio_continue __user *user_uffdio_continue; > struct userfaultfd_wake_range range; > + uffd_flags_t uffd_flags; > > user_uffdio_continue = (struct uffdio_continue __user *)arg; > > @@ -1898,10 +1905,12 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg) > if (uffdio_continue.mode & ~UFFDIO_CONTINUE_MODE_DONTWAKE) > goto out; > > + uffd_flags = UFFD_FLAGS_ACCESS_LIKELY; Can we add a comment why that makes sense? I think I know why -- someone is stuck waiting for that continue to happen :) > + > if (mmget_not_zero(ctx->mm)) { > ret = mcopy_continue(ctx->mm, uffdio_continue.range.start, > uffdio_continue.range.len, > - &ctx->mmap_changing, 0); > + &ctx->mmap_changing, uffd_flags); > mmput(ctx->mm); > } else { > return -ESRCH; > diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h > index 6331148023c1..e6ac165ec044 100644 > --- a/include/linux/userfaultfd_k.h > +++ b/include/linux/userfaultfd_k.h > @@ -58,6 +58,7 @@ enum mcopy_atomic_mode { > typedef unsigned int __bitwise uffd_flags_t; > > #define UFFD_FLAGS_WP ((__force uffd_flags_t)BIT(0)) > +#define UFFD_FLAGS_ACCESS_LIKELY ((__force uffd_flags_t)BIT(1)) > > extern int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd, > struct vm_area_struct *dst_vma, > diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h > index 005e5e306266..d9c8ce9ba777 100644 > --- a/include/uapi/linux/userfaultfd.h > +++ b/include/uapi/linux/userfaultfd.h > @@ -38,7 +38,8 @@ > UFFD_FEATURE_MINOR_HUGETLBFS | \ > UFFD_FEATURE_MINOR_SHMEM | \ > UFFD_FEATURE_EXACT_ADDRESS | \ > - UFFD_FEATURE_WP_HUGETLBFS_SHMEM) > + UFFD_FEATURE_WP_HUGETLBFS_SHMEM | \ > + UFFD_FEATURE_ACCESS_HINTS) > #define UFFD_API_IOCTLS \ > ((__u64)1 << _UFFDIO_REGISTER | \ > (__u64)1 << _UFFDIO_UNREGISTER | \ > @@ -203,6 +204,10 @@ struct uffdio_api { > * > * UFFD_FEATURE_WP_HUGETLBFS_SHMEM indicates that userfaultfd > * write-protection mode is supported on both shmem and hugetlbfs. > + * > + * UFFD_FEATURE_ACCESS_HINTS indicates that the copy supports > + * UFFDIO_COPY_MODE_ACCESS_LIKELY supports > + * UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY. I think that sentence doesn't make sense. > */ > #define UFFD_FEATURE_PAGEFAULT_FLAG_WP (1<<0) > #define UFFD_FEATURE_EVENT_FORK (1<<1) > @@ -217,6 +222,7 @@ struct uffdio_api { > #define UFFD_FEATURE_MINOR_SHMEM (1<<10) > #define UFFD_FEATURE_EXACT_ADDRESS (1<<11) > #define UFFD_FEATURE_WP_HUGETLBFS_SHMEM (1<<12) > +#define UFFD_FEATURE_ACCESS_HINTS (1<<13) > __u64 features; > > __u64 ioctls; > @@ -260,6 +266,13 @@ struct uffdio_copy { > * copy_from_user will not read the last 8 bytes. > */ > __s64 copy; > + /* > + * UFFDIO_COPY_MODE_ACCESS_LIKELY will set the mapped page as young. Setting the page young is an implementation detail. Can you phrase it more generically what the effect of that hint might be? > + * This can reduce the time that the first access to the page takes. > + * Yet, if set opportunistically to memory that is not used, it might > + * extend the time before the unused memory pages are reclaimed. > + */ > +#define UFFDIO_COPY_MODE_ACCESS_LIKELY ((__u64)1<<3) > }; > > struct uffdio_zeropage { > @@ -284,6 +297,10 @@ struct uffdio_writeprotect { > * UFFDIO_WRITEPROTECT_MODE_DONTWAKE: set the flag to avoid waking up > * any wait thread after the operation succeeds. > * > + * UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY: set the flag to mark the modified > + * memory as young, which can reduce the time that the first access > + * to the page takes. Dito. > + * > * NOTE: Write protecting a region (WP=1) is unrelated to page faults, > * therefore DONTWAKE flag is meaningless with WP=1. Removing write > * protection (WP=0) in response to a page fault wakes the faulting > @@ -291,6 +308,7 @@ struct uffdio_writeprotect { > */ > #define UFFDIO_WRITEPROTECT_MODE_WP ((__u64)1<<0) > #define UFFDIO_WRITEPROTECT_MODE_DONTWAKE ((__u64)1<<1) > +#define UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY ((__u64)1<<2) > __u64 mode; > }; [...] > @@ -691,6 +699,9 @@ ssize_t mfill_zeropage(struct mm_struct *dst_mm, unsigned long start, > unsigned long len, atomic_t *mmap_changing, > uffd_flags_t uffd_flags) > { > + /* There is no cost for setting the access bit of a zeropage */ > + uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY; > + > return __mcopy_atomic(dst_mm, start, 0, len, MCOPY_ATOMIC_ZEROPAGE, > mmap_changing, 0); > } > @@ -699,6 +710,9 @@ ssize_t mcopy_continue(struct mm_struct *dst_mm, unsigned long start, > unsigned long len, atomic_t *mmap_changing, > uffd_flags_t uffd_flags) > { > + /* The page is likely to be accessed */ > + uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY; Shoouldn't that be set by the caller already? > + > return __mcopy_atomic(dst_mm, start, 0, len, MCOPY_ATOMIC_CONTINUE, > mmap_changing, 0); > } In general, LGTM.
On Tue, Jun 21, 2022 at 10:48:51AM +0200, David Hildenbrand wrote: > > @@ -1871,6 +1877,7 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg) > > struct uffdio_continue uffdio_continue; > > struct uffdio_continue __user *user_uffdio_continue; > > struct userfaultfd_wake_range range; > > + uffd_flags_t uffd_flags; > > > > user_uffdio_continue = (struct uffdio_continue __user *)arg; > > > > @@ -1898,10 +1905,12 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg) > > if (uffdio_continue.mode & ~UFFDIO_CONTINUE_MODE_DONTWAKE) > > goto out; > > > > + uffd_flags = UFFD_FLAGS_ACCESS_LIKELY; > > Can we add a comment why that makes sense? I think I know why -- someone > is stuck waiting for that continue to happen :) I think we shouldn't apply it by default for CONTINUE at all, at least not sololy for CONTINUE. CONTINUE can be used similarly as COPY at least in VM migration use case, afaict, so we can proactively install pgtables even if the page was not faulted. [...] > > diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h > > index 005e5e306266..d9c8ce9ba777 100644 > > --- a/include/uapi/linux/userfaultfd.h > > +++ b/include/uapi/linux/userfaultfd.h > > @@ -38,7 +38,8 @@ > > UFFD_FEATURE_MINOR_HUGETLBFS | \ > > UFFD_FEATURE_MINOR_SHMEM | \ > > UFFD_FEATURE_EXACT_ADDRESS | \ > > - UFFD_FEATURE_WP_HUGETLBFS_SHMEM) > > + UFFD_FEATURE_WP_HUGETLBFS_SHMEM | \ > > + UFFD_FEATURE_ACCESS_HINTS) Is the access_hint feature gonna cover the next dirty bit patch? If so I'd suggest we add the feature declaration in a separate patch after all bits ready. Thanks,
On Jun 21, 2022, at 1:48 AM, David Hildenbrand <david@redhat.com> wrote: > ⚠ External Email > > On 20.06.22 01:34, Nadav Amit wrote: >> From: Nadav Amit <namit@vmware.com> >> >> Using a PTE on x86 with cleared access-bit (aka young-bit) >> takes ~600 cycles more than when the access bit is set. At the same >> time, setting the access-bit for memory that is not used (e.g., >> prefetched) can introduce greater overheads, as the prefetched memory is >> reclaimed later than it should be. >> >> Userfaultfd currently does not set the access-bit (excluding the >> huge-pages case). Arguably, it is best to let the user control whether >> the access bit should be set or not. The expected use is to request >> userfaultfd to set the access-bit when the copy/wp operation is done to >> resolve a page-fault, and not to set the access-bit when the memory is >> prefetched. >> >> Introduce UFFDIO_COPY_MODE_ACCESS_LIKELY and >> UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY to enable userspace to request >> the young bit to be set. Set for UFFDIO_CONTINUE and UFFDIO_ZEROPAGE the >> bit unconditionally since the former is only used to resolve page-faults >> and the latter would not benefit from not setting the access-bit. >> >> Cc: Mike Kravetz <mike.kravetz@oracle.com> >> Cc: Hugh Dickins <hughd@google.com> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: Axel Rasmussen <axelrasmussen@google.com> >> Cc: Peter Xu <peterx@redhat.com> >> Cc: David Hildenbrand <david@redhat.com> >> Cc: Mike Rapoport <rppt@linux.ibm.com> >> Signed-off-by: Nadav Amit <namit@vmware.com> >> --- >> fs/userfaultfd.c | 23 ++++++++++++++++------- >> include/linux/userfaultfd_k.h | 1 + >> include/uapi/linux/userfaultfd.h | 20 +++++++++++++++++++- >> mm/userfaultfd.c | 18 ++++++++++++++++-- >> 4 files changed, 52 insertions(+), 10 deletions(-) >> >> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c >> index 5daafa54eb3f..35a8c4347c54 100644 >> --- a/fs/userfaultfd.c >> +++ b/fs/userfaultfd.c >> @@ -1700,7 +1700,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx, >> struct uffdio_copy uffdio_copy; >> struct uffdio_copy __user *user_uffdio_copy; >> struct userfaultfd_wake_range range; >> - bool mode_wp; >> + bool mode_wp, mode_access_likely; >> uffd_flags_t uffd_flags; >> >> user_uffdio_copy = (struct uffdio_copy __user *) arg; >> @@ -1726,12 +1726,15 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx, >> ret = -EINVAL; >> if (uffdio_copy.src + uffdio_copy.len <= uffdio_copy.src) >> goto out; >> - if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP)) >> + if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP| >> + UFFDIO_COPY_MODE_ACCESS_LIKELY)) >> goto out; >> >> mode_wp = uffdio_copy.mode & UFFDIO_COPY_MODE_WP; >> + mode_access_likely = uffdio_copy.mode & UFFDIO_COPY_MODE_ACCESS_LIKELY; > > I *relly* prefer just > > if (uffdio_copy.mode & UFFDIO_COPY_MODE_ACCESS_LIKELY) > uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY > [...] > >> - uffd_flags = (mode_wp ? UFFD_FLAGS_WP : 0); >> + uffd_flags = (mode_wp ? UFFD_FLAGS_WP : 0) | >> + (mode_access_likely ? UFFD_FLAGS_ACCESS_LIKELY : 0); > > Dito. > >> if (mmget_not_zero(ctx->mm)) { >> ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start, >> @@ -1871,6 +1877,7 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg) >> struct uffdio_continue uffdio_continue; >> struct uffdio_continue __user *user_uffdio_continue; >> struct userfaultfd_wake_range range; >> + uffd_flags_t uffd_flags; >> >> user_uffdio_continue = (struct uffdio_continue __user *)arg; >> >> @@ -1898,10 +1905,12 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg) >> if (uffdio_continue.mode & ~UFFDIO_CONTINUE_MODE_DONTWAKE) >> goto out; >> >> + uffd_flags = UFFD_FLAGS_ACCESS_LIKELY; > > Can we add a comment why that makes sense? I think I know why -- someone > is stuck waiting for that continue to happen :) > >> + >> if (mmget_not_zero(ctx->mm)) { >> ret = mcopy_continue(ctx->mm, uffdio_continue.range.start, >> uffdio_continue.range.len, >> - &ctx->mmap_changing, 0); >> + &ctx->mmap_changing, uffd_flags); >> mmput(ctx->mm); >> } else { >> return -ESRCH; >> diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h >> index 6331148023c1..e6ac165ec044 100644 >> --- a/include/linux/userfaultfd_k.h >> +++ b/include/linux/userfaultfd_k.h >> @@ -58,6 +58,7 @@ enum mcopy_atomic_mode { >> typedef unsigned int __bitwise uffd_flags_t; >> >> #define UFFD_FLAGS_WP ((__force uffd_flags_t)BIT(0)) >> +#define UFFD_FLAGS_ACCESS_LIKELY ((__force uffd_flags_t)BIT(1)) >> >> extern int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd, >> struct vm_area_struct *dst_vma, >> diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h >> index 005e5e306266..d9c8ce9ba777 100644 >> --- a/include/uapi/linux/userfaultfd.h >> +++ b/include/uapi/linux/userfaultfd.h >> @@ -38,7 +38,8 @@ >> UFFD_FEATURE_MINOR_HUGETLBFS | \ >> UFFD_FEATURE_MINOR_SHMEM | \ >> UFFD_FEATURE_EXACT_ADDRESS | \ >> - UFFD_FEATURE_WP_HUGETLBFS_SHMEM) >> + UFFD_FEATURE_WP_HUGETLBFS_SHMEM | \ >> + UFFD_FEATURE_ACCESS_HINTS) >> #define UFFD_API_IOCTLS \ >> ((__u64)1 << _UFFDIO_REGISTER | \ >> (__u64)1 << _UFFDIO_UNREGISTER | \ >> @@ -203,6 +204,10 @@ struct uffdio_api { >> * >> * UFFD_FEATURE_WP_HUGETLBFS_SHMEM indicates that userfaultfd >> * write-protection mode is supported on both shmem and hugetlbfs. >> + * >> + * UFFD_FEATURE_ACCESS_HINTS indicates that the copy supports >> + * UFFDIO_COPY_MODE_ACCESS_LIKELY supports >> + * UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY. > > I think that sentence doesn't make sense. English… :) How about * UFFD_FEATURE_ACCESS_HINTS indicates that the ioctl operations * supports the UFFDIO_*_MODE_[ACCESS|WRITE]_LIKELY and * UFFDIO_*_MODE_[ACCESS|WRITE]_LIKELY hints. But that would mean that for consistency, I would need to provide zero/continue hints (which might be disregarded by the kernel)? >> */ >> #define UFFD_FEATURE_PAGEFAULT_FLAG_WP (1<<0) >> #define UFFD_FEATURE_EVENT_FORK (1<<1) >> @@ -217,6 +222,7 @@ struct uffdio_api { >> #define UFFD_FEATURE_MINOR_SHMEM (1<<10) >> #define UFFD_FEATURE_EXACT_ADDRESS (1<<11) >> #define UFFD_FEATURE_WP_HUGETLBFS_SHMEM (1<<12) >> +#define UFFD_FEATURE_ACCESS_HINTS (1<<13) >> __u64 features; >> >> __u64 ioctls; >> @@ -260,6 +266,13 @@ struct uffdio_copy { >> * copy_from_user will not read the last 8 bytes. >> */ >> __s64 copy; >> + /* >> + * UFFDIO_COPY_MODE_ACCESS_LIKELY will set the mapped page as young. > > Setting the page young is an implementation detail. Can you phrase it > more generically what the effect of that hint might be? Err. I forgot to fix it before sending. How about: * UFFDIO_COPY_MODE_ACCESS_LIKELY provides a hint to the kernel * that the page is likely to be access in the near future. Providing * the hint properly can improve performance. ? > >> @@ -691,6 +699,9 @@ ssize_t mfill_zeropage(struct mm_struct *dst_mm, unsigned long start, >> unsigned long len, atomic_t *mmap_changing, >> uffd_flags_t uffd_flags) >> { >> + /* There is no cost for setting the access bit of a zeropage */ >> + uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY; >> + >> return __mcopy_atomic(dst_mm, start, 0, len, MCOPY_ATOMIC_ZEROPAGE, >> mmap_changing, 0); >> } >> @@ -699,6 +710,9 @@ ssize_t mcopy_continue(struct mm_struct *dst_mm, unsigned long start, >> unsigned long len, atomic_t *mmap_changing, >> uffd_flags_t uffd_flags) >> { >> + /* The page is likely to be accessed */ >> + uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY; > > Shoouldn't that be set by the caller already? I thought that it belongs conceptually to mm/userfaultfd and not fs/userfaultfd. I will wait for Axel input as to how to handle the CONTINUE case and fix it accordingly.
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 5daafa54eb3f..35a8c4347c54 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -1700,7 +1700,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx, struct uffdio_copy uffdio_copy; struct uffdio_copy __user *user_uffdio_copy; struct userfaultfd_wake_range range; - bool mode_wp; + bool mode_wp, mode_access_likely; uffd_flags_t uffd_flags; user_uffdio_copy = (struct uffdio_copy __user *) arg; @@ -1726,12 +1726,15 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx, ret = -EINVAL; if (uffdio_copy.src + uffdio_copy.len <= uffdio_copy.src) goto out; - if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP)) + if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP| + UFFDIO_COPY_MODE_ACCESS_LIKELY)) goto out; mode_wp = uffdio_copy.mode & UFFDIO_COPY_MODE_WP; + mode_access_likely = uffdio_copy.mode & UFFDIO_COPY_MODE_ACCESS_LIKELY; - uffd_flags = mode_wp ? UFFD_FLAGS_WP : 0; + uffd_flags = (mode_wp ? UFFD_FLAGS_WP : 0) | + (mode_access_likely ? UFFD_FLAGS_ACCESS_LIKELY : 0); if (mmget_not_zero(ctx->mm)) { ret = mcopy_atomic(ctx->mm, uffdio_copy.dst, uffdio_copy.src, @@ -1816,7 +1819,7 @@ static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx, struct uffdio_writeprotect uffdio_wp; struct uffdio_writeprotect __user *user_uffdio_wp; struct userfaultfd_wake_range range; - bool mode_wp, mode_dontwake; + bool mode_wp, mode_dontwake, mode_access_likely; uffd_flags_t uffd_flags; if (atomic_read(&ctx->mmap_changing)) @@ -1834,16 +1837,19 @@ static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx, return ret; if (uffdio_wp.mode & ~(UFFDIO_WRITEPROTECT_MODE_DONTWAKE | - UFFDIO_WRITEPROTECT_MODE_WP)) + UFFDIO_WRITEPROTECT_MODE_WP | + UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY)) return -EINVAL; mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP; mode_dontwake = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_DONTWAKE; + mode_access_likely = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY; if (mode_wp && mode_dontwake) return -EINVAL; - uffd_flags = (mode_wp ? UFFD_FLAGS_WP : 0); + uffd_flags = (mode_wp ? UFFD_FLAGS_WP : 0) | + (mode_access_likely ? UFFD_FLAGS_ACCESS_LIKELY : 0); if (mmget_not_zero(ctx->mm)) { ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start, @@ -1871,6 +1877,7 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg) struct uffdio_continue uffdio_continue; struct uffdio_continue __user *user_uffdio_continue; struct userfaultfd_wake_range range; + uffd_flags_t uffd_flags; user_uffdio_continue = (struct uffdio_continue __user *)arg; @@ -1898,10 +1905,12 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg) if (uffdio_continue.mode & ~UFFDIO_CONTINUE_MODE_DONTWAKE) goto out; + uffd_flags = UFFD_FLAGS_ACCESS_LIKELY; + if (mmget_not_zero(ctx->mm)) { ret = mcopy_continue(ctx->mm, uffdio_continue.range.start, uffdio_continue.range.len, - &ctx->mmap_changing, 0); + &ctx->mmap_changing, uffd_flags); mmput(ctx->mm); } else { return -ESRCH; diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h index 6331148023c1..e6ac165ec044 100644 --- a/include/linux/userfaultfd_k.h +++ b/include/linux/userfaultfd_k.h @@ -58,6 +58,7 @@ enum mcopy_atomic_mode { typedef unsigned int __bitwise uffd_flags_t; #define UFFD_FLAGS_WP ((__force uffd_flags_t)BIT(0)) +#define UFFD_FLAGS_ACCESS_LIKELY ((__force uffd_flags_t)BIT(1)) extern int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd, struct vm_area_struct *dst_vma, diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h index 005e5e306266..d9c8ce9ba777 100644 --- a/include/uapi/linux/userfaultfd.h +++ b/include/uapi/linux/userfaultfd.h @@ -38,7 +38,8 @@ UFFD_FEATURE_MINOR_HUGETLBFS | \ UFFD_FEATURE_MINOR_SHMEM | \ UFFD_FEATURE_EXACT_ADDRESS | \ - UFFD_FEATURE_WP_HUGETLBFS_SHMEM) + UFFD_FEATURE_WP_HUGETLBFS_SHMEM | \ + UFFD_FEATURE_ACCESS_HINTS) #define UFFD_API_IOCTLS \ ((__u64)1 << _UFFDIO_REGISTER | \ (__u64)1 << _UFFDIO_UNREGISTER | \ @@ -203,6 +204,10 @@ struct uffdio_api { * * UFFD_FEATURE_WP_HUGETLBFS_SHMEM indicates that userfaultfd * write-protection mode is supported on both shmem and hugetlbfs. + * + * UFFD_FEATURE_ACCESS_HINTS indicates that the copy supports + * UFFDIO_COPY_MODE_ACCESS_LIKELY supports + * UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY. */ #define UFFD_FEATURE_PAGEFAULT_FLAG_WP (1<<0) #define UFFD_FEATURE_EVENT_FORK (1<<1) @@ -217,6 +222,7 @@ struct uffdio_api { #define UFFD_FEATURE_MINOR_SHMEM (1<<10) #define UFFD_FEATURE_EXACT_ADDRESS (1<<11) #define UFFD_FEATURE_WP_HUGETLBFS_SHMEM (1<<12) +#define UFFD_FEATURE_ACCESS_HINTS (1<<13) __u64 features; __u64 ioctls; @@ -260,6 +266,13 @@ struct uffdio_copy { * copy_from_user will not read the last 8 bytes. */ __s64 copy; + /* + * UFFDIO_COPY_MODE_ACCESS_LIKELY will set the mapped page as young. + * This can reduce the time that the first access to the page takes. + * Yet, if set opportunistically to memory that is not used, it might + * extend the time before the unused memory pages are reclaimed. + */ +#define UFFDIO_COPY_MODE_ACCESS_LIKELY ((__u64)1<<3) }; struct uffdio_zeropage { @@ -284,6 +297,10 @@ struct uffdio_writeprotect { * UFFDIO_WRITEPROTECT_MODE_DONTWAKE: set the flag to avoid waking up * any wait thread after the operation succeeds. * + * UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY: set the flag to mark the modified + * memory as young, which can reduce the time that the first access + * to the page takes. + * * NOTE: Write protecting a region (WP=1) is unrelated to page faults, * therefore DONTWAKE flag is meaningless with WP=1. Removing write * protection (WP=0) in response to a page fault wakes the faulting @@ -291,6 +308,7 @@ struct uffdio_writeprotect { */ #define UFFDIO_WRITEPROTECT_MODE_WP ((__u64)1<<0) #define UFFDIO_WRITEPROTECT_MODE_DONTWAKE ((__u64)1<<1) +#define UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY ((__u64)1<<2) __u64 mode; }; diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c index 734de6aa0b8e..140c8d3e946e 100644 --- a/mm/userfaultfd.c +++ b/mm/userfaultfd.c @@ -92,6 +92,9 @@ int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd, */ _dst_pte = pte_wrprotect(_dst_pte); + if (uffd_flags & UFFD_FLAGS_ACCESS_LIKELY) + _dst_pte = pte_mkyoung(_dst_pte); + dst_pte = pte_offset_map_lock(dst_mm, dst_pmd, dst_addr, &ptl); if (vma_is_shmem(dst_vma)) { @@ -202,7 +205,8 @@ static int mcopy_atomic_pte(struct mm_struct *dst_mm, static int mfill_zeropage_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd, struct vm_area_struct *dst_vma, - unsigned long dst_addr) + unsigned long dst_addr, + uffd_flags_t uffd_flags) { pte_t _dst_pte, *dst_pte; spinlock_t *ptl; @@ -225,6 +229,10 @@ static int mfill_zeropage_pte(struct mm_struct *dst_mm, ret = -EEXIST; if (!pte_none(*dst_pte)) goto out_unlock; + + if (uffd_flags & UFFD_FLAGS_ACCESS_LIKELY) + _dst_pte = pte_mkyoung(_dst_pte); + set_pte_at(dst_mm, dst_addr, dst_pte, _dst_pte); /* No need to invalidate - it was non-present before */ update_mmu_cache(dst_vma, dst_addr, dst_pte); @@ -498,7 +506,7 @@ static __always_inline ssize_t mfill_atomic_pte(struct mm_struct *dst_mm, uffd_flags); else err = mfill_zeropage_pte(dst_mm, dst_pmd, - dst_vma, dst_addr); + dst_vma, dst_addr, uffd_flags); } else { err = shmem_mfill_atomic_pte(dst_mm, dst_pmd, dst_vma, dst_addr, src_addr, @@ -691,6 +699,9 @@ ssize_t mfill_zeropage(struct mm_struct *dst_mm, unsigned long start, unsigned long len, atomic_t *mmap_changing, uffd_flags_t uffd_flags) { + /* There is no cost for setting the access bit of a zeropage */ + uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY; + return __mcopy_atomic(dst_mm, start, 0, len, MCOPY_ATOMIC_ZEROPAGE, mmap_changing, 0); } @@ -699,6 +710,9 @@ ssize_t mcopy_continue(struct mm_struct *dst_mm, unsigned long start, unsigned long len, atomic_t *mmap_changing, uffd_flags_t uffd_flags) { + /* The page is likely to be accessed */ + uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY; + return __mcopy_atomic(dst_mm, start, 0, len, MCOPY_ATOMIC_CONTINUE, mmap_changing, 0); }