On Mon 14-09-26 18:49:38, Kyle Meyer wrote:
> Soft offlining a HugeTLB page dissolves it, permanently reducing the
> HugeTLB page pool. This can be problematic for workloads that depend on
> a fixed number of HugeTLB pages.
>
> Currently, soft offline must be disabled to prevent HugeTLB pages from
> being soft offlined.
>
> This patch allows soft offline to be disabled for HugeTLB pages while
> remaining enabled for non-HugeTLB pages.
>
> Commit 56374430c5dfc ("mm/memory-failure: userspace controls
> soft-offlining pages") introduced the following sysctl interface to
> control soft offline:
>
> /proc/sys/vm/enable_soft_offline
>
> The interface does not distinguish between page types:
>
> 0 - Soft offline is disabled
> 1 - Soft offline is enabled
>
> Convert enable_soft_offline to a bitmask and support disabling soft
> offline for HugeTLB pages:
>
> Bits:
>
> 0 - Enable soft offline
> 1 - Disable soft offline for HugeTLB pages
>
> Supported values:
>
> 0 - Soft offline is disabled
> 1 - Soft offline is enabled
> 3 - Soft offline is enabled (disabled for HugeTLB pages)
>
> Existing behavior is preserved.
>
> Update documentation and HugeTLB soft offline selftests.
This is adding a lot of user interfaces to control something you can
disable by config option for an admin only functionality.
I fail to to see any actual justification for all of that. If an admin
can disolve a hugetlb page it has power to allocate a new one as well.
Not to menation that the whole soft offlining is mostly a testing
feature so adding a lot of fine grained configuration space seems
excessive to me.
> Suggested-by: Tony Luck <[email protected]>
> Signed-off-by: Kyle Meyer <[email protected]>
> ---
>
> Tony's patch:
> * https://lore.kernel.org/all/[email protected]
>
> v1:
> * https://lore.kernel.org/all/[email protected]
>
> v1 -> v2:
> * Make the interface extensible, as suggested by David.
> * Preserve existing behavior, as suggested by Jiaqi and David.
> * https://lore.kernel.org/all/[email protected]
>
> v2 -> v3:
> * Minor documentation updates.
> * Use page_folio(page) instead of pfn_folio(pfn) for HugeTLB page check.
>
> ---
> .../ABI/testing/sysfs-memory-page-offline | 3 +++
> Documentation/admin-guide/sysctl/vm.rst | 27 +++++++++++++++----
> mm/memory-failure.c | 17 +++++++++---
> .../selftests/mm/hugetlb-soft-offline.c | 19 ++++++++++---
> 4 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-memory-page-offline
> b/Documentation/ABI/testing/sysfs-memory-page-offline
> index 00f4e35f916f..19aa539fb910 100644
> --- a/Documentation/ABI/testing/sysfs-memory-page-offline
> +++ b/Documentation/ABI/testing/sysfs-memory-page-offline
> @@ -20,6 +20,9 @@ Description:
> number, or a error when the offlining failed. Reading
> the file is not allowed.
>
> + Soft-offline can be controlled via sysctl:
> + Documentation/admin-guide/sysctl/vm.rst
> +
> What: /sys/devices/system/memory/hard_offline_page
> Date: Sep 2009
> KernelVersion: 2.6.33
> diff --git a/Documentation/admin-guide/sysctl/vm.rst
> b/Documentation/admin-guide/sysctl/vm.rst
> index 5b318d17aa4b..ee1941e44643 100644
> --- a/Documentation/admin-guide/sysctl/vm.rst
> +++ b/Documentation/admin-guide/sysctl/vm.rst
> @@ -309,16 +309,33 @@ physical memory) vs performance / capacity implications
> in transparent and
> HugeTLB cases.
>
> For all architectures, enable_soft_offline controls whether to soft offline
> -memory pages. When set to 1, kernel attempts to soft offline the pages
> -whenever it thinks needed. When set to 0, kernel returns EOPNOTSUPP to
> -the request to soft offline the pages. Its default value is 1.
> +memory pages.
>
> -It is worth mentioning that after setting enable_soft_offline to 0, the
> +enable_soft_offline is a bitmask:
> +
> +Bits::
> +
> + 0 - Enable soft offline
> + 1 - Disable soft offline for HugeTLB pages
> +
> +Supported values::
> +
> + 0 - Soft offline is disabled
> + 1 - Soft offline is enabled
> + 3 - Soft offline is enabled (disabled for HugeTLB pages)
> +
> +The default value is 1.
> +
> +If soft offline is disabled for the requested page type, EOPNOTSUPP is
> returned.
> +
> +It is worth mentioning that after disabling soft offline, the
> following requests to soft offline pages will not be performed:
>
> +- Request to soft offline pages from sysfs.
> +
> - Request to soft offline pages from RAS Correctable Errors Collector.
>
> -- On ARM, the request to soft offline pages from GHES driver.
> +- Request to soft offline pages from GHES driver.
>
> - On PARISC, the request to soft offline pages from Page Deallocation Table.
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index a8b03e2920ba..6c4ce39f46df 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -69,11 +69,14 @@
> #include "page_alloc.h"
> #include "internal.h"
>
> +#define SOFT_OFFLINE_ENABLED BIT(0)
> +#define SOFT_OFFLINE_SKIP_HUGETLB BIT(1)
> +
> 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_ENABLED;
>
> static int sysctl_panic_on_unrecoverable_mf __read_mostly;
>
> @@ -157,7 +160,7 @@ static const struct ctl_table memory_failure_table[] = {
> .mode = 0644,
> .proc_handler = proc_dointvec_minmax,
> .extra1 = SYSCTL_ZERO,
> - .extra2 = SYSCTL_ONE,
> + .extra2 = SYSCTL_THREE,
> },
> {
> .procname = "panic_on_unrecoverable_memory_failure",
> @@ -2982,12 +2985,20 @@ int soft_offline_page(unsigned long pfn, int flags)
> return -EIO;
> }
>
> - if (!sysctl_enable_soft_offline) {
> + if (!(sysctl_enable_soft_offline & SOFT_OFFLINE_ENABLED)) {
> pr_info_once("disabled by /proc/sys/vm/enable_soft_offline\n");
> put_ref_page(pfn, flags);
> return -EOPNOTSUPP;
> }
>
> + if (sysctl_enable_soft_offline & SOFT_OFFLINE_SKIP_HUGETLB) {
> + if (folio_test_hugetlb(page_folio(page))) {
> + pr_info_once("disabled for HugeTLB pages by
> /proc/sys/vm/enable_soft_offline\n");
> + put_ref_page(pfn, flags);
> + return -EOPNOTSUPP;
> + }
> + }
> +
> mutex_lock(&mf_mutex);
>
> if (PageHWPoison(page)) {
> diff --git a/tools/testing/selftests/mm/hugetlb-soft-offline.c
> b/tools/testing/selftests/mm/hugetlb-soft-offline.c
> index bc202e4ed2bd..9837c315a81e 100644
> --- a/tools/testing/selftests/mm/hugetlb-soft-offline.c
> +++ b/tools/testing/selftests/mm/hugetlb-soft-offline.c
> @@ -5,6 +5,8 @@
> * offlining failed with EOPNOTSUPP.
> * - if enable_soft_offline = 1, a hugepage should be dissolved and
> * nr_hugepages/free_hugepages should be reduced by 1.
> + * - if enable_soft_offline = 3, HugeTLB pages should stay intact and soft
> + * offlining failed with EOPNOTSUPP.
> *
> * The test allocates 8 default hugepages
> */
> @@ -31,6 +33,9 @@
>
> #define EPREFIX " !!! "
>
> +#define SOFT_OFFLINE_ENABLED (1 << 0)
> +#define SOFT_OFFLINE_SKIP_HUGETLB (1 << 1)
> +
> static int do_soft_offline(int fd, size_t len, int expect_errno)
> {
> char *filemap = NULL;
> @@ -55,6 +60,7 @@ static int do_soft_offline(int fd, size_t len, int
> expect_errno)
> ksft_print_msg("Allocated %#lx bytes of hugetlb pages\n", len);
>
> hwp_addr = filemap + len / 2;
> + errno = 0;
> ret = madvise(hwp_addr, pagesize, MADV_SOFT_OFFLINE);
> ksft_print_msg("MADV_SOFT_OFFLINE %p ret=%d, errno=%d\n",
> hwp_addr, ret, errno);
> @@ -82,7 +88,7 @@ static int set_enable_soft_offline(int value)
> char cmd[256] = {0};
> FILE *cmdfile = NULL;
>
> - if (value != 0 && value != 1)
> + if (value < 0 || value > 3)
> return -EINVAL;
>
> sprintf(cmd, "echo %d > /proc/sys/vm/enable_soft_offline", value);
> @@ -128,13 +134,17 @@ static int create_hugetlbfs_file(struct statfs
> *file_stat)
> static void test_soft_offline_common(int enable_soft_offline)
> {
> int fd;
> - int expect_errno = enable_soft_offline ? 0 : EOPNOTSUPP;
> + int expect_errno = 0;
> struct statfs file_stat;
> unsigned long hugepagesize_kb = 0;
> unsigned long nr_hugepages_before = 0;
> unsigned long nr_hugepages_after = 0;
> int ret;
>
> + if (!(enable_soft_offline & SOFT_OFFLINE_ENABLED) ||
> + (enable_soft_offline & SOFT_OFFLINE_SKIP_HUGETLB))
> + expect_errno = EOPNOTSUPP;
> +
> ksft_print_msg("Test soft-offline when enabled_soft_offline=%d\n",
> enable_soft_offline);
>
> @@ -165,7 +175,7 @@ static void test_soft_offline_common(int
> enable_soft_offline)
> // No need for the hugetlbfs file from now on.
> close(fd);
>
> - if (enable_soft_offline) {
> + if (expect_errno == 0) {
> if (nr_hugepages_before != nr_hugepages_after + 1) {
> ksft_test_result_fail("MADV_SOFT_OFFLINE should reduced
> 1 hugepage\n");
> return;
> @@ -190,8 +200,9 @@ int main(int argc, char **argv)
> if (!hugetlb_setup_default(8))
> ksft_exit_skip("not enough hugetlb pages\n");
>
> - ksft_set_plan(2);
> + ksft_set_plan(3);
>
> + test_soft_offline_common(3);
> test_soft_offline_common(1);
> test_soft_offline_common(0);
>
> --
> 2.51.0
--
Michal Hocko
SUSE Labs
