Re: [PATCH 3/3] kmsan: fix metadata loss during HugeTLB struct page reinitialization

[Muchun Song <muchun.song@linux.dev>]

> On Nov 15, 2026, at 14:23, Chris Down <chris@chrisdown.name> wrote:
> 
> Hi Muchun,

Hi,

> 
> I can reproduce the crash and the series fixes it, and I think page_ext is
> the right place for these pointers. I do have a few comments, though.
> 
> Muchun Song writes:
>> -	struct page *kmsan_shadow;
>> -	struct page *kmsan_origin;
> 
> This takes struct page from 80 bytes to 64 on a KMSAN kernel, which means
> that hugetlb_vmemmap_optimizable_size() no longer returns 0 and HVO can be
> used there for the first time. From a quick check with
> hugetlb_free_vmemmap=on it seems to work, but nobody would guess that from
> the subject, so I think the changelog should mention it.

Make sense. Going forward, KMSAN can now work with HVO.

> 
> Documentation/dev-tools/kmsan.rst also still describes the two pointers as
> members of struct page, under "Metadata allocation", so that needs
> updating as well.

Right.

> 
>> +ifdef CONFIG_KMSAN
>> +KMSAN_SANITIZE_page_ext.o	:= n
>> +KCOV_INSTRUMENT_page_ext.o	:= n
>> +UBSAN_SANITIZE_page_ext.o	:= n
>> +CFLAGS_page_ext.o		+= -DDISABLE_BRANCH_PROFILING
>> +CFLAGS_REMOVE_page_ext.o	= $(CC_FLAGS_FTRACE)
>> +endif
> 
> This takes the instrumentation away from all of page_ext.c, when the only
> thing that needs it is the lookup. page_ext_size is already visible in

Absolutely right!

> page_ext.h for page_ext_next(), so I think page_ext_lookup_raw() could be
> a static inline there instead. That way it gets compiled into
> mm/kmsan/shadow.c with the right flags already and needs none of the
> finagling above, and it saves a call on a very hot path as well.

I'll include this change in the next version.

> 
>> +	preempt_disable_notrace();
>> +	page_ext = page_ext_lookup_raw(page_to_pfn(page));
>> +	if (page_ext)
>> +		meta = *(struct page_metadata *)page_ext_data(page_ext, &kmsan_ops);
>> +	preempt_enable_no_resched_notrace();
> 
> This is quite a lot more work than the two loads from struct page that it
> replaces, and kmsan_get_shadow_origin_ptr() has to go through it twice
> for every instrumented access to direct-mapped memory, because it calls
> kmsan_get_metadata() once for the shadow and then again for the origin.
> You might think that's a bit academic, but it does add up. For example, on
> a 2 CPU VM, perf bench sched pipe -l 30000 goes from about 1.37s to about
> 1.58s with the series applied, perf bench syscall basic -l 300000 from
> about 0.66s to about 0.75s, and a dd of 300000 blocks of 512 bytes from
> /dev/zero to /dev/null from about 1.63s to about 1.90s.

Good catch, and the numbers are hard to argue with.

> 
> 1/3 says that keeping the pair together lets callers avoid duplicate
> lookups, but the caller that matters most doesn't make use of that yet.

Sorry, that's my oversight.

> From a quick hack that has kmsan_get_shadow_origin_ptr() look the page up
> once and take both pointers from the result, the same three come out at
> about 1.38s, 0.62s and 1.54s, which is no worse than before the series.
> Could you do something like that here?

That's a fair point. I'll amend patch 1 so that kmsan_get_shadow_origin_ptr()
uses the shadow/origin pair introduced there and obtains both pointers from a
single page metadata lookup.

Thanks very much for your review.

Muchun

> 
> Thanks!
> 
> Chris