Re: PATCH v3 ACPI: APEI: GHES: Don't offline huge pages just because BIOS asked

[Shuai Xue <xueshuai@linux.alibaba.com>]

在 2025/9/18 23:43, Jiaqi Yan 写道:
> On Wed, Sep 17, 2025 at 8:39 PM Shuai Xue <xueshuai@linux.alibaba.com> wrote:
>>
>>
>>
>> 在 2025/9/9 03:14, Kyle Meyer 写道:> On Fri, Sep 05, 2025 at 12:59:00PM -0700, Jiaqi Yan wrote:
>>   >> On Fri, Sep 5, 2025 at 12:39 PM <jane.chu@oracle.com> wrote:
>>   >>>
>>   >>>
>>   >>> On 9/5/2025 11:17 AM, Luck, Tony wrote:
>>   >>>> BIOS can supply a GHES error record that reports that the corrected
>>   >>>> error threshold has been exceeded. Linux will attempt to soft offline
>>   >>>> the page in response.
>>   >>>>
>>   >>>> But "exceeded threshold" has many interpretations. Some BIOS versions
>>   >>>> accumulate error counts per-rank, and then report threshold exceeded
>>   >>>> when the number of errors crosses a threshold for the rank. Taking
>>   >>>> a page offline in this case is unlikely to solve any problems. But
>>   >>>> losing a 4KB page will have little impact on the overall system.
>>
>> Hi, Tony,
>>
>> Thank you for your detailed explanation. I believe this is exactly the problem
>> we're encountering in our production environment.
>>
>> As you mentioned, memory access is typically interleaved between channels. When
>> the per-rank threshold is exceeded, soft-offlining the last accessed address
>> seems unreasonable - regardless of whether it's a 4KB page or a huge page. The
>> error accumulation happens at the rank level, but the action is taken on a
>> specific page that happened to trigger the threshold, which doesn't address the
>> underlying issue.
>>
>> I'm curious about the intended use case for the CPER_SEC_ERROR_THRESHOLD_EXCEEDED
>> flag. What scenario was Intel BIOS expecting the OS to handle when this flag is set?
>> Is there a specific interpretation of "threshold exceeded" that would make
>> page-level offline action meaningful? If not, how about disabling soft offline from
>> GHES and leave that to userspace tools like rasdaemon (mcelog) ?
> 
> The existing /proc/sys/vm/enable_soft_offline can already entirely
> disable soft offline. GHES may still ask for soft offline to
> memory-failure.c, but soft_offline_page will discard the ask as long
> as userspace sets 0 to /proc/sys/vm/enable_soft_offline.
> 

I see. Thanks.

