Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/mremap: fix locked_vm accounting for MREMAP_DONTUNMAP
@ 2026-08-28  9:48 Kunwu Chan
  2026-08-28 10:16 ` Lorenzo Stoakes (ARM)
  0 siblings, 1 reply; 3+ messages in thread
From: Kunwu Chan @ 2026-08-28  9:48 UTC (permalink / raw)
  To: akpm, liam, ljs, vbabka, jannh, pfalcato
  Cc: linux-mm, linux-kernel, Kunwu Chan, stable

From: Kunwu Chan <kunwu.chan@gmail.com>

When mremap() is called with MREMAP_DONTUNMAP on a locked VMA,
vrm_stat_account() increments mm->locked_vm for the new VMA while the
source VMA is still marked VM_LOCKED.

For a normal mremap(), the source VMA is subsequently unmapped and the
munmap path decrements mm->locked_vm, balancing this accounting.

With MREMAP_DONTUNMAP, the source VMA is left in place and
dontunmap_complete() clears VMA_LOCKED_MASK.  When the source VMA is
subsequently unmapped, VMA_LOCKED_BIT is already clear, so the munmap
path does not undo the increment from vrm_stat_account().  This leaves
mm->locked_vm over-accounted.

Undo the locked_vm increment in dontunmap_complete() before clearing
VMA_LOCKED_MASK.  MREMAP_DONTUNMAP requires old_len == new_len, and
only VMA_LOCKED_BIT is checked to match the accounting performed by
vrm_stat_account().

Fixes: b714ccb02a76 ("mm/mremap: complete refactor of move_vma()")
Cc: stable@vger.kernel.org
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
---
 mm/mremap.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/mm/mremap.c b/mm/mremap.c
index e8df5cdb0ac..37ea5ee1986 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1334,6 +1334,14 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,
 	unsigned long old_start = vrm->vma->vm_start;
 	unsigned long old_end = vrm->vma->vm_end;
 
+	/*
+	 * vrm_stat_account() accounted the new VMA while the source VMA
+	 * was still locked.  Since DONTUNMAP leaves the source VMA in
+	 * place after clearing VMA_LOCKED_MASK, undo that accounting here.
+	 */
+	if (vma_test(vrm->vma, VMA_LOCKED_BIT))
+		vrm->vma->vm_mm->locked_vm -= vrm->old_len >> PAGE_SHIFT;
+
 	/* We always clear VMA_LOCKED[ONFAULT]_BIT on the old VMA. */
 	vma_clear_flags_mask(vrm->vma, VMA_LOCKED_MASK);
 
@@ -1343,8 +1351,6 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,
 	 */
 	if (new_vma != vrm->vma && start == old_start && end == old_end)
 		unlink_anon_vmas(vrm->vma);
-
-	/* Because we won't unmap we don't need to touch locked_vm. */
 }
 
 static unsigned long move_vma(struct vma_remap_struct *vrm)
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm/mremap: fix locked_vm accounting for MREMAP_DONTUNMAP
  2026-08-28  9:48 [PATCH] mm/mremap: fix locked_vm accounting for MREMAP_DONTUNMAP Kunwu Chan
@ 2026-08-28 10:16 ` Lorenzo Stoakes (ARM)
  2026-08-29 13:53   ` KunWu Chan
  0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-28 10:16 UTC (permalink / raw)
  To: Kunwu Chan
  Cc: akpm, liam, vbabka, jannh, pfalcato, linux-mm, linux-kernel,
	stable

On Fri, Aug 28, 2026 at 05:48:22PM +0800, Kunwu Chan wrote:
> From: Kunwu Chan <kunwu.chan@gmail.com>
>
> When mremap() is called with MREMAP_DONTUNMAP on a locked VMA,
> vrm_stat_account() increments mm->locked_vm for the new VMA while the
> source VMA is still marked VM_LOCKED.
>
> For a normal mremap(), the source VMA is subsequently unmapped and the
> munmap path decrements mm->locked_vm, balancing this accounting.
>
> With MREMAP_DONTUNMAP, the source VMA is left in place and
> dontunmap_complete() clears VMA_LOCKED_MASK.  When the source VMA is
> subsequently unmapped, VMA_LOCKED_BIT is already clear, so the munmap
> path does not undo the increment from vrm_stat_account().  This leaves
> mm->locked_vm over-accounted.
>
> Undo the locked_vm increment in dontunmap_complete() before clearing
> VMA_LOCKED_MASK.  MREMAP_DONTUNMAP requires old_len == new_len, and
> only VMA_LOCKED_BIT is checked to match the accounting performed by
> vrm_stat_account().

Hmm I don't love this approach.

>
> Fixes: b714ccb02a76 ("mm/mremap: complete refactor of move_vma()")

Are you certain that my rework patch was what introduced it?

> Cc: stable@vger.kernel.org
> Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>

In general while the patch doesn't seem super AI-y I do see a sudden upswing in
patches from you in 2026 so I do have to ask whether LLMs were used? If so
please follow https://docs.kernel.org/process/coding-assistants.html

(I hate having to say these things but hey, it's 2026, so.)

I'm kind of inclined to do a fix myself a different way here.

Given it's going to be a backported patch and you're relatively new to mm, I
feel like this is the safer route, sorry about that.

Happy to add a Reported-by/Closes for this however.

> ---
>  mm/mremap.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/mm/mremap.c b/mm/mremap.c
> index e8df5cdb0ac..37ea5ee1986 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -1334,6 +1334,14 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,
>  	unsigned long old_start = vrm->vma->vm_start;
>  	unsigned long old_end = vrm->vma->vm_end;
>
> +	/*
> +	 * vrm_stat_account() accounted the new VMA while the source VMA
> +	 * was still locked.  Since DONTUNMAP leaves the source VMA in
> +	 * place after clearing VMA_LOCKED_MASK, undo that accounting here.
> +	 */
> +	if (vma_test(vrm->vma, VMA_LOCKED_BIT))
> +		vrm->vma->vm_mm->locked_vm -= vrm->old_len >> PAGE_SHIFT;
> +

I don't love this as an approach in general.

As above, I think I will address this slightly differently.

>  	/* We always clear VMA_LOCKED[ONFAULT]_BIT on the old VMA. */
>  	vma_clear_flags_mask(vrm->vma, VMA_LOCKED_MASK);
>
> @@ -1343,8 +1351,6 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,
>  	 */
>  	if (new_vma != vrm->vma && start == old_start && end == old_end)
>  		unlink_anon_vmas(vrm->vma);
> -
> -	/* Because we won't unmap we don't need to touch locked_vm. */
>  }
>
>  static unsigned long move_vma(struct vma_remap_struct *vrm)
> --
> 2.43.0
>

--
Cheers, Lorenzo


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm/mremap: fix locked_vm accounting for MREMAP_DONTUNMAP
  2026-08-28 10:16 ` Lorenzo Stoakes (ARM)
