On 2024/6/15 0:30, Jiaqi Yan wrote:
> On Fri, Jun 14, 2024 at 1:35 AM Lance Yang <ioworker0@xxxxxxxxx> wrote:
>>
>> Hi Jiaqi,
>>
>> On Wed, Jun 12, 2024 at 5:56 AM 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 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
>
> Will fix this typo in v3.
>
>>
>>>
>>> Signed-off-by: Jiaqi Yan <jiaqiyan@xxxxxxxxxx>
>>> ---
>>> 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.
>
> Miaohe suggested -EOPNOTSUPP. I agree both EOPNOTSUPP and EPERM may be
> better than EINVAL. But I wonder how about EAGAIN? With EAGAIN plus
> showing "disabled by /proc/sys/vm/enable_soft_offline" in dmesg, users
> now should be clear that they can try again with
> /proc/sys/vm/enable_soft_offline=1.
IMHO, it might not be suitable to use EAGAIN. Because it means "Resource temporarily unavailable" and
this can be solved by simply retry later without any further actions taken. But I might be wrong.
Thanks.
.
