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 <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]/
Signed-off-by: Suren Baghdasaryan <[email protected]>
---
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);
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
FAULT_FLAG_VMA_LOCK flag was introduced without proper description. Fix
this by documenting it.
Fixes: 863be34fc093 ("mm: add FAULT_FLAG_VMA_LOCK flag")
Reported-by: Stephen Rothwell <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Suren Baghdasaryan <[email protected]>
---
Fix cleanly apply over mm-unstable, SHA in "Fixes" is from that tree.
include/linux/mm_types.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 89bbf7d8a312..ef74ea892c5b 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -1064,6 +1064,7 @@ typedef struct {
* mapped after the fault.
* @FAULT_FLAG_ORIG_PTE_VALID: whether the fault has vmf->orig_pte cached.
* We should only access orig_pte if this flag set.
+ * @FAULT_FLAG_VMA_LOCK: The fault is handled under VMA lock.
*
* About @FAULT_FLAG_ALLOW_RETRY and @FAULT_FLAG_TRIED: we can specify
* whether we would allow page faults to retry by specifying these two
--
2.39.2.722.g9855ee24e9-goog
On 01.03.23 03:27, Suren Baghdasaryan wrote:
> 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 <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
It would be great to mention that this simply silences a smatch warning.
Otherwise, one might be mislead that this commit fixes an actual BUG ;)
> Link: https://lore.kernel.org/r/[email protected]/
> Signed-off-by: Suren Baghdasaryan <[email protected]>
> ---
> 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);
> if (vp->adj_next)
> vma_start_write(vp->adj_next);
> /* vp->insert is always a newly created VMA, no need for locking */
Yes, that looks correct.
Reviewed-by: David Hildenbrand <[email protected]>
--
Thanks,
David / dhildenb
On 01.03.23 03:27, Suren Baghdasaryan wrote:
> FAULT_FLAG_VMA_LOCK flag was introduced without proper description. Fix
> this by documenting it.
>
> Fixes: 863be34fc093 ("mm: add FAULT_FLAG_VMA_LOCK flag")
> Reported-by: Stephen Rothwell <[email protected]>
> Link: https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Suren Baghdasaryan <[email protected]>
> ---
> Fix cleanly apply over mm-unstable, SHA in "Fixes" is from that tree.
Okay, that should be squashed then. LGTM.
--
Thanks,
David / dhildenb
* Suren Baghdasaryan <[email protected]> [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 <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]/
> Signed-off-by: Suren Baghdasaryan <[email protected]>
> ---
> 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?
I'm happy either way.
Reviewed-by: Liam R. Howlett <[email protected]>
> 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
>
On Wed, Mar 1, 2023 at 6:10 AM Liam R. Howlett <[email protected]> wrote:
>
> * Suren Baghdasaryan <[email protected]> [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 <[email protected]>
> > Reported-by: Dan Carpenter <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]/
> > Signed-off-by: Suren Baghdasaryan <[email protected]>
> > ---
> > 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 <[email protected]>
>
> > 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
> >
On Wed, Mar 1, 2023 at 12:27 AM David Hildenbrand <[email protected]> wrote:
>
> On 01.03.23 03:27, Suren Baghdasaryan wrote:
> > FAULT_FLAG_VMA_LOCK flag was introduced without proper description. Fix
> > this by documenting it.
> >
> > Fixes: 863be34fc093 ("mm: add FAULT_FLAG_VMA_LOCK flag")
> > Reported-by: Stephen Rothwell <[email protected]>
> > Link: https://lore.kernel.org/all/[email protected]/
> > Signed-off-by: Suren Baghdasaryan <[email protected]>
> > ---
> > Fix cleanly apply over mm-unstable, SHA in "Fixes" is from that tree.
>
> Okay, that should be squashed then. LGTM.
Yeah, both fixes in this patchset could be squashed into the original
series without information loss. I'll leave that to Andrew to decide
what makes more sense here.
Thanks!
>
> --
> Thanks,
>
> David / dhildenb
>