@ 2026-08-29 13:53   ` KunWu Chan
  0 siblings, 0 replies; 3+ messages in thread
From: KunWu Chan @ 2026-08-29 13:53 UTC (permalink / raw)
  To: Lorenzo Stoakes (ARM)
  Cc: akpm, liam, vbabka, jannh, pfalcato, linux-mm, linux-kernel,
	stable

On Fri, Aug 28, 2026 at 6:16 PM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
>
> On Fri, Aug 28, 2026 at 05:48:22PM +0800, Kunwu Chan wrote:
> > From: Kunwu Chan <kunwu.chan@gmail.com>
> >
> > When mremap() is called with MREMAP_DONTUNMAP on a locked VMA,
> > vrm_stat_account() increments mm->locked_vm for the new VMA while the
> > source VMA is still marked VM_LOCKED.
> >
> > For a normal mremap(), the source VMA is subsequently unmapped and the
> > munmap path decrements mm->locked_vm, balancing this accounting.
> >
> > With MREMAP_DONTUNMAP, the source VMA is left in place and
> > dontunmap_complete() clears VMA_LOCKED_MASK.  When the source VMA is
> > subsequently unmapped, VMA_LOCKED_BIT is already clear, so the munmap
> > path does not undo the increment from vrm_stat_account().  This leaves
> > mm->locked_vm over-accounted.
> >
> > Undo the locked_vm increment in dontunmap_complete() before clearing
> > VMA_LOCKED_MASK.  MREMAP_DONTUNMAP requires old_len == new_len, and
> > only VMA_LOCKED_BIT is checked to match the accounting performed by
> > vrm_stat_account().
>

Hi Lorenzo,
Thanks for the detailed reply and your fix patch.

> Hmm I don't love this approach.
>

I agree that explicitly undoing the accounting in dontunmap_complete() is not
the cleanest way to fix this. And your patch is a much cleaner solution: move
vrm_stat_account after the if statement for MREMAP_DONTUNMAP to
keep the behavior unchanged.

> >
> > Fixes: b714ccb02a76 ("mm/mremap: complete refactor of move_vma()")
>
> Are you certain that my rework patch was what introduced it?
>

Yes. I compared the MREMAP_DONTUNMAP path before and after
b714ccb02a76 and traced the ordering of the relevant operations.

Before the rework, the DONTUNMAP path cleared VMA_LOCKED_BIT before
vrm_stat_account() was called [1], so the new VMA was not counted towards
mm->locked_vm.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/tree/mm/mremap.c?id=d5c8aec0542e2d79b64de9089b88fabdebe05c1e#n1016

After b714ccb02a76, move_vma() calls vrm_stat_account() first [2], while the
source VMA is still VM_LOCKED, incrementing mm->locked_vm. The
DONTUNMAP path then calls dontunmap_complete(), which clears
VMA_LOCKED_MASK but leaves the source VMA in place. Thus, when the source
VMA is later unmapped, the corresponding decrement is skipped.
[2] https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/tree/mm/mremap.c?id=b714ccb02a76e170f3e6475749ed0812ee25f777#n1161

This before/after ordering difference is what led me to identify
b714ccb02a76 as the regression commit.

> > Cc: stable@vger.kernel.org
> > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
>
> In general while the patch doesn't seem super AI-y I do see a sudden upswing in
> patches from you in 2026 so I do have to ask whether LLMs were used? If so
> please follow https://docs.kernel.org/process/coding-assistants.html
>
> (I hate having to say these things but hey, it's 2026, so.)
>

Regarding the increase in my mm patches, I have been focusing more
on mm since this year, and had already been doing quite a bit of mm-related
work with some friends before that. Like [3] [4].
[3]  https://lore.kernel.org/linux-mm/20260430040427.4672-2-baohua@kernel.org/
[4] https://lore.kernel.org/all/20260422021842.78495-1-baohua@kernel.org/

I was also away from community activity for a period due to wedding leave
and later leave to accompany my wife for the birth of our child. During that
time, I continued working on some of the mm/DAMON work I had planned but had
not yet finished. After returning, I had several pieces ready to send
upstream, which is why a number of patches appeared within a relatively
short period. Many thanks as well to my wife for her support and efforts.

Thanks for the reminder. I'll follow the kernel's coding-assistant
disclosure requirements
for any LLM-assisted work.

> I'm kind of inclined to do a fix myself a different way here.
>
> Given it's going to be a backported patch and you're relatively new to mm, I
> feel like this is the safer route, sorry about that.
>
> Happy to add a Reported-by/Closes for this however.

Thanks for the fix and for adding the Reported-by and Closes tags. I'll
keep learning and working on mm, and hopefully grow out of the newbie
stage soon.

>
> > ---
> >  mm/mremap.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/mremap.c b/mm/mremap.c
> > index e8df5cdb0ac..37ea5ee1986 100644
> > --- a/mm/mremap.c
> > +++ b/mm/mremap.c
> > @@ -1334,6 +1334,14 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,
> >       unsigned long old_start = vrm->vma->vm_start;
> >       unsigned long old_end = vrm->vma->vm_end;
> >
> > +     /*
> > +      * vrm_stat_account() accounted the new VMA while the source VMA
> > +      * was still locked.  Since DONTUNMAP leaves the source VMA in
> > +      * place after clearing VMA_LOCKED_MASK, undo that accounting here.
> > +      */
> > +     if (vma_test(vrm->vma, VMA_LOCKED_BIT))
> > +             vrm->vma->vm_mm->locked_vm -= vrm->old_len >> PAGE_SHIFT;
> > +
>
> I don't love this as an approach in general.
>
> As above, I think I will address this slightly differently.
>

I saw your patch — it's a very elegant solution and, I think, the best way
to fix this. I was a bit too eager to fix the symptom and didn't think
deeply enough about fixing it at the source.

The fix looks clean to me from the code review. I'll test it as well.

Thanks again!
KunWu

> >       /* We always clear VMA_LOCKED[ONFAULT]_BIT on the old VMA. */
> >       vma_clear_flags_mask(vrm->vma, VMA_LOCKED_MASK);
> >
> > @@ -1343,8 +1351,6 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,
> >        */
> >       if (new_vma != vrm->vma && start == old_start && end == old_end)
> >               unlink_anon_vmas(vrm->vma);
> > -
> > -     /* Because we won't unmap we don't need to touch locked_vm. */
> >  }
> >
> >  static unsigned long move_vma(struct vma_remap_struct *vrm)
> > --
> > 2.43.0
> >
>
> --
> Cheers, Lorenzo


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-29 13:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-28  9:48 [PATCH] mm/mremap: fix locked_vm accounting for MREMAP_DONTUNMAP Kunwu Chan
2026-08-28 10:16 ` Lorenzo Stoakes (ARM)
2026-08-29 13:53   ` KunWu Chan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox