Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Unaligned physical offsets or lengths can cause PG_hwpoison to be prematurely cleared on partially poisoned pages. - [High] pmem_mkpage_present() fails to clear PG_hwpoison and MCE nospec state for poisoned tail pages when FS-DAX compound pages are used. -- commit 70db5695d2bfd34aa8380a01c117956722afebc5 Author: Muchun Song <[email protected]> nvdimm/pmem: avoid HWPoison flag updates for clean pages This commit updates pmem_mkpage_present() to avoid atomic clear operations on clean PFNs. It checks PageHWPoison() before clearing the poison state, preventing conflicts with FS-DAX vmemmap optimizations where clean PFNs might be backed by shared read-only metadata. > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c > index 30a51c365ce8b..b14f75daeda78 100644 > --- a/drivers/nvdimm/pmem.c > +++ b/drivers/nvdimm/pmem.c > @@ -80,7 +80,7 @@ static void pmem_mkpage_present(struct pmem_device *pmem, > phys_addr_t offset, [Severity: High] This isn't a bug introduced by this patch, but unaligned physical offsets or lengths can cause PG_hwpoison to be prematurely cleared on partially poisoned pages. If pmem_do_write() processes a block I/O bio segment and calls pmem_clear_poison(pmem, pmem_off, len) with an unaligned physical offset and length resulting from a partial page write, it will eventually call: pmem_mkpage_present(pmem, offset, cleared) Which calculates the bounds earlier in this function: pfn_start = PHYS_PFN(phys); pfn_end = pfn_start + PHYS_PFN(len); This truncates the physical address bounds. Does this mean the loop will call test_and_clear_pmem_poison() for an entire PFN, even if only a subset of its bytes were actually overwritten and cleared by hardware, causing the kernel to lose memory failure tracking for the remaining poisoned bytes? > * here since we're in the driver I/O path and > * outstanding I/O requests pin the dev_pagemap. > */ > - if (test_and_clear_pmem_poison(page)) > + if (PageHWPoison(page) && test_and_clear_pmem_poison(page)) [Severity: High] This is a pre-existing issue, but this conditional checks the tail page directly rather than the compound head, failing to match the state set during the memory failure. If a hardware memory failure occurs on a tail page of an FS-DAX compound folio, memory_failure_dev_pagemap() sets PG_hwpoison exclusively on the head page. Because PageHWPoison(page) relies on PF_ANY which strictly checks the provided page struct (the tail page), it evaluates to false. Since this patch modifies this condition by adding PageHWPoison(page) without resolving the compound page interaction, will clear_mce_nospec(pfn) be skipped for the tail page, causing the head page to permanently retain the PG_hwpoison flag? > clear_mce_nospec(pfn); > } > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
