| Message ID | 20241117080931.600731-2-surenb@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | move per-vma lock into vm_area_struct | expand |
On Sun, Nov 17, 2024 at 12:09:27AM -0800, Suren Baghdasaryan wrote: > Introduce helper functions which can be used to read-lock a VMA when > holding mmap_lock for read. Replace direct accesses to vma->vm_lock > with these new helpers. > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> LGTM thanks: Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> > --- > include/linux/mm.h | 24 ++++++++++++++++++++++++ > mm/userfaultfd.c | 22 +++++----------------- > 2 files changed, 29 insertions(+), 17 deletions(-) > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index fecd47239fa9..1ba2e480ae63 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -722,6 +722,30 @@ static inline bool vma_start_read(struct vm_area_struct *vma) > return true; > } > > +/* > + * Use only while holding mmap read lock which guarantees that locking will not > + * fail (nobody can concurrently write-lock the vma). vma_start_read() should > + * not be used in such cases because it might fail due to mm_lock_seq overflow. > + * This functionality is used to obtain vma read lock and drop the mmap read lock. > + */ > +static inline void vma_start_read_locked_nested(struct vm_area_struct *vma, int subclass) > +{ > + mmap_assert_locked(vma->vm_mm); > + down_read_nested(&vma->vm_lock->lock, subclass); > +} > + > +/* > + * Use only while holding mmap read lock which guarantees that locking will not > + * fail (nobody can concurrently write-lock the vma). vma_start_read() should > + * not be used in such cases because it might fail due to mm_lock_seq overflow. > + * This functionality is used to obtain vma read lock and drop the mmap read lock. > + */ > +static inline void vma_start_read_locked(struct vm_area_struct *vma) > +{ > + mmap_assert_locked(vma->vm_mm); > + down_read(&vma->vm_lock->lock); > +} > + > static inline void vma_end_read(struct vm_area_struct *vma) > { > rcu_read_lock(); /* keeps vma alive till the end of up_read */ > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c > index 60a0be33766f..87db4b32b82a 100644 > --- a/mm/userfaultfd.c > +++ b/mm/userfaultfd.c > @@ -84,16 +84,8 @@ static struct vm_area_struct *uffd_lock_vma(struct mm_struct *mm, > > mmap_read_lock(mm); > vma = find_vma_and_prepare_anon(mm, address); > - if (!IS_ERR(vma)) { > - /* > - * We cannot use vma_start_read() as it may fail due to > - * false locked (see comment in vma_start_read()). We > - * can avoid that by directly locking vm_lock under > - * mmap_lock, which guarantees that nobody can lock the > - * vma for write (vma_start_write()) under us. > - */ > - down_read(&vma->vm_lock->lock); > - } > + if (!IS_ERR(vma)) > + vma_start_read_locked(vma); > > mmap_read_unlock(mm); > return vma; > @@ -1476,14 +1468,10 @@ static int uffd_move_lock(struct mm_struct *mm, > mmap_read_lock(mm); > err = find_vmas_mm_locked(mm, dst_start, src_start, dst_vmap, src_vmap); > if (!err) { > - /* > - * See comment in uffd_lock_vma() as to why not using > - * vma_start_read() here. > - */ > - down_read(&(*dst_vmap)->vm_lock->lock); > + vma_start_read_locked(*dst_vmap); > if (*dst_vmap != *src_vmap) > - down_read_nested(&(*src_vmap)->vm_lock->lock, > - SINGLE_DEPTH_NESTING); > + vma_start_read_locked_nested(*src_vmap, > + SINGLE_DEPTH_NESTING); > } > mmap_read_unlock(mm); > return err; > -- > 2.47.0.338.g60cca15819-goog >
On Sun, 17 Nov 2024, Suren Baghdasaryan wrote: >Introduce helper functions which can be used to read-lock a VMA when >holding mmap_lock for read. Replace direct accesses to vma->vm_lock >with these new helpers. > Reviewed-by: Davidlohr Bueso <dave@stgolabs.net> >Signed-off-by: Suren Baghdasaryan <surenb@google.com>
diff --git a/include/linux/mm.h b/include/linux/mm.h index fecd47239fa9..1ba2e480ae63 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -722,6 +722,30 @@ static inline bool vma_start_read(struct vm_area_struct *vma) return true; } +/* + * Use only while holding mmap read lock which guarantees that locking will not + * fail (nobody can concurrently write-lock the vma). vma_start_read() should + * not be used in such cases because it might fail due to mm_lock_seq overflow. + * This functionality is used to obtain vma read lock and drop the mmap read lock. + */ +static inline void vma_start_read_locked_nested(struct vm_area_struct *vma, int subclass) +{ + mmap_assert_locked(vma->vm_mm); + down_read_nested(&vma->vm_lock->lock, subclass); +} + +/* + * Use only while holding mmap read lock which guarantees that locking will not + * fail (nobody can concurrently write-lock the vma). vma_start_read() should + * not be used in such cases because it might fail due to mm_lock_seq overflow. + * This functionality is used to obtain vma read lock and drop the mmap read lock. + */ +static inline void vma_start_read_locked(struct vm_area_struct *vma) +{ + mmap_assert_locked(vma->vm_mm); + down_read(&vma->vm_lock->lock); +} + static inline void vma_end_read(struct vm_area_struct *vma) { rcu_read_lock(); /* keeps vma alive till the end of up_read */ diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c index 60a0be33766f..87db4b32b82a 100644 --- a/mm/userfaultfd.c +++ b/mm/userfaultfd.c @@ -84,16 +84,8 @@ static struct vm_area_struct *uffd_lock_vma(struct mm_struct *mm, mmap_read_lock(mm); vma = find_vma_and_prepare_anon(mm, address); - if (!IS_ERR(vma)) { - /* - * We cannot use vma_start_read() as it may fail due to - * false locked (see comment in vma_start_read()). We - * can avoid that by directly locking vm_lock under - * mmap_lock, which guarantees that nobody can lock the - * vma for write (vma_start_write()) under us. - */ - down_read(&vma->vm_lock->lock); - } + if (!IS_ERR(vma)) + vma_start_read_locked(vma); mmap_read_unlock(mm); return vma; @@ -1476,14 +1468,10 @@ static int uffd_move_lock(struct mm_struct *mm, mmap_read_lock(mm); err = find_vmas_mm_locked(mm, dst_start, src_start, dst_vmap, src_vmap); if (!err) { - /* - * See comment in uffd_lock_vma() as to why not using - * vma_start_read() here. - */ - down_read(&(*dst_vmap)->vm_lock->lock); + vma_start_read_locked(*dst_vmap); if (*dst_vmap != *src_vmap) - down_read_nested(&(*src_vmap)->vm_lock->lock, - SINGLE_DEPTH_NESTING); + vma_start_read_locked_nested(*src_vmap, + SINGLE_DEPTH_NESTING); } mmap_read_unlock(mm); return err;
Introduce helper functions which can be used to read-lock a VMA when holding mmap_lock for read. Replace direct accesses to vma->vm_lock with these new helpers. Signed-off-by: Suren Baghdasaryan <surenb@google.com> --- include/linux/mm.h | 24 ++++++++++++++++++++++++ mm/userfaultfd.c | 22 +++++----------------- 2 files changed, 29 insertions(+), 17 deletions(-)