| Message ID | 20221030212929.335473-5-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/hugetlb: Make huge_pte_offset() thread-safe for pmd unshare | expand |
On Sun, Oct 30, 2022 at 2:29 PM Peter Xu <peterx@redhat.com> wrote: > > RCU makes sure the pte_t* won't go away from under us. Please refer to the > comment above huge_pte_offset() for more information. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > fs/userfaultfd.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 07c81ab3fd4d..4e813e68e4f8 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -243,6 +243,9 @@ static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx, > > mmap_assert_locked(mm); > > + /* For huge_pte_offset() */ > + rcu_read_lock(); userfaultfd_huge_must_wait is called after we set the task's state to blocking. Is it always safe to call rcu_read_lock (and rcu_read_unlock) in this case? (With my basic understanding of RCU, this seems like it should be safe, but I'm not sure.) - James > + > ptep = huge_pte_offset(mm, address, vma_mmu_pagesize(vma)); > > if (!ptep) > @@ -261,6 +264,7 @@ static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx, > if (!huge_pte_write(pte) && (reason & VM_UFFD_WP)) > ret = true; > out: > + rcu_read_unlock(); > return ret; > } > #else > -- > 2.37.3 >
On Wed, Nov 02, 2022 at 11:06:16AM -0700, James Houghton wrote: > On Sun, Oct 30, 2022 at 2:29 PM Peter Xu <peterx@redhat.com> wrote: > > > > RCU makes sure the pte_t* won't go away from under us. Please refer to the > > comment above huge_pte_offset() for more information. > > > > Signed-off-by: Peter Xu <peterx@redhat.com> > > --- > > fs/userfaultfd.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > > index 07c81ab3fd4d..4e813e68e4f8 100644 > > --- a/fs/userfaultfd.c > > +++ b/fs/userfaultfd.c > > @@ -243,6 +243,9 @@ static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx, > > > > mmap_assert_locked(mm); > > > > + /* For huge_pte_offset() */ > > + rcu_read_lock(); > > userfaultfd_huge_must_wait is called after we set the task's state to > blocking. Is it always safe to call rcu_read_lock (and > rcu_read_unlock) in this case? (With my basic understanding of RCU, > this seems like it should be safe, but I'm not sure.) I'm not aware of an issue here, but please shoot if you have any further concerns or clues, because I'm definitely not a rcu person so I can overlook things. What I remember is my smoke test should be with LOCKDEP, it didn't trigger anything so far I think.
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 07c81ab3fd4d..4e813e68e4f8 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -243,6 +243,9 @@ static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx, mmap_assert_locked(mm); + /* For huge_pte_offset() */ + rcu_read_lock(); + ptep = huge_pte_offset(mm, address, vma_mmu_pagesize(vma)); if (!ptep) @@ -261,6 +264,7 @@ static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx, if (!huge_pte_write(pte) && (reason & VM_UFFD_WP)) ret = true; out: + rcu_read_unlock(); return ret; } #else
RCU makes sure the pte_t* won't go away from under us. Please refer to the comment above huge_pte_offset() for more information. Signed-off-by: Peter Xu <peterx@redhat.com> --- fs/userfaultfd.c | 4 ++++ 1 file changed, 4 insertions(+)