Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] mm: panic when accepting clean pages sharing an unaccepted unit with poisoned pages - [Medium] mm: lost memory failure stats due to lockless update in hwpoison_boot_page() --
commit 92cea9db628cbe8f9651b225628684a39c80ddd4 Author: Breno Leitao <[email protected]> Subject: mm/memory-failure: keep inherited poisoned frames out of the buddy allocator This commit intercepts inherited poisoned pages before they enter the buddy allocator. It processes memory blocks by checking for poisoned pages, flagging them, and passing the remaining clean pages through the allocator. This prevents unaccepted memory blocks with poisoned frames from being mistakenly re-merged. > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index b07b5f4751cb9..cca67a2702a42 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c [ ... ] > @@ -1597,6 +1597,25 @@ static void __meminit accept_and_free_block(struct > page *page, > __free_pages_ok(page, order, FPI_TO_TAIL); > } > > +static void __meminit free_poisoned_block(struct page *page, unsigned int > order, > + enum meminit_context context) > +{ > + unsigned long i, nr_pages = 1UL << order; > + > + for (i = 0; i < nr_pages; i++) { > + struct page *p = page + i; > + phys_addr_t phys = page_to_phys(p); > + > + if (range_contains_poisoned_memory(phys, PAGE_SIZE)) { > + hwpoison_boot_page(p, context); [Severity: Medium] Is this call to hwpoison_boot_page() safe from data races during memory hotplug? When called during hotplug (where context == MEMINIT_HOTPLUG), hwpoison_boot_page() calls update_per_node_mf_stats() which performs non-atomic read-modify-write operations on the node stats: mm/memory-failure.c:update_per_node_mf_stats() { ... ++mf_stats->recovered; ... } Since hwpoison_boot_page() does not acquire mf_mutex, could a concurrent memory_failure() execution on the same NUMA node corrupt the recovered and total counters? > + continue; > + } > + > + /* this part of the block is not poisoned */ > + accept_and_free_block(p, 0); [Severity: High] Can this sequence cause a kernel panic when accepting clean pages that share an unaccepted memory unit with poisoned pages? When a clean page belongs to an unaccepted memory region, accept_and_free_block() invokes accept_memory(). However, accept_memory() operates at the unit_size granularity (typically 2MB) and rounds down the start address to this boundary before calling arch_accept_memory(): drivers/firmware/efi/unaccepted_memory.c:accept_memory() { ... arch_accept_memory(phys_start, phys_end); ... } Because the clean page and the poisoned page share the same unaccepted unit_size block, won't arch_accept_memory() be inadvertently executed on the poisoned page as well? If arch_accept_memory() attempts to accept physically poisoned memory (such as via TDX TDG.MEM.PAGE.ACCEPT), could this trigger an unrecoverable Machine Check Exception? > + } > +} > + [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
