Re: [PATCH v5 8/9] mm/memory-failure: add hwpoison_boot_page() to flag an inherited frame

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: memory_failure_stats data race in hwpoison_boot_page()
--

commit 95ab593413859ad843df67ede9095a08529eb0cd
Author: Breno Leitao <leitao@debian.org>

mm/memory-failure: add hwpoison_boot_page() to flag an inherited frame

This commit adds a helper, hwpoison_boot_page(), to flag inherited frames
recorded as poisoned by the previous kernel before they reach the allocator.
It leaves the frame in the state expected for poisoned frames, allowing
existing PG_hwpoison logic to cover it appropriately.

> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index d9b8be696aac3..60e9682434700 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
[ ... ]
> @@ -137,6 +137,32 @@ phys_addr_t range_last_hwpoison(phys_addr_t start, unsigned long size)
>  	return range_hwpoison(start, size, false);
>  }
>  
> +static void update_per_node_mf_stats(unsigned long pfn, enum mf_result result);
> +
> +void __meminit hwpoison_boot_page(struct page *page,
> +				  enum meminit_context context)
> +{
> +	unsigned long pfn = page_to_pfn(page);
> +
> +	if (PageHWPoison(page))
> +		return;
> +
> +	SetPageHWPoison(page);
> +	set_page_count(page, 1);
> +	/* The page has been completely isolated == MF_RECOVERED */
> +	update_per_node_mf_stats(pfn, MF_RECOVERED);

[Severity: Medium]
Is it safe to call update_per_node_mf_stats() here without acquiring mf_mutex?

During memory hotplug (when context == MEMINIT_HOTPLUG), if
hwpoison_boot_page() is called for inherited poisoned frames, it appears this
could race with runtime hardware error processing on other CPUs.

Other paths like memory_failure() update these same per-node statistics while
holding mf_mutex. Because update_per_node_mf_stats() performs non-atomic
increments:

mm/memory-failure.c:update_per_node_mf_stats() {
        ...
        case MF_RECOVERED:
                ++mf_stats->recovered;
                break;
        ...
        ++mf_stats->total;
}

Could calling it here circumvent the mutex and corrupt the memory failure
sysfs statistics by losing counter updates?

> +
> +	/*
> +	 * The per memory block half of num_poisoned_pages_inc() has no block to
> +	 * find at boot, and divides by zero looking for one. A hotplugged block
> +	 * is already there.
> +	 */
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-hwpoison-kho-v5-0-3bc7a57bd503@debian.org?part=8