Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Brendan Jackman" <brendan.jackman@linux.dev>
To: "Mike Rapoport (Microsoft)" <rppt@kernel.org>,
	"Andrew Morton" <akpm@linux-foundation.org>
Cc: "Adrian Barnaś" <abarnas@google.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Alexander Gordeev" <agordeev@linux.ibm.com>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Borislav Petkov" <bp@alien8.de>,
	"Brendan Jackman" <brendan.jackman@linux.dev>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Christian Borntraeger" <borntraeger@linux.ibm.com>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	"David Hildenbrand" <david@kernel.org>,
	"Gerald Schaefer" <gerald.schaefer@linux.ibm.com>,
	"Heiko Carstens" <hca@linux.ibm.com>,
	"Huacai Chen" <chenhuacai@kernel.org>,
	"Ingo Molnar" <mingo@redhat.com>, "Len Brown" <lenb@kernel.org>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Paul Walmsley" <pjw@kernel.org>,
	"Pavel Machek" <pavel@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Ryan Roberts" <ryan.roberts@arm.com>,
	"Sven Schnelle" <svens@linux.ibm.com>,
	"Thomas Gleixner" <tglx@kernel.org>,
	"Uladzislau Rezki" <urezki@gmail.com>,
	"Vasily Gorbik" <gor@linux.ibm.com>,
	"WANG Xuerui" <kernel@xen0n.name>,
	"Will Deacon" <will@kernel.org>,
	x86@kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-pm@vger.kernel.org, linux-riscv@lists.infradead.org,
	linux-s390@vger.kernel.org, loongarch@lists.linux.dev,
	yosry@kernel.org
Subject: Re: [PATCH 6/6] Revert "arch: introduce set_direct_map_valid_noflush()"
Date: Mon, 17 Aug 2026 17:14:19 +0200	[thread overview]
Message-ID: <DKRBF6LQY1L7.13CIBHAU0A87D@linux.dev> (raw)
In-Reply-To: <20260816-execmem-set-vm-perms-v0-2-v1-6-90944a3ad43f@kernel.org>

On Sun Aug 16, 2026 at 12:59 PM CEST, Mike Rapoport (Microsoft) wrote:
> Commit 0c6378a71574 ("arch: introduce set_direct_map_valid_noflush()")
> added set_direct_map_valid_noflush() to allow updating the direct map
> for a physically contiguous range in execmem.
>
> As Brendan recently pointed out [1], this API is confusing because on
> arm64 it means that is sets VALID bit in ptes, while on other
> architectures it is an analog of set_direct_map_default_noflush().
>
> The only user of set_direct_map_valid_noflush() was execmem's ROX cache
> freeing path and it was switched to utilize VM_FLUSH_RESET_PERMS for
> resetting permissions of the direct map alias.
>
> With the last user gone and with set_direct_map_{invalid,default}_noflush()
> accepting number of pages as a parameter, set_direct_map_valid_noflush()
> become a copy of set_memory_valid() on arm64 and a duplicate of
> set_direct_map_{invalid,default}_noflush() on other architecture, it is
> safe to remove set_direct_map_valid_noflush().
>
> Also drop a stale comment in arm64::__kernel_map_pages() that Linus
> bothered to add when merging changes containing set_direct_map_valid_noflush()
> to his tree.
>
> This reverts commit 0c6378a71574daa6cd1534ad42a956e3262756c7.
>
> [1] https://lore.kernel.org/all/DJ69RCVRBO0Y.3JCYSW50IC4RC@linux.dev
>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Quick dump of my understanding (ignoring the NG bit on arm64)

- set_direct_map_invalid_noflush():

    x86: clear P and RW (and DIRTY)
    arm64: clear VALID

  So these look out of sync to me

- set_direct_map_default_noflush():

    x86: set P and RW
    arm64: set VALID and WRITE, clear RDONLY

- set_direct_map_valid_noflush(..., true):

    x86: exactly the same as set_direct_map_default_noflush()
         (but with a `nr` arg)
    arm64: set VALID

- set_direct_map_valid_noflush(..., false):

    both archs: exactly the same as set_direct_map_invalid_noflush()
         (but with a `nr` arg)

So basically the big issue here is specifically that
set_direct_map_valid_noflush(..., true) is special on arm64 and not x86.

And we fix that by jut deleting the API. SGTM!

The other issue I can see here is that set_direct_map_invalid_noflush()
clears RW on x86 but doesn't set RDONLY on arm64. So if you unmap
something using set_direct_map_invalid_noflush(), then map it again
using something other than set_direct_map_default_noflush(), you get
different behaviour between the archs.

I think the answer to that is probably: doing that is a bug, i.e.
_invalid_noflush() and _default_noflush() are a pair that you have to
use together. But I haven't checked if this is currently the case. Maybe
it would still make sense to just align these fully?

Anyway, aside from all this yapping, getting rid of _valid_noflush()
seems like an unambiguous win here so thanks for the cleanup!

Reviewed-by: Brendan Jackman <brendan.jackman@linux.dev>


      reply	other threads:[~2026-08-17 15:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-16 10:59 [PATCH 0/6] arch, mm/execmem: resolve confusion about set_direct_map_valid_noflush() Mike Rapoport (Microsoft)
2026-08-16 10:59 ` [PATCH 1/6] set_memory: add number of pages parameter to set_direct_map APIs Mike Rapoport (Microsoft)
2026-08-17 12:10   ` Brendan Jackman
2026-08-16 10:59 ` [PATCH 2/6] mm/vmalloc: set area's page_order after allocation succeeds Mike Rapoport (Microsoft)
2026-08-16 10:59 ` [PATCH 3/6] mm/vmalloc: constify vm parameter of get_vm_area_page_order() Mike Rapoport (Microsoft)
2026-08-16 10:59 ` [PATCH 4/6] mm/vmalloc: make set_area_direct_map HUGE_VMAP friendly Mike Rapoport (Microsoft)
2026-08-17 12:21   ` Brendan Jackman
2026-08-16 10:59 ` [PATCH 5/6] mm/execmem: use VM_FLUSH_RESET_PERMS for ROX cache allocations Mike Rapoport (Microsoft)
2026-08-16 10:59 ` [PATCH 6/6] Revert "arch: introduce set_direct_map_valid_noflush()" Mike Rapoport (Microsoft)
2026-08-17 15:14   ` Brendan Jackman [this message]

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=DKRBF6LQY1L7.13CIBHAU0A87D@linux.dev \
    --to=brendan.jackman@linux.dev \
    --cc=abarnas@google.com \
    --cc=agordeev@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex@ghiti.fr \
    --cc=aou@eecs.berkeley.edu \
    --cc=borntraeger@linux.ibm.com \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=chenhuacai@kernel.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@kernel.org \
    --cc=gerald.schaefer@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=hpa@zytor.com \
    --cc=kernel@xen0n.name \
    --cc=lenb@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=loongarch@lists.linux.dev \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=palmer@dabbelt.com \
    --cc=pavel@kernel.org \
    --cc=peterz@infradead.org \
    --cc=pjw@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rppt@kernel.org \
    --cc=ryan.roberts@arm.com \
    --cc=svens@linux.ibm.com \
    --cc=tglx@kernel.org \
    --cc=urezki@gmail.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    --cc=yosry@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox