On Mon, Jun 17, 2024 at 8:01 PM Miaohe Lin <linmiaohe@xxxxxxxxxx> wrote:
>
> On 2024/6/18 7:17, Jiaqi Yan wrote:
> > On Mon, Jun 17, 2024 at 12:13 PM Andrew Morton
> > <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> >>
> >> On Mon, 17 Jun 2024 17:05:43 +0000 Jiaqi Yan <jiaqiyan@xxxxxxxxxx> wrote:
> >>
> >>> Correctable memory errors are very common on servers with large
> >>> amount of memory, and are corrected by ECC. Soft offline is kernel's
> >>> additional recovery handling for memory pages having (excessive)
> >>> corrected memory errors. Impacted page is migrated to a healthy page
> >>> if it is in-use; the original page is discarded for any future use.
> >>>
> >>> The actual policy on whether (and when) to soft offline should be
> >>> maintained by userspace, especially in case of an 1G HugeTLB page.
> >>> Soft-offline dissolves the HugeTLB page, either in-use or free, into
> >>> chunks of 4K pages, reducing HugeTLB pool capacity by 1 hugepage.
> >>> If userspace has not acknowledged such behavior, it may be surprised
> >>> when later failed to mmap hugepages due to lack of hugepages.
> >>> In case of a transparent hugepage, it will be split into 4K pages
> >>> as well; userspace will stop enjoying the transparent performance.
> >>>
> >>> In addition, discarding the entire 1G HugeTLB page only because of
> >>> corrected memory errors sounds very costly and kernel better not
> >>> doing under the hood. But today there are at least 2 such cases
> >>> doing so:
> >>> 1. GHES driver sees both GHES_SEV_CORRECTED and
> >>> CPER_SEC_ERROR_THRESHOLD_EXCEEDED after parsing CPER.
> >>> 2. RAS Correctable Errors Collector counts correctable errors per
> >>> PFN and when the counter for a PFN reaches threshold
> >>> In both cases, userspace has no control of the soft offline performed
> >>> by kernel's memory failure recovery.
> >>>
> >>> This commit gives userspace the control of softofflining any page:
> >>> kernel only soft offlines raw page / transparent hugepage / HugeTLB
> >>> hugepage if userspace has agreed to. The interface to userspace is a
> >>> new sysctl at /proc/sys/vm/enable_soft_offline. By default its value
> >>> is set to 1 to preserve existing behavior in kernel. When set to 0,
> >>> soft-offline (e.g. MADV_SOFT_OFFLINE) will fail with EOPNOTSUPP.
> >>>
> >>
> >> Seems reasonable. A very simple patch.
> >
> > Thanks for taking a look, Andrew!
> >
> >>
> >> Is there sufficient instrumentation in place for userspace to be able
> >> to know that these errors are occurring? To be able to generally
> >> monitor the machine's health?
> >
> > For corrected memory errors, in general they are available in kernel
> > logs. On X86 Machine Check handling will log unparsed MCs (one needs
> > to read mci_status to know what exactly the error is). On ARM, GHES
> > logs parsed CPER (already containing error type and error severity).
> > The shortcoming is logs are rate limited. So in a burst of corrected
> > memory errors the user may not be able to figure out exactly how many
> > there were.
> >
> > For uncorrectable memory errors, num_poisoned_pages is a reliable counter.
> >
> >>
> >>> @@ -2783,6 +2795,12 @@ int soft_offline_page(unsigned long pfn, int flags)
> >>> return -EIO;
> >>> }
> >>>
> >>> + if (!sysctl_enable_soft_offline) {
> >>> + pr_info("%#lx: OS-wide disabled\n", pfn);
> >>
> >> This doesn't seem a very good message. There's no indication that it
> >> comes from the memory failure code at all. If the sysadmin sees this
> >> come out in the kernels logs, he/she will have to grep the kernel
> >> sources just to figure out where the message came from. Perhaps we can
> >> be more helpful here..
> >
> > For sure. I took it for granted that any pr_info will have the "Memory
> > failure: " prefix, but now realize there is a `#undef pr_fmt` +
> > `#define pr_fmt(fmt) "" fmt` just above unpoison_memory.
> >
> > I propose to do `#define pr_fmt(fmt) "Soft offline: " fmt` above
> > mf_isolate_folio, so that any soft-offline related code generates logs
> > with the same following format:
> >
> > "Soft offline: 0x${pfn}: ${detailed_message}"
> >
> > If everyone thinks this is reasonable, in v4 I can insert a new commit
> > to make the log formats unified.
>
> This sounds fine to me. And even better, `#define pr_fmt(fmt) "Unpoison: " fmt` can
> also be done just above unpoison_memory.
Of course. I just sent out a standalone patch for unpoison_memory to you.
>
> Thanks.
> .
>
