Re: Linux 6.18-rc6
From: David Hildenbrand (Red Hat)
Date: Tue Nov 18 2025 - 02:37:58 EST
On 18.11.25 02:10, Linus Torvalds wrote:
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)
That sounds reasonable given that the only caller we have wants to iterate.
(b) make it have an actually return value to indicate whether it
initialized things
Works for me.
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.
Ack.
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.
:)
It should optimize out on !arm64 and optimize arm64 as well (less function calls for higher-order pages), so that's clearly better.
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?
Works for me and saves me from continuing my fight with KASAN on s390x I started yesterday evening to find out if the one-liner would be problematic with KASAN poisoning.
I very much prefer to let kernel_init_pages() handle ordinary (non-tag) initialization after KASAN did its unpoison magic.
Do you want to quickly send that patch with linux-mm on CC or do you just want to commit it? If you're busy I can quickly send it around.
In any case, feel free to add my
Reviewed-by: David Hildenbrand (Red Hat) <david@xxxxxxxxxx>
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.
I stumbled over that just recently myself, and it's just done extremely inconsistently even within MM.
Maybe this one is worth spelling out in the coding style, as I was recently also unsure what the best practice is in the end. Let me see if I can find time for that.
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.
I general, I agree if it's about real "features" that consist of a single function. I think it's a different story once a feature actually consists of multiple functions that can be cleanly abstracted in a config option.
--
Cheers
David