Re: Possible 6.5 regression: Huge values for "commited memory"

[Linus Torvalds]

On Sat, 16 Sept 2023 at 12:31, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> Does the attached patch fix the problem?

So while I didn't confirm the fix myself, I'm pretty sure that was it.
Getting the return value wrong would cause an incorrect extra
vm_acct_memory() call in the non-error case when VM_ACCOUNT is set
(and mean the loss of one in the error case, but the error case never
happens in practice).

Which then causes 'vm_committed_as' to grow when it shouldn't, and
causes exactly that "Committed_AS" in /proc/meminfo to be off.

So here's the same patch, but now with a proper commit message etc.

I haven't pushed it out (because it would be lovely to get a
"Tested-by" for it, and that will make the commit ID change), but I'll
probably do so later today, with or without confirmation, because it
does seem to be the problem.

              Linus
From 65a0e100c8a6f8763a9c3bf2c0b361c8f436e42d Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Sat, 16 Sep 2023 12:31:42 -0700
Subject: [PATCH] vm: fix move_vma() memory accounting being off

Commit 408579cd627a ("mm: Update do_vmi_align_munmap() return
semantics") seems to have updated one of the callers of do_vmi_munmap()
incorrectly: it used to check for the error case (which didn't
change: negative means error).

That commit changed the check to the success case (which did change:
before that commit, 0 was success, and 1 was "success and lock
downgraded".  After the change, it's always 0 for success, and the lock
will have been released if requested).

This didn't change any actual VM behavior _except_ for memory accounting
when 'VM_ACCOUNT' was set on the vma.  Which made the wrong return value
test fairly subtle, since everything continues to work.

Or rather - it continues to work but the "Committed memory" accounting
goes all wonky (Committed_AS value in /proc/meminfo), and depending on
settings that then causes problems much much later as the VM relies on
bogus statistics for its heuristics.

Revert that one line of the change back to the original logic.

Fixes: 408579cd627a ("mm: Update do_vmi_align_munmap() return semantics")
Reported-by: Christoph Biedl <linux-kernel.bfrz@xxxxxxxxxxxxxxxxxx>
Reported-and-bisected-by: Michael Labiuk <michael.labiuk@xxxxxxxxxxxxx>
Cc: Bagas Sanjaya <bagasdotme@xxxxxxxxx>
Cc: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
Link: https://lore.kernel.org/all/1694366957@xxxxxxxxxxxxxxxxxxxxxxxx/
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
---
 mm/mremap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mremap.c b/mm/mremap.c
index 056478c106ee..382e81c33fc4 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -715,7 +715,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 	}
 
 	vma_iter_init(&vmi, mm, old_addr);
-	if (!do_vmi_munmap(&vmi, mm, old_addr, old_len, uf_unmap, false)) {
+	if (do_vmi_munmap(&vmi, mm, old_addr, old_len, uf_unmap, false) < 0) {
 		/* OOM: unable to split vma, just get accounts right */
 		if (vm_flags & VM_ACCOUNT && !(flags & MREMAP_DONTUNMAP))
 			vm_acct_memory(old_len >> PAGE_SHIFT);
-- 
2.42.0.rc0.30.gca81aba3b0