Re: [PATCH v3 1/3] mm/memory-failure: userspace controls soft-offlining pages

[Jiaqi Yan <jiaqiyan@google.com>]

On Mon, Jun 17, 2024 at 12:13 PM Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> On Mon, 17 Jun 2024 17:05:43 +0000 Jiaqi Yan <jiaqiyan@google.com> 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.

>
> > +             put_ref_page(pfn, flags);
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> >       mutex_lock(&mf_mutex);
> >
>
>