From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Josef Bacik <josef@toxicpanda.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
"Liam R. Howlett" <liam@infradead.org>,
David Hildenbrand <david@kernel.org>,
Vlastimil Babka <vbabka@kernel.org>,
Jann Horn <jannh@google.com>, Pedro Falcato <pfalcato@suse.de>,
Muchun Song <muchun.song@linux.dev>,
Oscar Salvador <osalvador@suse.de>,
linux-mm@kvack.org
Subject: Re: [PATCH 6/6] mm/mremap: split first so a failed split is reported
Date: Mon, 2 Nov 2026 13:57:01 +0000 [thread overview]
Message-ID: <auiSshtJveaEdL8b@gremlin> (raw)
In-Reply-To: <20261030-b4-mremap-pre-split-v1-6-c1829811aed0@toxicpanda.com>
On Fri, Oct 30, 2026 at 02:42:51PM +0000, Josef Bacik wrote:
> mremap() can return the new address although the old range is still mapped,
> which leaves the caller with a VMA it does not know about.
But this is established behaviour and a trade-off, e.g. 'the unmap failed, just
get accounts right' in unmap_source_vma().
So all you're left with is an unexpected MREMAP_DONTUNMAP, basically. And only
when the process is doomed where it doesn't matter.
>
> To move a part of a VMA, that part has to be split off, and this is left to
> the unmap at the end of the move. If the split fails there, the page tables
> have moved already and there is nothing to go back to. unmap_source_vma()
> returns void, so move_vma() reports success.
But it can't fail there as we already did all the checks ahead of time.
>
> This was reproduced with fault injection, by failing each allocation of
Yes, of course :)
> such an mremap() in turn. Anonymous and shmem mappings hit it at 2 to 7 of
> 7 to 13 allocations, with numbers that differ from boot to boot, and no
> longer hit it with this patch.
The only way it can happen in reality is huge memory pressure + a fatal signal +
bad luck on timing.
And at that point the process is going to die anyway, so it doesn't matter.
>
> Fix by splitting the source VMA in prep_move_vma() whenever it is going to
> be unmapped, so that a failed split is reported before anything has moved.
No, this is a big cost for no real-world benefit. Now any kind of failure
results in an unwanted pre-split.
This is more likely to screw people up than rather pointlessly trying to do a
favour for a doomed process.
> An old size of 0 unmaps nothing and does not split. If a later step fails,
> the VMA stays split, as it does after a munmap() that fails half way. The
> unmap at the end can still fail in its allocations, or if copy_vma() has
> merged the range and it has to be split again.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
So yeah, overall, I don't think we want this patch.
It's not really accomplishing anything that matters in reality, adds a bunch of
complexity and causes pain for normal users.
So please drop this one thanks :)
--
Cheers, Lorenzo
next prev parent reply other threads:[~2026-11-02 13:57 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-30 14:42 [PATCH 0/6] mm/mremap: report a failed split before moving anything Josef Bacik
2026-10-30 14:42 ` [PATCH 1/6] mm/mremap: fix hugetlb reservation leak on partial move Josef Bacik
2026-11-02 13:13 ` Lorenzo Stoakes (ARM)
2026-10-30 14:42 ` [PATCH 2/6] selftests/mm: test a partial mremap() of hugetlb memory Josef Bacik
2026-11-02 13:14 ` Lorenzo Stoakes (ARM)
2026-10-30 14:42 ` [PATCH 3/6] mm/mremap: reaccount on unmap failure only if accounted Josef Bacik
2026-11-02 13:17 ` Lorenzo Stoakes (ARM)
2026-10-30 14:42 ` [PATCH 4/6] mm/mremap: restore VM_ACCOUNT if the unmap fails Josef Bacik
2026-11-02 13:38 ` Lorenzo Stoakes (ARM)
2026-10-30 14:42 ` [PATCH 5/6] selftests/mm: test mremap() with an old size of 0 Josef Bacik
2026-11-02 13:39 ` Lorenzo Stoakes (ARM)
2026-10-30 14:42 ` [PATCH 6/6] mm/mremap: split first so a failed split is reported Josef Bacik
2026-10-31 19:27 ` Jose A. Perez de Azpillaga
2026-11-02 13:57 ` Lorenzo Stoakes (ARM) [this message]
2026-11-02 12:50 ` [PATCH 0/6] mm/mremap: report a failed split before moving anything Lorenzo Stoakes (ARM)
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=auiSshtJveaEdL8b@gremlin \
--to=ljs@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=david@kernel.org \
--cc=jannh@google.com \
--cc=josef@toxicpanda.com \
--cc=liam@infradead.org \
--cc=linux-mm@kvack.org \
--cc=muchun.song@linux.dev \
--cc=osalvador@suse.de \
--cc=pfalcato@suse.de \
--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.