Re: Linux 6.18-rc6

["David Wang" <00107082@163.com>]

At 2025-11-18 09:10:50, "Linus Torvalds" <torvalds@linux-foundation.org> wrote:
>On Mon, 17 Nov 2025 at 11:17, David Hildenbrand (Red Hat)
><david@kernel.org> 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?


I tried this patch, my prometheus service crash with:
        fatal error: acquireSudog: found s.elem != nil in cache
seems some memory is still not properly zeroed. (I guess)
But this time, my old go compiler works fine.


FYI
David W






>
>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