Re: [PATCH] cxl/mce: Only act on uncorrected memory errors

On Tue, Aug 11, 2026 at 12:22:15PM -0700, Alison Schofield wrote:
> On Tue, Aug 11, 2026 at 12:00:13AM +0530, Shaikh Kamaluddin wrote:
> > cxl_handle_mce() offlines the aliased page of an ELC region on any
> > record with a usable address; it does not check MCI_STATUS_UC or
> > filter non-memory errors. uc_decode_notifier(), the equivalent
> > handler for plain memory on the same chain at the same priority,
> > filters on mce->severity and leaves corrected errors untouched.
> > cxl_handle_mce() has no such gate, so a corrected error - which
> > the generic handler ignores - still causes the alias to be
> > permanently retired via memory_failure().
> > 
> > Corrected errors do reach the chain: machine_check_poll() logs
> > them via the same mce_gen_pool_process() path that feeds
> > x86_mce_decoder_chain, and cxl_extended_linear_cache_resize()
> > extends p->res to cover the DRAM half of the ELC pair, so a
> > routine DRAM CE carries an address inside the region resource.
> > 
> > Filter the record as nfit_handle_mce() does. Commit fc08a4703a41
> > ("acpi, nfit: Fix the memory error check in nfit_handle_mce()") and
> > commit 5d96c9342c23 ("acpi/nfit, x86/mce: Handle only uncorrectable
> > machine checks") established this filter for an equivalent handler
> > on the same notifier chain; the consequence here is more severe, as
> > the CXL handler calls memory_failure() rather than recording a bad
> > block.
> > 
> > mce_is_correctable() is used instead of copying
> > uc_decode_notifier()'s AO/DEFERRED test because the alias must
> > still be offlined on MCE_AR_SEVERITY, where kill_me_maybe() owns
> > the reported page but nothing owns the alias.
> > 
> > Fixes: 516e5bd0b6bf ("cxl: Add mce notifier to emit aliased address for extended linear cache")
> > 
> 
> Thanks for the patch Shaikh,
> 
> The patch looks good to me, and the reproduction details show that this has
> been thoroughly investigated.
> 
> The commit log is harder to follow than the code change itself. It seems
> that too much of the investigation has leaked into the commit log which
> makes it very dense and not very clarifying. For what is ultimately a
> small and straightforward behavioral fix, the log spends a lot of time
> narrating the code paths that establish the problem, while the important
> impact, a reduction in usable memory, gets lost.
> 
> The commit log should introduce the code change and make it easier to
> understand. I'd prefer the usual behavioral progression: describe what
> happens today, why that is wrong and its impact, then state how the patch
> fixes it. Also, please spell out Extended Linear Cache (ELC) on first use.
> 
> The subject can describe the behavior being fixed more directly as well.
> 
> For example: (if you use this example, you need to proofread it, to
> be sure I actually captured this correctly ;))
> 
> cxl/mce: Avoid alias page retirement for corrected errors
> 
> cxl_handle_mce() offlines the aliased page of an Extended Linear Cache
> (ELC) region for any MCE with a usable address in the region. This
> includes corrected errors and non-memory errors, needlessly reducing
> usable memory.
> 
> Restrict ELC MCE handling to uncorrected memory errors. Use
> mce_is_correctable() so that MCE_AR_SEVERITY errors continue to retire
> the alias when the reported page is handled elsewhere.
> 
> Fixes: 516e5bd0b6bf ("cxl: Add mce notifier to emit aliased address for extended linear cache")
>
Thanks for the detailed feedback, Alison - that's a much better
structure for the commit log, and matches what you captured.
> 
> more below...
> 
> 
> > Signed-off-by: Shaikh Kamaluddin <[email protected]>
> > ---
> 
> The detailed investigation and reproduction below the scissors is useful and
> should stay there.
> 
> snip
> 
> > 
> > diff --git a/drivers/cxl/core/mce.c b/drivers/cxl/core/mce.c
> > index 65fed913b221..ee70c1c9f9b9 100644
> > --- a/drivers/cxl/core/mce.c
> > +++ b/drivers/cxl/core/mce.c
> > @@ -18,7 +18,14 @@ static int cxl_handle_mce(struct notifier_block *nb, unsigned long val,
> >  	u64 spa, spa_alias;
> >  	unsigned long pfn;
> >  
> > -	if (!mce || !mce_usable_address(mce))
> > +	if (!mce)
> > +		return NOTIFY_DONE;
> > +
> > +	/* Only uncorrected memory errors warrant taking down the alias page */
> > +	if (!mce_is_memory_error(mce) || mce_is_correctable(mce))
> > +		return NOTIFY_DONE;
> > +
> 
> The comment restates what the if condition says. I think the comment can
> simply be dropped.
>
Dropped the comment from the code  and sent the v2 
https://lore.kernel.org/all/[email protected]/
>
> 
> > +	if (!mce_usable_address(mce))
> >  		return NOTIFY_DONE;
> >  
> >  	spa = mce->addr & MCI_ADDR_PHYSADDR;
> > 
> > base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> > -- 
> > 2.43.0
> >