Re: [PATCH v2 5/6] ACPI: APEI: add separate Memory Error Section 2 handlers

[Pierre Gondois]

Hi Ashok,

On 10/24/26 23:50, Ashok Raj wrote:
> Hi Pierre,
>
>
> On Mon, Oct 19, 2026 at 10:38:44PM +0200, Pierre Gondois wrote:
> > Add dedicated GHES and EDAC callbacks for Memory Error Section 2
> > records. Use a separate GHES notifier chain so existing consumers
> > continue to receive legacy memory error sections.
> >
> > Signed-off-by: Pierre Gondois <pierre.gondois@xxxxxxx>
> > ---
> >   drivers/acpi/apei/ghes.c |  49 +++++++++++++++-
> >   drivers/edac/ghes_edac.c | 118 +++++++++++++++++++++++++++++++++++++++
> >   include/acpi/ghes.h      |   2 +
> >   3 files changed, 168 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> > index fe10ab0e02f6..c7fb15543b56 100644
> > --- a/drivers/acpi/apei/ghes.c
> > +++ b/drivers/acpi/apei/ghes.c
> > @@ -101,6 +101,7 @@
> >   #endif
> [snip]
>
> > @@ -952,6 +977,16 @@ static void ghes_do_proc(struct ghes *ghes,
> >   
> >   			arch_apei_report_mem_error(sev, mem_err);
> >   			queued = ghes_handle_memory_failure(gdata, sev, sync);
> One issue here.
>
> > +		} else if (guid_equal(sec_type, &CPER_SEC_PLATFORM_MEM2)) {
> > +			struct cper_sec_mem_err2 *mem_err = acpi_hest_get_payload(gdata);
> > +
> > +			if (gdata->error_data_length < sizeof(*mem_err)) {
> > +				pr_warn(FW_WARN "Memory Error Section 2 length is too small\n");
> > +				continue;
> > +			}
> > +
> > +			atomic_notifier_call_chain(&ghes_report_mem2_chain, sev, mem_err);
> > +			queued = ghes_handle_memory_failure2(gdata, sev, sync);
> could this accumulate queued with |= ?
>
> queued describes whether any section in this GHES status record queued
> memory-failure work. The synchronous GHES path sends SIGBUS after the
> section loop when `!queued`.
>
> For example, a synchronous record can contain an eligible legacy memory
> section followed by a Memory Error Section 2 record without a valid
> physical address. The legacy section queues memory-failure work and sets
> queued, but the Section 2 handler returns false and this assignment clears
> the previous result. ghes_do_proc() then reaches the `sync && !queued`
> path and sends SIGBUS despite having queued recovery work for the earlier
> section.
>
> This series appears to be based before the queued accumulation fix [1].
> Could the new Memory Error Section 2 branch follow the same pattern?
>
> > +                     queued = ghes_handle_memory_failure2(gdata, sev, sync);
> should be:
>
>          queued |= ghes_handle_memory_failure2(gdata, sev, sync);
>
> Otherwise, an earlier section can queue memory-failure work, while a later
> Section 2 record for which no work is queued clears `queued`. For a
> synchronous GHES record, ghes_do_proc() will then reach `sync && !queued`
> and send SIGBUS despite recovery work having been queued for the earlier
> section.
>
> Thoughts?
>
> [1] https://lore.kernel.org/r/20260925-ghes-queued-accumulate-v1-1-df50553957d4@xxxxxxxxxx/
Ok sure, I will rebase the serie and do similar changes