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

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")


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.



> +	if (!mce_usable_address(mce))
>  		return NOTIFY_DONE;
>  
>  	spa = mce->addr & MCI_ADDR_PHYSADDR;
> 
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> -- 
> 2.43.0
>