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

[Liam R. Howlett]

* Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> [230916 15:32]:
> On Sat, 16 Sept 2023 at 04:43, Bagas Sanjaya <bagasdotme@xxxxxxxxx> wrote:
> >
> > Thanks for the regression report. Michael had already bisected it [1], so
> > telling regzbot:
> >
> > #regzbot ^introduced: 408579cd627a15
> > #regzbot title: huge committed memory due to returning 0 on do_vmi_align_mmunmap() success
> >
> > [1]: https://lore.kernel.org/linux-parisc/30f16b4f-a2fa-fc42-fe6e-abad01c3f794@xxxxxxxxxxxxx/
> 
> Funky. That commit isn't actually supposed to change anything, and the
> only locking change was because it incorrectly ended up doing the
> unlock a bit too early (before it did a validate_mm() - fixed in
> commit b5641a5d8b8b ("mm: don't do validate_mm() unnecessarily and
> without mmap locking").
> 
> HOWEVER.
> 
> Now that I look at it again, I note this change in move_vma().
> 
> -       if (do_vmi_munmap(&vmi, mm, old_addr, old_len, uf_unmap, false) < 0) {
> +       if (!do_vmi_munmap(&vmi, mm, old_addr, old_len, uf_unmap, false)) {
> 
> and I think that is wrong.
> 
> The return value that changed was the old "return 1 if successful
> _and_ lock downgraded".
> 
> Now it does "lock is always released on success if requested". So the
> special "1" return went away, but the failure case didn't change.
> 
> So that change to "move_vma()" seems to be bogus. It used to do "if
> failed". Now it does "if success".
> 
> Does the attached patch fix the problem?
> 
> Liam - or am I just crazy? That return value check change really looks
> bogus to me, but it looks *so* bogus that it makes me think I'm
> missing something.

You are correct.  I should not have changed that check.

I haven't devised a way to check this on parisc (I finally have a qemu
parisc vm booting), but it should happen on any call to move_vma().

It looks like you have a tester though, so I'll return to dinner.

Thanks,
Liam