Messages in this thread | | | From | Suren Baghdasaryan <> | Date | Wed, 1 Mar 2023 09:54:31 -0800 | Subject | Re: [PATCH 1/2] mm/mmap: remove unnecessary vp->vma check in vma_prepare |
| |
On Wed, Mar 1, 2023 at 6:10 AM Liam R. Howlett <Liam.Howlett@oracle.com> wrote: > > * Suren Baghdasaryan <surenb@google.com> [230228 21:27]: > > vp->vma in vma_prepare() is always non-NULL, therefore checking it is > > not necessary. Remove the extra check. > > > > Fixes: e8f071350ea5 ("mm/mmap: write-lock VMAs in vma_prepare before modifying them") > > Reported-by: kernel test robot <lkp@intel.com> > > Reported-by: Dan Carpenter <error27@gmail.com> > > Link: https://lore.kernel.org/r/202302281802.J93Nma7q-lkp@intel.com/ > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > --- > > Fix cleanly apply over mm-unstable, SHA in "Fixes" is from that tree. > > > > mm/mmap.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index 0cd3714c2182..0759d53b470c 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -505,8 +505,7 @@ static inline void init_vma_prep(struct vma_prepare *vp, > > */ > > static inline void vma_prepare(struct vma_prepare *vp) > > { > > - if (vp->vma) > > - vma_start_write(vp->vma); > > + vma_start_write(vp->vma); > > Would a WARN_ON_ONCE() be worth it? Maybe not since it will be detected > rather quickly once we dereference it below, but it might make it more > clear as to what happened?
WARN_ON_ONCE() seems like an overkill to me. It always follows after init_multi_vma_prep()/init_vma_prep() both of which set the VMA. Risk should be minimal here and as you said, misuse is easily discoverable.
> > I'm happy either way. > > Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> > > > if (vp->adj_next) > > vma_start_write(vp->adj_next); > > /* vp->insert is always a newly created VMA, no need for locking */ > > -- > > 2.39.2.722.g9855ee24e9-goog > >
| |