Re: Linux 6.18-rc6

[Linus Torvalds]

On Mon, 17 Nov 2025 at 11:17, David Hildenbrand (Red Hat)
<david@xxxxxxxxxx> wrote:
>
> So, I briefly tried on x86 with KASAN and the one-liner. I was assuming
> that KASAN would complain because we are clearing the page before doing
> the kasan_unpoison_pages() (IOW, writing to a KASAN-poisoned page).
>
> It didn't trigger, and I assume it is because clear_highpage() on x86
> will not be instrumented by KASAN (my theory).
>
> The comment in kernel_init_pages() indicates that s390x uses memset()
> for that purpose and I would assume that that one would be instrumented.

So I have thought about this some more, and I am not entirely happy
about any of this, but I think the way forward is to

 (a) make tag_clear_highpage() just do multiple pages in one go (and
rename it as tag_clear_highpage*s*() in the process)

 (b) make it have an actually return value to indicate whether it
initialized things

which means that the post_alloc_hook() code just becomes

        if (zero_tags)
                init = tag_clear_highpages(page, 1 << order);

and then the generic fallback becomes just

  static inline bool tag_clear_highpages(struct page *page, int numpages)
  {
         return false;
  }

which makes this all a complete no-op for architectures that don't do
this memory tagging.

And the one architecture that *does* do it - arm64 - actually
simplifies too, because now instead of being called in a loop - and
having that

        if (!system_supports_mte()) {
                 clear_highpage(page);
                 return;
        }

in every iteration of the loop, it now just gets called *once*, and it
instead just does

        if (!system_supports_mte())
                return false;

and then it does the *clearing* in a loop instead.

End result: that all looks much saner to me, and should avoid all the
issues with KASAN (well, arm64 currently clearly depends on
mte_zero_clear_page_tags() being assembly code that doesn't trigger
KASAN anyway).

But maybe it looks saner to me just because I've written that code now.

Anyway, here's my suggested patch. I still prefer this over having
more config variables and #ifdef's. I'd much rather have code that
just does the right thing and becomes null and void when it's
effecitlvely disabled by not having hardware support.

Comments?

This is all entirely untested, but I did build it on both x86-64 and
arm64. So it must be perfect. Right?

Side note: I really *detest* that stupid "__HAVE_ARCH_XYZ" pattern. I
hate it. Why do people insist on that stupid pattern? We *have* a name
already: the name of the thing that the architecture implements. Don't
make up a new one with all caps and a __HAVE_ARCH_ prefix. If an
architecture implements the feature "xyz", it should just do "define
xyz xyz" and be done with it, and then code can test whether it is
implemented by just doing "#ifdef xyz".

But I did *not* change that stupid existing pattern. I left it alone,
and just added the 'S' since now it's multiple pages.  But I really do
want to bring this up again, because it's so silly to make up new
names to say "I defined that other name". Just *use* the name.

If you implement "xyz" as a macro, you're done. And if it's
implemented as an inline function, just add the "#define xyz xyz" to
show that you did it.

Don't make up new names that only makes it harder to grep for things,
and makes things pointlessly have two different names.

Please.

              Linus
 arch/arm64/include/asm/page.h |  4 ++--
 arch/arm64/mm/fault.c         | 21 +++++++++++----------
 include/linux/highmem.h       |  6 ++++--
 mm/page_alloc.c               |  9 ++-------
 4 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
index 2312e6ee595f..258cca4b4873 100644
--- a/arch/arm64/include/asm/page.h
+++ b/arch/arm64/include/asm/page.h
@@ -33,8 +33,8 @@ struct folio *vma_alloc_zeroed_movable_folio(struct vm_area_struct *vma,
 						unsigned long vaddr);
 #define vma_alloc_zeroed_movable_folio vma_alloc_zeroed_movable_folio
 
-void tag_clear_highpage(struct page *to);
-#define __HAVE_ARCH_TAG_CLEAR_HIGHPAGE
+bool tag_clear_highpages(struct page *to, int numpages);
+#define __HAVE_ARCH_TAG_CLEAR_HIGHPAGES
 
 #define clear_user_page(page, vaddr, pg)	clear_page(page)
 #define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 125dfa6c613b..a193b6a5d1e6 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -967,20 +967,21 @@ struct folio *vma_alloc_zeroed_movable_folio(struct vm_area_struct *vma,
 	return vma_alloc_folio(flags, 0, vma, vaddr);
 }
 
-void tag_clear_highpage(struct page *page)
+bool tag_clear_highpages(struct page *page, int numpages)
 {
 	/*
 	 * Check if MTE is supported and fall back to clear_highpage().
 	 * get_huge_zero_folio() unconditionally passes __GFP_ZEROTAGS and
-	 * post_alloc_hook() will invoke tag_clear_highpage().
+	 * post_alloc_hook() will invoke tag_clear_highpages().
 	 */
-	if (!system_supports_mte()) {
-		clear_highpage(page);
-		return;
-	}
+	if (!system_supports_mte())
+		return false;
 
-	/* Newly allocated page, shouldn't have been tagged yet */
-	WARN_ON_ONCE(!try_page_mte_tagging(page));
-	mte_zero_clear_page_tags(page_address(page));
-	set_page_mte_tagged(page);
+	/* Newly allocated pages, shouldn't have been tagged yet */
+	for (int i = 0; i < numpages; i++, page++) {
+		WARN_ON_ONCE(!try_page_mte_tagging(page));
+		mte_zero_clear_page_tags(page_address(page));
+		set_page_mte_tagged(page);
+	}
+	return true;
 }
diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index 105cc4c00cc3..abc20f9810fd 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -249,10 +249,12 @@ static inline void clear_highpage_kasan_tagged(struct page *page)
 	kunmap_local(kaddr);
 }
 
-#ifndef __HAVE_ARCH_TAG_CLEAR_HIGHPAGE
+#ifndef __HAVE_ARCH_TAG_CLEAR_HIGHPAGES
 
-static inline void tag_clear_highpage(struct page *page)
+/* Return false to let people know we did not initialize the pages */
+static inline bool tag_clear_highpages(struct page *page, int numpages)
 {
+	return false;
 }
 
 #endif
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 600d9e981c23..4319cfa7f77d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1822,14 +1822,9 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
 	 * If memory tags should be zeroed
 	 * (which happens only when memory should be initialized as well).
 	 */
-	if (zero_tags) {
-		/* Initialize both memory and memory tags. */
-		for (i = 0; i != 1 << order; ++i)
-			tag_clear_highpage(page + i);
+	if (zero_tags)
+		init = tag_clear_highpages(page, 1 << order);
 
-		/* Take note that memory was initialized by the loop above. */
-		init = false;
-	}
 	if (!should_skip_kasan_unpoison(gfp_flags) &&
 	    kasan_unpoison_pages(page, order, init)) {
 		/* Take note that memory was initialized by KASAN. */