Message-ID: <1b2ab63f.a8c2.19a974009bc.Coremail.00107082@163.com>
Date: Tue, 18 Nov 2025 21:55:47 +0800 (CST)
From: "David Wang" <00107082@....com>
To: "Linus Torvalds" <torvalds@...ux-foundation.org>
Cc: "David Hildenbrand (Red Hat)" <david@...nel.org>,
	catalin.marinas@....com, lance.yang@...ux.dev, b-padhi@...com,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	"Jan Polensky" <japo@...ux.ibm.com>
Subject: Re: Linux 6.18-rc6



At 2025-11-18 12:13:15, "David Wang" <00107082@....com> wrote:
>
>
>At 2025-11-18 09:10:50, "Linus Torvalds" <torvalds@...ux-foundation.org> wrote:
>>On Mon, 17 Nov 2025 at 11:17, David Hildenbrand (Red Hat)
>><david@...nel.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.


Update: with this patch, my go programs still crash, It was just  that
the first time I test the patch, old go compiler happened to work. When I reboot, my
go program start to crash again.  The crash seems random, but on my system,
go program crashes with *very* high probability.

(And I applied the patch based on 6.18-rc6.)

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

Powered by blists - more mailing lists