Re: [PATCH 2/4] nvdimm/pmem: avoid HWPoison flag updates for clean pages

[sashiko-bot@kernel.org]

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 <songmuchun@bytedance.com>

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/20260903122128.12264-1-songmuchun@bytedance.com?part=2