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

[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

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

[Lorenzo Stoakes (ARM)]

NAK.

You are flooding the kernel with patches, 17 series across 11
subsystems over 24 hrs:

https://lore.kernel.org/all/?q=f%3Anickolai%40csail.mit.edu

This is in strict violation of kernel policy.

If there is anything valid here it will be treated like an anonymous AI
report.

Do NOT engage in this kind of behaviour moving forwards, it's harmful to
the kernel.

--
Cheers, Lorenzo