All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Kunwu Chan <kunwu.chan@gmail.com>
Cc: akpm@linux-foundation.org, liam@infradead.org, vbabka@kernel.org,
	 jannh@google.com, pfalcato@suse.de, linux-mm@kvack.org,
	 linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] mm/mremap: fix locked_vm accounting for MREMAP_DONTUNMAP
Date: Fri, 28 Aug 2026 11:16:32 +0100	[thread overview]
Message-ID: <apFa2eTBzBP0RPeb@gremlin> (raw)
In-Reply-To: <20260828094823.594279-1-kunwu.chan@linux.dev>

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


  reply	other threads:[~2026-08-28 10:16 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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) [this message]
2026-08-29 13:53   ` KunWu Chan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=apFa2eTBzBP0RPeb@gremlin \
    --to=ljs@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=jannh@google.com \
    --cc=kunwu.chan@gmail.com \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=pfalcato@suse.de \
    --cc=stable@vger.kernel.org \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.