linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
To: Pedro Falcato <pfalcato@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"Liam R . Howlett" <Liam.Howlett@oracle.com>,
	David Hildenbrand <david@redhat.com>,
	Vlastimil Babka <vbabka@suse.cz>, Jann Horn <jannh@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Jeff Xu <jeffxu@chromium.org>
Subject: Re: [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check
Date: Mon, 14 Jul 2025 16:23:13 +0100	[thread overview]
Message-ID: <cd3516af-8481-4418-9f72-a7738a9fd024@lucifer.local> (raw)
In-Reply-To: <ky2jvl6uyi75qwfmpwzmwu6qfnlwxshk2zunywe3pve2pshdxj@p2ihhzov3imx>

On Mon, Jul 14, 2025 at 04:17:23PM +0100, Pedro Falcato wrote:
> On Mon, Jul 14, 2025 at 02:00:39PM +0100, Lorenzo Stoakes wrote:
> > The check_mm_seal() function is doing something general - checking whether
> > a range contains only VMAs (or rather that it does NOT contain any unmapped
> > regions).
> >
> > Generalise this and put the logic in mm/vma.c - introducing
> > range_contains_unmapped(). Additionally we can simplify the logic, we are
> > simply checking whether the last vma->vm_end has either a VMA starting
> > after it or ends before the end parameter.
> >
>
> I don't like this. Unless you have any other user for this in mind,
> we'll proliferate this awful behavior (and add this into core vma code).

I'm not sure how putting it in an internal-only mm file perpetuates
anything.

I'm naming the function by what it does, and putting it where it belongs in
the VMA logic, and additionally making the function less horrible.

Let's not please get stuck on the isues with mseal implementation which
will catch-22 us into not being able to refactor.

We can do the refactoring first and it's fine to just yank this if it's not
used.

I'm not having a function like this sat in mm/mseal.c when it has
absolutely nothing to do with mseal specifically though.

>
> I have some patches locally to fully remove this upfront check, and AFAIK
> we're somewhat in agreement that we can simply nuke this check (for
> various reasons, including that we *still* don't have a man page for the
> syscall). I can send them for proper discussion after your series lands.

Yes I agree this check is odd, I don't really see why on earth we're
concerned with whether there are gaps or not, you'd surely want to just
seal whatever VMAs exist in the range?

But this belongs as something separate (a _functional_ change), let's get
the code sane first.

And ack on manpage, that's a horrible oversight that needs addressing!

>
> --
> Pedro


  reply	other threads:[~2025-07-14 15:23 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-14 13:00 [PATCH 0/5] mseal cleanups Lorenzo Stoakes
2025-07-14 13:00 ` [PATCH 1/5] mm/mseal: always define VM_SEALED Lorenzo Stoakes
2025-07-14 14:31   ` David Hildenbrand
2025-07-14 15:20   ` Pedro Falcato
2025-07-14 15:32   ` Liam R. Howlett
2025-07-14 13:00 ` [PATCH 2/5] mm/mseal: move madvise() logic to mm/madvise.c Lorenzo Stoakes
2025-07-14 14:37   ` David Hildenbrand
2025-07-14 14:56     ` Lorenzo Stoakes
2025-07-14 15:03       ` David Hildenbrand
2025-07-14 15:18         ` Lorenzo Stoakes
2025-07-14 15:24           ` David Hildenbrand
2025-07-14 15:27             ` Lorenzo Stoakes
2025-07-14 15:37               ` David Hildenbrand
2025-07-14 15:31         ` Pedro Falcato
2025-07-14 15:37           ` Lorenzo Stoakes
2025-07-14 15:41           ` David Hildenbrand
2025-07-14 15:45             ` Lorenzo Stoakes
2025-07-14 15:52               ` David Hildenbrand
2025-07-14 16:01                 ` Lorenzo Stoakes
2025-07-14 15:18   ` Pedro Falcato
2025-07-14 15:33   ` Liam R. Howlett
2025-07-14 13:00 ` [PATCH 3/5] mm/mseal: small cleanups Lorenzo Stoakes
2025-07-14 14:39   ` David Hildenbrand
2025-07-14 15:23   ` Pedro Falcato
2025-07-14 15:33   ` Liam R. Howlett
2025-07-14 13:00 ` [PATCH 4/5] mm/mseal: separate out and simplify VMA gap check Lorenzo Stoakes
2025-07-14 14:41   ` David Hildenbrand
2025-07-14 15:17   ` Pedro Falcato
2025-07-14 15:23     ` Lorenzo Stoakes [this message]
2025-07-14 15:25       ` David Hildenbrand
2025-07-14 15:32         ` Lorenzo Stoakes
2025-07-14 15:40           ` Liam R. Howlett
2025-07-14 15:35   ` Liam R. Howlett
2025-07-14 15:40     ` Lorenzo Stoakes
2025-07-14 15:43       ` Liam R. Howlett
2025-07-14 15:47         ` Lorenzo Stoakes
2025-07-14 13:00 ` [PATCH 5/5] mm/mseal: rework mseal apply logic Lorenzo Stoakes
2025-07-14 15:26   ` Pedro Falcato

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=cd3516af-8481-4418-9f72-a7738a9fd024@lucifer.local \
    --to=lorenzo.stoakes@oracle.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=jannh@google.com \
    --cc=jeffxu@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=pfalcato@suse.de \
    --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;
as well as URLs for NNTP newsgroup(s).