linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Stoakes <lstoakes@gmail.com>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	patches@lists.linux.dev, stable@vger.kenrel.org,
	"Liam R . Howlett" <Liam.Howlett@oracle.com>
Subject: Re: [PATCH] mm, mmap: remove vma_merge()
Date: Mon, 1 Apr 2024 20:45:42 +0100	[thread overview]
Message-ID: <38b67f6a-6c64-43db-85fb-027efac9d9d6@lucifer.local> (raw)
In-Reply-To: <20240401192623.18575-2-vbabka@suse.cz>

On Mon, Apr 01, 2024 at 09:26:24PM +0200, Vlastimil Babka wrote:
> With SLAB gone I was looking for what I could delete next, and then it
> dawned on me - it should be vma_merge(). It's simply an unmaintainable
> mess.
>
> The code is extremely hard to follow. There has been a number of recent
> attempts to made it more readable, by Liam, Lorenzo and myself. Almost
> every time this only resulted in subtle embarassing bugs [1] [2] [3] [4]
> [5]. For many of the bugs the only reliable testcase we have is the rpm
> process running in the Open Build Service. Which is worrying, as that
> clearly makes vma_merge() a potential supply chain attack vector for
> planting backdoors.
>
> Thus simply remove vma_merge() and all the related code, and adjust all
> callers to take the path as if merging was attempted and failed.
> The only downside is that there might now be more vmas and maple tree
> nodes due to no merging. To extensively validate that, I've booted the
> kernel with virtme-ng before and after the change and checked slabinfo
> for the number of active objects:
>
> before:
> maple_node          1159
> vm_area_struct      1538
>
> after:
> maple_node          1113
> vm_area_struct      1861
>
> Yeah there's more vma's but interestingly, less maple tree nodes. I
> believe it's worth it. The larger number of smaller vmas could even help
> with better parallelism thanks to per-vma locking. So the conclusion is
> obvious - we should have never tried merging vmas in the first place,
> hence the tags on this patch.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d014cd7c1c35
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4c6759967826
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7e7757876f25
> [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1419430c8abb
> [5] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fc0c8f9089c2
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: <stable@vger.kenrel.org> # CVE mandatory
> Cc: Lorenzo Stoakes <lstoakes@gmail.com>
> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
> Singed-off-by-one: Vlastimil Babka <vbabka@suse.cz>
> ---

[snip]

This is a beautiful patch and about as perfect a clean up as one could imagine
(I mean #thebestcodeisnocode right?), but as we have now eradicated VMA merging,
I can't be a hypocrite and advocate for patch merging.

As a result, sadly I must:

Hope-Liam-Nacks-It-By: Lorenzo Stoakes <lstoakes@gmail.com>


  reply	other threads:[~2024-04-01 19:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-01 19:26 Vlastimil Babka
2024-04-01 19:45 ` Lorenzo Stoakes [this message]
2024-04-02  7:28 ` Mike Rapoport

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=38b67f6a-6c64-43db-85fb-027efac9d9d6@lucifer.local \
    --to=lstoakes@gmail.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kenrel.org \
    --cc=vbabka@suse.cz \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox