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

["Luck, Tony" <tony.luck@intel.com>]

On Mon, Sep 08, 2025 at 02:14:42PM -0500, Kyle Meyer wrote:
> 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.

Seems like a solid plan.

Needs an update to Documentation/admin-guide/sysctl/vm.rst
to match the new functionality and describe the reason that
users might want to avoid taking huge pages offline.

Also this line in existing text:

- On ARM, the request to soft offline pages from GHES driver.

should read:

- On ARM and X86, the request to soft offline pages from GHES driver.

> 
> Maybe something like...
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index fc30ca4804bf..efa535d405a8 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -64,11 +64,17 @@
>  #include "internal.h"
>  #include "ras/ras_event.h"
>  
> +enum soft_offline {
> +	SOFT_OFFLINE_DISABLED = 0,
> +	SOFT_OFFLINE_ENABLED_SKIP_HUGEPAGES,
> +	SOFT_OFFLINE_ENABLED
> +};
> +
>  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;
> +static int sysctl_enable_soft_offline __read_mostly = SOFT_OFFLINE_SKIP_HUGEPAGES;

This is changing the default behavior (which allowed offline
of huge pages). I'm in favor of the change. But you should call it
out explicitly in the commit message when you write up a patch.

>  
>  atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);
>  
> @@ -150,7 +156,7 @@ static const struct ctl_table memory_failure_table[] = {
>  		.mode		= 0644,
>  		.proc_handler	= proc_dointvec_minmax,
>  		.extra1		= SYSCTL_ZERO,
> -		.extra2		= SYSCTL_ONE,
> +		.extra2		= SYSCTL_TWO,
>  	}
>  };
>  
> @@ -2799,12 +2805,20 @@ int soft_offline_page(unsigned long pfn, int flags)
>  		return -EIO;
>  	}
>  
> -	if (!sysctl_enable_soft_offline) {
> -		pr_info_once("disabled by /proc/sys/vm/enable_soft_offline\n");
> +	if (sysctl_enable_soft_offline == SOFT_OFFLINE_DISABLED) {
> +		pr_info("disabled by /proc/sys/vm/enable_soft_offline\n");
>  		put_ref_page(pfn, flags);
>  		return -EOPNOTSUPP;
>  	}
>  
> +	if (sysctl_enable_soft_offline == SOFT_OFFLINE_ENABLED_SKIP_HUGEPAGES) {
> +		if (folio_test_hugetlb(pfn_folio(pfn))) {
> +			pr_info("disabled by /proc/sys/vm/enable_soft_offline\n");
> +			put_ref_page(pfn, flags);
> +			return -EOPNOTSUPP;
> +		}
> +	}
> +
>  	mutex_lock(&mf_mutex);
>  
>  	if (PageHWPoison(page)) {
> 
> > > I don't know, the patch itself is fine, it's the issue that it has
> > > exposed that is more concerning.
> 
> Thanks,
> Kyle Meyer

-Tony