>>
>>   >>
>>   >> Hi Tony,
>>   >>
>>   >> This is exactly the problem I encountered [1], and I agree with Jane
>>   >> that disabling soft offline via /proc/sys/vm/enable_soft_offline
>>   >> should work for your case.
>>   >>
>>   >> [1] https://lore.kernel.org/all/20240628205958.2845610-3-jiaqiyan@google.com/T/#me8ff6bc901037e853d61d85d96aa3642cbd93b86
>>   >
>>   > If that doesn't work for your case, I just want to mention that hugepages might
>>   > still be soft offlined with that check in ghes_handle_memory_failure().
>>   >
>>   >>>>
>>   >>>> On the other hand, taking a huge page offline will have significant
>>   >>>> impact (and still not solve any problems).
>>   >>>>
>>   >>>> Check if the GHES record refers to a huge page. Skip the offline
>>   >>>> process if the page is huge.
>>   >
>>   > AFAICT, we're still notifying the MCE decoder chain and CEC will soft offline
>>   > the hugepage once the "action threshold" is reached.
>>   >
>>   > This could be moved to soft_offline_page(). That would prevent other sources
>>   > (/sys/devices/system/memory/soft_offline_page, CEC, etc.) from being able to
>>   > soft offline hugepages, not just GHES.
>>   >
>>   >>>> Reported-by: Shawn Fan <shawn.fan@intel.com>
>>   >>>> Signed-off-by: Tony Luck <tony.luck@intel.com>
>>   >>>> ---
>>   >>>>
>>   >>>> Change since v2:
>>   >>>>
>>   >>>> Me: Add sanity check on the address (pfn) that BIOS provided. It might
>>   >>>> be in some reserved area that doesn't have a "struct page" which would
>>   >>>> likely result in an OOPs if fed to pfn_folio().
>>   >>>>
>>   >>>> The original code relied on sanity check of the pfn received from the
>>   >>>> BIOS when this eventually feeds into memory_failure(). That used to
>>   >>>> result in:
>>   >>>>        pr_err("%#lx: memory outside kernel control\n", pfn);
>>   >>>> which won't happen with this change, since memory_failure is not
>>   >>>> called. Was that a useful message? A Google search mostly shows
>>   >>>> references to the code. There are few instances of people reporting
>>   >>>> they saw this message.
>>   >>>>
>>   >>>>
>>   >>>>    drivers/acpi/apei/ghes.c | 13 +++++++++++--
>>   >>>>    1 file changed, 11 insertions(+), 2 deletions(-)
>>   >>>>
>>   >>>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>>   >>>> index a0d54993edb3..c2fc1196438c 100644
>>   >>>> --- a/drivers/acpi/apei/ghes.c
>>   >>>> +++ b/drivers/acpi/apei/ghes.c
>>   >>>> @@ -540,8 +540,17 @@ static bool ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata,
>>   >>>>
>>   >>>>        /* iff following two events can be handled properly by now */
>>   >>>>        if (sec_sev == GHES_SEV_CORRECTED &&
>>   >>>> -         (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED))
>>   >>>> -             flags = MF_SOFT_OFFLINE;
>>   >>>> +         (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED)) {
>>   >>>> +             unsigned long pfn = PHYS_PFN(mem_err->physical_addr);
>>   >>>> +
>>   >>>> +             if (pfn_valid(pfn)) {
>>   >>>> +                     struct folio *folio = pfn_folio(pfn);
>>   >>>> +
>>   >>>> +                     /* Only try to offline non-huge pages */
>>   >>>> +                     if (!folio_test_hugetlb(folio))
>>   >>>> +                             flags = MF_SOFT_OFFLINE;
>>   >>>> +             }
>>   >>>> +     }
>>   >>>>        if (sev == GHES_SEV_RECOVERABLE && sec_sev == GHES_SEV_RECOVERABLE)
>>   >>>>                flags = sync ? MF_ACTION_REQUIRED : 0;
>>   >>>>
>>   >>>
>>   >>> So the issue is the result of inaccurate MCA record about per rank CE
>>   >>> threshold being crossed. If OS offline the indicted page, it might be
>>   >>> signaled to offline another 4K page in the same rank upon access.
>>   >>>
>>   >>> Both MCA and offline-op are performance hitter, and as argued by this
>>   >>> patch, offline doesn't help except loosing a already corrected page.
>>   >>>
>>   >>> Here we choose to bypass hugetlb page simply because it's huge.  Is it
>>   >>> possible to argue that because the page is huge, it's less likely to get
>>   >>> another MCA on another page from the same rank?
>>   >>>
>>   >>> A while back this patch
>>   >>> 56374430c5dfc mm/memory-failure: userspace controls soft-offlining pages
>>   >>> has provided userspace control over whether to soft offline, could it be
>>   >>> a more preferable option?
>>   >
>>   > Optionally, a 3rd setting could be added to /proc/sys/vm/enable_soft_offline:
>>   >
>>   > 0: Soft offline is disabled.
>>   > 1: Soft offline is enabled for normal pages (skip hugepages).
>>   > 2: Soft offline is enabled for normal pages and hugepages.
>>   >
>>
>> I prefer having soft-offline fully controlled by userspace, especially
>> for DPDK-style applications. These applications use hugepage mappings and maintain
>> their own VA-to-PA mappings. When the kernel migrates a hugepage to a new physical
>> page during soft-offline, DPDK continues accessing the old physical address,
>> leading to data corruption or access errors.
> 
> Just curious, does the DPDK applications pin (pin_user_pages) the
> VA-to-PA mappings? If so I would expect both soft offline and hard
> offline will fail and become no-op.
> 

I think these does. We encountered this problem in older kernel
versions. However, since it's application-specific behavior, I agree
that using enable_soft_offline for userspace control is a good solution.

Thanks.
Shuai