[PATCH v1] mm/mremap: do not charge a shrinking move twice

[PATCH v1] mm/mremap: do not charge a shrinking move twice

[Nickolai Zeldovich]

Since v6.15 every mremap() that shrinks an accounted mapping and moves
it with MREMAP_MAYMOVE | MREMAP_FIXED leaves old_len - new_len bytes
charged to vm_committed_as for good.  An unprivileged process that
repeats such a call keeps raising Committed_AS in /proc/meminfo; with
vm.overcommit_memory=2 it reaches CommitLimit, after which every
accounted allocation in the system (private writable mmap(), brk(),
fork()) fails with ENOMEM until reboot.  A loop of 1000 calls moving a
2 MiB private anonymous mapping to a 1 MiB fixed destination raises
Committed_AS by 1 GiB.

mremap_to() first unmaps the tail of the source with shrink_vma(),
which uncharges it, and then sets old_len to new_len so that the move
looks like a same-size one.  vrm->delta, the absolute difference of the
two lengths computed in do_mremap(), is not updated, and
vrm_calc_charge() charges delta pages for the move.  For a move that
does not unmap the source the charge only has to cover the growth from
old_len to new_len, which here is zero; nothing ever releases the extra
charge.  Before the vma_remap_struct conversion move_vma() charged
new_len - old_len with the updated old_len.

Recompute delta after the shrink.

Tested with the program in the notes under QEMU (x86_64 defconfig):
Committed_AS grows by about 1 GiB per run without this patch and stays
flat with it.

Fixes: b714ccb02a76 ("mm/mremap: complete refactor of move_vma()")
Assisted-by: LLM
Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>
---

