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

[Lance Yang <ioworker0@gmail.com>]

Hi Jiaqi,

On Wed, Jun 12, 2024 at 5:56 AM 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 inuse; 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 mmap hugepages MAP_FAILED 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:
> 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 called enable_soft_offline under /proc/sys/vm. By default
> enable_soft_line is 1 to preserve existing behavior in kernel.

s/enable_soft_line/enable_soft_offline

>
> Signed-off-by: Jiaqi Yan <jiaqiyan@google.com>
> ---
>  mm/memory-failure.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index d3c830e817e3..23415fe03318 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -68,6 +68,8 @@ static int sysctl_memory_failure_early_kill __read_mostly;
>
>  static int sysctl_memory_failure_recovery __read_mostly = 1;
>
> +static int sysctl_enable_soft_offline __read_mostly = 1;
> +
>  atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);
>
>  static bool hw_memory_failure __read_mostly = false;
> @@ -141,6 +143,15 @@ static struct ctl_table memory_failure_table[] = {
>                 .extra1         = SYSCTL_ZERO,
>                 .extra2         = SYSCTL_ONE,
>         },
> +       {
> +               .procname       = "enable_soft_offline",
> +               .data           = &sysctl_enable_soft_offline,
> +               .maxlen         = sizeof(sysctl_enable_soft_offline),
> +               .mode           = 0644,
> +               .proc_handler   = proc_dointvec_minmax,
> +               .extra1         = SYSCTL_ZERO,
> +               .extra2         = SYSCTL_ONE,
> +       }
>  };
>
>  /*
> @@ -2771,6 +2782,11 @@ int soft_offline_page(unsigned long pfn, int flags)
>         bool try_again = true;
>         struct page *page;
>
> +       if (!sysctl_enable_soft_offline) {
> +               pr_info("soft offline: %#lx: OS-wide disabled\n", pfn);
> +               return -EINVAL;

IMO, "-EPERM" might sound better ;)

Using "-EPERM" indicates that the operation is not permitted due to
the OS-wide configuration.

Thanks,
Lance

> +       }
> +
>         if (!pfn_valid(pfn)) {
>                 WARN_ON_ONCE(flags & MF_COUNT_INCREASED);
>                 return -ENXIO;
> --
> 2.45.2.505.gda0bf45e8d-goog
>
>