Notes:
    Reproducer (any accounted private mapping; watch Committed_AS):
    
    #define _GNU_SOURCE
    #include <stdio.h>
    #include <sys/mman.h>
    
    static long committed_kb(void)
    {
    	char line[128];
    	long kb = -1;
    	FILE *f = fopen("/proc/meminfo", "r");
    
    	while (f && fgets(line, sizeof(line), f))
    		if (sscanf(line, "Committed_AS: %ld kB", &kb) == 1)
    			break;
    	if (f)
    		fclose(f);
    	return kb;
    }
    
    int main(void)
    {
    	const size_t old_len = 2 << 20, new_len = 1 << 20;
    	long before = committed_kb();
    
    	for (int i = 0; i < 1000; i++) {
    		char *src = mmap(NULL, old_len, PROT_READ | PROT_WRITE,
    				 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    		char *dst = mmap(NULL, new_len, PROT_NONE,
    				 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    
    		if (mremap(src, old_len, new_len, MREMAP_MAYMOVE | MREMAP_FIXED,
    			   dst) != dst)
    			return perror("mremap"), 1;
    		munmap(dst, new_len);
    	}
    	printf("leaked %ld kB\n", committed_kb() - before);
    	return 0;
    }
    
    $ gcc -O2 -o mremap-shrink-charge mremap-shrink-charge.c && ./mremap-shrink-charge
    before: leaked 1024000 kB   (every run; Committed_AS 3744 -> 1027744 -> 2051744 kB)
    after:  leaked 0 kB

 mm/mremap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/mremap.c b/mm/mremap.c
index 444f37d7cd28..2dd41c611fdd 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1523,6 +1523,7 @@ static unsigned long mremap_to(struct vma_remap_struct *vrm)
 
 		/* Set up for the move now shrink has been executed. */
 		vrm->old_len = vrm->new_len;
+		vrm_set_delta(vrm);
 	}
 
 	/* MREMAP_DONTUNMAP expands by old_len since old_len == new_len */

base-commit: 70dd4ee72eb2d5793c62e71c14e3aba78e5a4e56
-- 
2.53.0

Re: [PATCH v1] mm/mremap: do not charge a shrinking move twice

[Andrew Morton]

On Sat, 17 Oct 2026 19:35:26 -0400 Nickolai Zeldovich <nickolai@csail.mit.edu> wrote:

> Since v6.15 every mremap() that shrinks an accounted mapping and moves
> it with MREMAP_MAYMOVE | MREMAP_FIXED leaves old_len - new_len bytes
> charged to vm_committed_as for good.  An unprivileged process that
> repeats such a call keeps raising Committed_AS in /proc/meminfo; with
> vm.overcommit_memory=2 it reaches CommitLimit, after which every
> accounted allocation in the system (private writable mmap(), brk(),
> fork()) fails with ENOMEM until reboot.  A loop of 1000 calls moving a
> 2 MiB private anonymous mapping to a 1 MiB fixed destination raises
> Committed_AS by 1 GiB.
> 
> mremap_to() first unmaps the tail of the source with shrink_vma(),
> which uncharges it, and then sets old_len to new_len so that the move
> looks like a same-size one.  vrm->delta, the absolute difference of the
> two lengths computed in do_mremap(), is not updated, and
> vrm_calc_charge() charges delta pages for the move.  For a move that
> does not unmap the source the charge only has to cover the growth from
> old_len to new_len, which here is zero; nothing ever releases the extra
> charge.  Before the vma_remap_struct conversion move_vma() charged
> new_len - old_len with the updated old_len.
> 
> Recompute delta after the shrink.
> 
> Tested with the program in the notes under QEMU (x86_64 defconfig):
> Committed_AS grows by about 1 GiB per run without this patch and stays
> flat with it.

So I'm training my LLM to do better reviews.  It sucks in Tejun's
review prompts which suck in Chris's prompts (which Sashiko also uses).
It also incorporates some of the prompts which Sashiko overlays on
Chris's prompts.

And now rather than just checking code I'm getting it to step back and
take a broader, more architectural/design view.  Scan the lists for
similar changes, look into the revision history to understand why
things are as they are.  Study comparable code sites.  One way I'm
doing this, which might be innovative, is to feed it some of the more
impressive human/maintainer reviews I see and ask it "what rules can
you set in place to make your reviews more like those people's".

I'd like to start using it on people so I can also obtain
author/maintainer comments on its reviews.  I can then feed those
reviews-of-reviews back into it to get it to learn and improve.

It's a work in progress. Here's what it said about this patch:


Hi Nickolai,

The code change looks correct to me. After shrink_vma() has unmapped
and uncharged the tail, resetting vrm->delta to zero correctly avoids
charging again for the remaining same-size move. I checked the other
delta consumers and error paths and found no introduced problems.

One changelog correction:

> For a move that does not unmap the source the charge only has to
> cover the growth from old_len to new_len

This is backwards. MREMAP_DONTUNMAP retains the source and charges
the entire new mapping. An ordinary move unmaps the source and
requires additional commitment only for growth.

I'd suggest:

"For an ordinary move, which unmaps the source, only an increase in
mapping size requires additional commit accounting. Here shrink_vma()
has already unmapped and uncharged the tail, so the remaining
same-size move requires no additional charge."

Two minor points:

- "Until reboot" is too strong: other processes can release
  committed memory, although the leaked charge itself persists.
- The vrm_set_delta() comment says "difference in VMA size specified
  by user", which is no longer quite accurate when called after
  adjusting old_len.

The Fixes: tag looks correct. Given the potentially system-wide
ENOMEM failures under strict overcommit, Cc: stable seems appropriate.

With those minor changes, I think this is ready to go.

[PATCH v2] mm/mremap: do not charge a shrinking move twice

[Nickolai Zeldovich]

Since v6.15 every mremap() that shrinks an accounted mapping and moves
it with MREMAP_MAYMOVE | MREMAP_FIXED leaves old_len - new_len bytes
charged to vm_committed_as for good; the charge outlives the process.
An unprivileged process that repeats such a call keeps raising
Committed_AS in /proc/meminfo.  With vm.overcommit_memory=2 it reaches
CommitLimit, after which every accounted allocation in the system
(private writable mmap(), brk(), fork()) fails with ENOMEM until other
processes release enough committed memory; the leaked charge itself is
never released.  A loop of 1000 calls moving a 2 MiB private anonymous
mapping to a 1 MiB fixed destination raises Committed_AS by 1 GiB.

mremap_to() first unmaps the tail of the source with shrink_vma(),
which uncharges it, and then sets old_len to new_len so that the move
looks like a same-size one.  vrm->delta, the absolute difference of the
two lengths computed in do_mremap(), is not updated, and
vrm_calc_charge() charges delta pages for the move.  For an ordinary
move, which unmaps the source, only an increase in mapping size
requires additional commit accounting.  Here shrink_vma() has already
unmapped and uncharged the tail, so the remaining same-size move
requires no additional charge.  Before the refactor move_vma() charged
new_len - old_len with the updated old_len.

Recompute delta after the shrink.  vrm_set_delta() now also runs after
old_len has been adjusted, so its comment no longer calls the
difference user-specified.

Tested under QEMU (x86_64 defconfig) with a program that repeats the
call 1000 times: Committed_AS grows by 1 GiB per run without this
patch and stays flat with it.

Fixes: d5c8aec0542e ("mm/mremap: initial refactor of move_vma()")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>
---

Notes:
    Changes in v2, after Andrew Morton's review:
    - The sentence on which move needs a charge had it backwards; an
      ordinary move unmaps the source and is charged only for growth,
      MREMAP_DONTUNMAP keeps the source and is charged the whole new
      length.  Reworded as suggested.
    - The ENOMEM state lasts until other processes release committed
      memory, not until reboot; only the leaked charge is permanent.
    - Comment on vrm_set_delta() updated, since it now also runs after
      old_len has been adjusted.
    - Fixes: now d5c8aec0542e, the commit that replaced the new_len - old_len
      charge in move_vma() with vrm->delta; b714ccb02a76 only reorganised
      that code.  The reproducer leaks on d5c8aec0542e and not on its parent.
    
    v1: https://lore.kernel.org/all/20261017233527.797191-1-nickolai@csail.mit.edu/

 mm/mremap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/mremap.c b/mm/mremap.c
index 444f37d7cd28..479befed1b2a 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -887,7 +887,7 @@ unsigned long move_page_tables(struct pagetable_move_control *pmc)
 	return pmc_progress(pmc);
 }
 
-/* Set vrm->delta to the difference in VMA size specified by user. */
+/* Set vrm->delta to the absolute difference between old_len and new_len. */
 static void vrm_set_delta(struct vma_remap_struct *vrm)
 {
 	vrm->delta = abs_diff(vrm->old_len, vrm->new_len);
@@ -1523,6 +1523,7 @@ static unsigned long mremap_to(struct vma_remap_struct *vrm)
 
 		/* Set up for the move now shrink has been executed. */
 		vrm->old_len = vrm->new_len;
+		vrm_set_delta(vrm);
 	}
 
 	/* MREMAP_DONTUNMAP expands by old_len since old_len == new_len */

base-commit: 58161ead29e50d70a317ea26b472e0c7508e69da
-- 
2.53.0