[PATCH v12 0/7] add hugetlb_optimize_vmemmap sysctl

[PATCH v12 0/7] add hugetlb_optimize_vmemmap sysctl

[Muchun Song]

This series is based on next-20220428.

This series amis to add hugetlb_optimize_vmemmap sysctl to enable or disable
the feature of optimizing vmemmap pages associated with HugeTLB pages.

v12:
  - Add patch 3 and patch 4 to handle the coexistence of hugetlb_free_vmemmap
    and memory_hotplug.memmap_on_memory (David).
  - Remove Reviewed-by from Mike in the last parch since it is changed.

v11:
  - Collect Reviewed-by from Mike.
  - Remove hugetlb_optimize_vmemmap_enabled() check from flush_free_hpage_work().

v10:
  - Collect Reviewed-by from Mike.
  - Remove hugetlb_optimize_vmemmap_enabled() check from
    hugetlb_optimize_vmemmap_pages() (Mike).
  - Add more explanation to Documentation/admin-guide/sysctl/vm.rst.
  - Fix cannot disable the feature via hugetlb_optimize_vmemmap sysctl (Mike).
  - Update patch 2's commit log (Mike).

v9:
  - Go back to v3 since checking the size of struct page at config time is
    very complex.

v8:
  - Fix compilation (scripts/selinux/mdp/mdp.c) error when
    CONFIG_SECURITY_SELINUX is selected.

v7:
  - Fix circular dependency issue reported by kernel test robot.
  - Introduce CONFIG_HUGETLB_PAGE_HAS_OPTIMIZE_VMEMMAP instead of
    STRUCT_PAGE_SIZE_IS_POWER_OF_2.
  - Add more comments into vm.rst to explain hugetlb_optimize_vmemmap (Andrew).
  - Drop the patch "sysctl: allow to set extra1 to SYSCTL_ONE".
  - Add a new patch "use kstrtobool for hugetlb_vmemmap param parsing".
  - Reuse static_key's refcount to count the number of HugeTLB pages with
    vmemmap pages optimized to simplify the lock scheme.

v6:
  - Remove "make syncconfig" from Kbuild.

v5:
  - Fix not working properly if one is workig off of a very clean build
    reported by Luis Chamberlain.
  - Add Suggested-by for Luis Chamberlain.

v4:
  - Introduce STRUCT_PAGE_SIZE_IS_POWER_OF_2 inspired by Luis.

v3:
  - Add pr_warn_once() (Mike).
  - Handle the transition from enabling to disabling (Luis)

v2:
  - Fix compilation when !CONFIG_MHP_MEMMAP_ON_MEMORY reported by kernel
    test robot <lkp@intel.com>.
  - Move sysctl code from kernel/sysctl.c to mm/hugetlb_vmemmap.c.

Muchun Song (7):
  mm: hugetlb_vmemmap: disable hugetlb_optimize_vmemmap when struct page
    crosses page boundaries
  mm: hugetlb_vmemmap: use kstrtobool for hugetlb_vmemmap param parsing
  mm: memory_hotplug: enumerate all supported section flags
  mm: hotplug: introduce SECTION_CANNOT_OPTIMIZE_VMEMMAP
  mm: hugetlb_vmemmap: remove hugetlb_optimize_vmemmap_enabled()
  sysctl: handle table->maxlen properly for proc_dobool
  mm: hugetlb_vmemmap: add hugetlb_optimize_vmemmap sysctl

 Documentation/admin-guide/kernel-parameters.txt | 22 +++----
 Documentation/admin-guide/sysctl/vm.rst         | 38 ++++++++++++
 arch/arm64/mm/flush.c                           | 13 +---
 fs/lockd/svc.c                                  |  2 +-
 include/linux/kconfig.h                         |  1 +
 include/linux/mmzone.h                          | 54 +++++++++++++---
 include/linux/page-flags.h                      | 16 +----
 kernel/sysctl.c                                 | 22 ++++---
 mm/hugetlb_vmemmap.c                            | 82 ++++++++++++++++++-------
 mm/memory_hotplug.c                             |  7 ++-
 mm/sparse.c                                     |  7 +++
 11 files changed, 186 insertions(+), 78 deletions(-)

-- 
2.11.0

[PATCH v12 1/7] mm: hugetlb_vmemmap: disable hugetlb_optimize_vmemmap when struct page crosses page boundaries

[Muchun Song]

If the size of "struct page" is not the power of two but with the feature
of minimizing overhead of struct page associated with each HugeTLB is
enabled, then the vmemmap pages of HugeTLB will be corrupted after
remapping (panic is about to happen in theory).  But this only exists when
!CONFIG_MEMCG && !CONFIG_SLUB on x86_64.  However, it is not a conventional
configuration nowadays.  So it is not a real word issue, just the result
of a code review.

But we cannot prevent anyone from configuring that combined configure.
This hugetlb_optimize_vmemmap should be disable in this case to fix this
issue.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Acked-by: David Hildenbrand <david@redhat.com>
---
 mm/hugetlb_vmemmap.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
index 29554c6ef2ae..6254bb2d4ae5 100644
--- a/mm/hugetlb_vmemmap.c
+++ b/mm/hugetlb_vmemmap.c
@@ -28,12 +28,6 @@ EXPORT_SYMBOL(hugetlb_optimize_vmemmap_key);
 
 static int __init hugetlb_vmemmap_early_param(char *buf)
 {
-	/* We cannot optimize if a "struct page" crosses page boundaries. */
-	if (!is_power_of_2(sizeof(struct page))) {
-		pr_warn("cannot free vmemmap pages because \"struct page\" crosses page boundaries\n");
-		return 0;
-	}
-
 	if (!buf)
 		return -EINVAL;
 
@@ -119,6 +113,12 @@ void __init hugetlb_vmemmap_init(struct hstate *h)
 	if (!hugetlb_optimize_vmemmap_enabled())
 		return;
 
+	if (!is_power_of_2(sizeof(struct page))) {
+		pr_warn_once("cannot optimize vmemmap pages because \"struct page\" crosses page boundaries\n");
+		static_branch_disable(&hugetlb_optimize_vmemmap_key);
+		return;
+	}
+
 	vmemmap_pages = (nr_pages * sizeof(struct page)) >> PAGE_SHIFT;
 	/*
 	 * The head page is not to be freed to buddy allocator, the other tail
-- 
2.11.0

[PATCH v12 2/7] mm: hugetlb_vmemmap: use kstrtobool for hugetlb_vmemmap param parsing

[Muchun Song]

Use kstrtobool rather than open coding "on" and "off" parsing in
mm/hugetlb_vmemmap.c,  which is more powerful to handle all kinds
of parameters like 'Yy1Nn0' or [oO][NnFf] for "on" and "off".

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Acked-by: David Hildenbrand <david@redhat.com>
---
 mm/hugetlb_vmemmap.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
index 6254bb2d4ae5..cc4ec752ec16 100644
--- a/mm/hugetlb_vmemmap.c
+++ b/mm/hugetlb_vmemmap.c
@@ -28,15 +28,15 @@ EXPORT_SYMBOL(hugetlb_optimize_vmemmap_key);
 
 static int __init hugetlb_vmemmap_early_param(char *buf)
 {
-	if (!buf)
+	bool enable;
+
+	if (kstrtobool(buf, &enable))
 		return -EINVAL;
 
-	if (!strcmp(buf, "on"))
+	if (enable)
 		static_branch_enable(&hugetlb_optimize_vmemmap_key);
-	else if (!strcmp(buf, "off"))
-		static_branch_disable(&hugetlb_optimize_vmemmap_key);
 	else
-		return -EINVAL;
+		static_branch_disable(&hugetlb_optimize_vmemmap_key);
 
 	return 0;
 }
-- 
2.11.0

[PATCH v12 3/7] mm: memory_hotplug: enumerate all supported section flags

[Muchun Song]

We are almost running out of free slots, only one bit is available in the
worst case (powerpc with 256k pages).  However, there are still some free
slots on other architectures (e.g. x86_64 has 10 bits available, arm64
has 8 bits available with worst case of 64K pages).  We have hard coded
those numbers in code, it is inconvenient to use those bits on other
architectures except powerpc.  So transfer those section flags to
enumeration to make it easy to add new section flags in the future. Also,
move SECTION_TAINT_ZONE_DEVICE into the scope of CONFIG_ZONE_DEVICE
to save a bit on non-zone-device case.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 include/linux/kconfig.h |  1 +
 include/linux/mmzone.h  | 37 +++++++++++++++++++++++++++++--------
 mm/memory_hotplug.c     |  6 ++++++
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 20d1079e92b4..7044032b9f42 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -10,6 +10,7 @@
 #define __LITTLE_ENDIAN 1234
 #endif
 
+#define __ARG_PLACEHOLDER_ 0,
 #define __ARG_PLACEHOLDER_1 0,
 #define __take_second_arg(__ignored, val, ...) val
 
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index aab70355d64f..af057e20b9d7 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1418,16 +1418,37 @@ extern size_t mem_section_usage_size(void);
  *      (equal SECTION_SIZE_BITS - PAGE_SHIFT), and the
  *      worst combination is powerpc with 256k pages,
  *      which results in PFN_SECTION_SHIFT equal 6.
- * To sum it up, at least 6 bits are available.
+ * To sum it up, at least 6 bits are available on all architectures.
+ * However, we can exceed 6 bits on some other architectures except
+ * powerpc (e.g. 15 bits are available on x86_64, 13 bits are available
+ * with the worst case of 64K pages on arm64) if we make sure the
+ * exceeded bit is not applicable to powerpc.
  */
-#define SECTION_MARKED_PRESENT		(1UL<<0)
-#define SECTION_HAS_MEM_MAP		(1UL<<1)
-#define SECTION_IS_ONLINE		(1UL<<2)
-#define SECTION_IS_EARLY		(1UL<<3)
-#define SECTION_TAINT_ZONE_DEVICE	(1UL<<4)
-#define SECTION_MAP_LAST_BIT		(1UL<<5)
+#define ENUM_SECTION_FLAG(MAPPER)						\
+	MAPPER(MARKED_PRESENT)							\
+	MAPPER(HAS_MEM_MAP)							\
+	MAPPER(IS_ONLINE)							\
+	MAPPER(IS_EARLY)							\
+	MAPPER(TAINT_ZONE_DEVICE, CONFIG_ZONE_DEVICE)				\
+	MAPPER(MAP_LAST_BIT)
+
+#define __SECTION_SHIFT_FLAG_MAPPER_0(x)
+#define __SECTION_SHIFT_FLAG_MAPPER_1(x)	SECTION_##x##_SHIFT,
+#define __SECTION_SHIFT_FLAG_MAPPER(x, ...)	\
+	__PASTE(__SECTION_SHIFT_FLAG_MAPPER_, IS_ENABLED(__VA_ARGS__))(x)
+
+#define __SECTION_FLAG_MAPPER_0(x)
+#define __SECTION_FLAG_MAPPER_1(x)		SECTION_##x = BIT(SECTION_##x##_SHIFT),
+#define __SECTION_FLAG_MAPPER(x, ...)		\
+	__PASTE(__SECTION_FLAG_MAPPER_, IS_ENABLED(__VA_ARGS__))(x)
+
+enum {
+	ENUM_SECTION_FLAG(__SECTION_SHIFT_FLAG_MAPPER)
+	ENUM_SECTION_FLAG(__SECTION_FLAG_MAPPER)
+};
+
 #define SECTION_MAP_MASK		(~(SECTION_MAP_LAST_BIT-1))
-#define SECTION_NID_SHIFT		6
+#define SECTION_NID_SHIFT		SECTION_MAP_LAST_BIT_SHIFT
 
 static inline struct page *__section_mem_map_addr(struct mem_section *section)
 {
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 111684878fd9..aef3f041dec7 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -655,12 +655,18 @@ static void __meminit resize_pgdat_range(struct pglist_data *pgdat, unsigned lon
 
 }
 
+#ifdef CONFIG_ZONE_DEVICE
 static void section_taint_zone_device(unsigned long pfn)
 {
 	struct mem_section *ms = __pfn_to_section(pfn);
 
 	ms->section_mem_map |= SECTION_TAINT_ZONE_DEVICE;
 }
+#else
+static inline void section_taint_zone_device(unsigned long pfn)
+{
+}
+#endif
 
 /*
  * Associate the pfn range with the given zone, initializing the memmaps
-- 
2.11.0

[PATCH v12 4/7] mm: hotplug: introduce SECTION_CANNOT_OPTIMIZE_VMEMMAP

[Muchun Song]

For now, the feature of hugetlb_free_vmemmap is not compatible with the
feature of memory_hotplug.memmap_on_memory, and hugetlb_free_vmemmap
takes precedence over memory_hotplug.memmap_on_memory. However, someone
wants to make memory_hotplug.memmap_on_memory takes precedence over
hugetlb_free_vmemmap since memmap_on_memory makes it more likely to
succeed memory hotplug in close-to-OOM situations.  So the decision
of making hugetlb_free_vmemmap take precedence is not wise and elegant.
The proper approach is to have hugetlb_vmemmap.c do the check whether
the section which the HugeTLB pages belong to can be optimized.  If
the section's vmemmap pages are allocated from the added memory block
itself, hugetlb_free_vmemmap should refuse to optimize the vmemmap,
otherwise, do the optimization.  Then both kernel parameters are
compatible.  So this patch introduces SECTION_CANNOT_OPTIMIZE_VMEMMAP
to indicate whether the section could be optimized.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 22 +++++++++++-----------
 include/linux/mmzone.h                          | 17 +++++++++++++++++
 mm/hugetlb_vmemmap.c                            | 16 +++++++++++++++-
 mm/memory_hotplug.c                             |  1 -
 mm/sparse.c                                     |  7 +++++++
 5 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 308da668bbb1..a0a014f2104c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1711,9 +1711,11 @@
 			Built with CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON=y,
 			the default is on.
 
-			This is not compatible with memory_hotplug.memmap_on_memory.
-			If both parameters are enabled, hugetlb_free_vmemmap takes
-			precedence over memory_hotplug.memmap_on_memory.
+			Note that the vmemmap pages may be allocated from the added
+			memory block itself when memory_hotplug.memmap_on_memory is
+			enabled, those vmemmap pages cannot be optimized even if this
+			feature is enabled.  Other vmemmap pages not allocated from
+			the added memory block itself do not be affected.
 
 	hung_task_panic=
 			[KNL] Should the hung task detector generate panics.
@@ -3038,10 +3040,12 @@
 			[KNL,X86,ARM] Boolean flag to enable this feature.
 			Format: {on | off (default)}
 			When enabled, runtime hotplugged memory will
-			allocate its internal metadata (struct pages)
-			from the hotadded memory which will allow to
-			hotadd a lot of memory without requiring
-			additional memory to do so.
+			allocate its internal metadata (struct pages,
+			those vmemmap pages cannot be optimized even
+			if hugetlb_free_vmemmap is enabled) from the
+			hotadded memory which will allow to hotadd a
+			lot of memory without requiring additional
+			memory to do so.
 			This feature is disabled by default because it
 			has some implication on large (e.g. GB)
 			allocations in some configurations (e.g. small
@@ -3051,10 +3055,6 @@
 			Note that even when enabled, there are a few cases where
 			the feature is not effective.
 
-			This is not compatible with hugetlb_free_vmemmap. If
-			both parameters are enabled, hugetlb_free_vmemmap takes
-			precedence over memory_hotplug.memmap_on_memory.
-
 	memtest=	[KNL,X86,ARM,M68K,PPC,RISCV] Enable memtest
 			Format: <integer>
 			default : 0 <disable>
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index af057e20b9d7..7b69acc5c2a9 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1430,6 +1430,7 @@ extern size_t mem_section_usage_size(void);
 	MAPPER(IS_ONLINE)							\
 	MAPPER(IS_EARLY)							\
 	MAPPER(TAINT_ZONE_DEVICE, CONFIG_ZONE_DEVICE)				\
+	MAPPER(CANNOT_OPTIMIZE_VMEMMAP, CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP)	\
 	MAPPER(MAP_LAST_BIT)
 
 #define __SECTION_SHIFT_FLAG_MAPPER_0(x)
@@ -1457,6 +1458,22 @@ static inline struct page *__section_mem_map_addr(struct mem_section *section)
 	return (struct page *)map;
 }
 
+#ifdef CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP
+static inline void section_mark_cannot_optimize_vmemmap(struct mem_section *ms)
+{
+	ms->section_mem_map |= SECTION_CANNOT_OPTIMIZE_VMEMMAP;
+}
+
+static inline int section_cannot_optimize_vmemmap(struct mem_section *ms)
+{
+	return (ms && (ms->section_mem_map & SECTION_CANNOT_OPTIMIZE_VMEMMAP));
+}
+#else
+static inline void section_mark_cannot_optimize_vmemmap(struct mem_section *ms)
+{
+}
+#endif
+
 static inline int present_section(struct mem_section *section)
 {
 	return (section && (section->section_mem_map & SECTION_MARKED_PRESENT));
diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
index cc4ec752ec16..970c36b8935f 100644
--- a/mm/hugetlb_vmemmap.c
+++ b/mm/hugetlb_vmemmap.c
@@ -75,12 +75,26 @@ int hugetlb_vmemmap_alloc(struct hstate *h, struct page *head)
 	return ret;
 }
 
+static unsigned int optimizable_vmemmap_pages(struct hstate *h,
+					      struct page *head)
+{
+	unsigned long pfn = page_to_pfn(head);
+	unsigned long end = pfn + pages_per_huge_page(h);
+
+	for (; pfn < end; pfn += PAGES_PER_SECTION) {
+		if (section_cannot_optimize_vmemmap(__pfn_to_section(pfn)))
+			return 0;
+	}
+
+	return hugetlb_optimize_vmemmap_pages(h);
+}
+
 void hugetlb_vmemmap_free(struct hstate *h, struct page *head)
 {
 	unsigned long vmemmap_addr = (unsigned long)head;
 	unsigned long vmemmap_end, vmemmap_reuse, vmemmap_pages;
 
-	vmemmap_pages = hugetlb_optimize_vmemmap_pages(h);
+	vmemmap_pages = optimizable_vmemmap_pages(h, head);
 	if (!vmemmap_pages)
 		return;
 
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index aef3f041dec7..1d0225d57166 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1270,7 +1270,6 @@ bool mhp_supports_memmap_on_memory(unsigned long size)
 	 *       populate a single PMD.
 	 */
 	return memmap_on_memory &&
-	       !hugetlb_optimize_vmemmap_enabled() &&
 	       IS_ENABLED(CONFIG_MHP_MEMMAP_ON_MEMORY) &&
 	       size == memory_block_size_bytes() &&
 	       IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
diff --git a/mm/sparse.c b/mm/sparse.c
index d2d76d158b39..8197ef9b7c4c 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -913,6 +913,13 @@ int __meminit sparse_add_section(int nid, unsigned long start_pfn,
 	ms = __nr_to_section(section_nr);
 	set_section_nid(section_nr, nid);
 	__section_mark_present(ms, section_nr);
+	/*
+	 * Mark whole section as non-optimizable once there is a subsection
+	 * whose vmemmap pages are allocated from alternative allocator. The
+	 * early section is always optimizable.
+	 */
+	if (!early_section(ms) && altmap)
+		section_mark_cannot_optimize_vmemmap(ms);
 
 	/* Align memmap to section boundary in the subsection case */
 	if (section_nr_to_pfn(section_nr) != start_pfn)
-- 
2.11.0

[PATCH v12 5/7] mm: hugetlb_vmemmap: remove hugetlb_optimize_vmemmap_enabled()

[Muchun Song]

There is only one user of hugetlb_optimize_vmemmap_enabled() outside of
hugetlb_vmemmap, that is flush_dcache_page() in arch/arm64/mm/flush.c.
However, it does not need to call hugetlb_optimize_vmemmap_enabled()
in flush_dcache_page() since HugeTLB pages are always fully mapped
and only head page will be set PG_dcache_clean meaning only head page
's flag may need to be cleared (see commit cf5a501d985b).  After this
change, there is no users of hugetlb_optimize_vmemmap_enabled() outside
of hugetlb_vmemmap. So remove hugetlb_optimize_vmemmap_enabled() to
simplify the code.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/mm/flush.c      | 13 +++----------
 include/linux/page-flags.h | 14 ++------------
 mm/hugetlb_vmemmap.c       |  3 ++-
 3 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/mm/flush.c b/arch/arm64/mm/flush.c
index fc4f710e9820..5f9379b3c8c8 100644
--- a/arch/arm64/mm/flush.c
+++ b/arch/arm64/mm/flush.c
@@ -76,17 +76,10 @@ EXPORT_SYMBOL_GPL(__sync_icache_dcache);
 void flush_dcache_page(struct page *page)
 {
 	/*
-	 * Only the head page's flags of HugeTLB can be cleared since the tail
-	 * vmemmap pages associated with each HugeTLB page are mapped with
-	 * read-only when CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP is enabled (more
-	 * details can refer to vmemmap_remap_pte()).  Although
-	 * __sync_icache_dcache() only set PG_dcache_clean flag on the head
-	 * page struct, there is more than one page struct with PG_dcache_clean
-	 * associated with the HugeTLB page since the head vmemmap page frame
-	 * is reused (more details can refer to the comments above
-	 * page_fixed_fake_head()).
+	 * HugeTLB pages are always fully mapped and only head page will be
+	 * set PG_dcache_clean (see comments in __sync_icache_dcache()).
 	 */
-	if (hugetlb_optimize_vmemmap_enabled() && PageHuge(page))
+	if (PageHuge(page))
 		page = compound_head(page);
 
 	if (test_bit(PG_dcache_clean, &page->flags))
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index b70124b9c7c1..404f4ede17f5 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -203,12 +203,6 @@ enum pageflags {
 DECLARE_STATIC_KEY_MAYBE(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
 			 hugetlb_optimize_vmemmap_key);
 
-static __always_inline bool hugetlb_optimize_vmemmap_enabled(void)
-{
-	return static_branch_maybe(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
-				   &hugetlb_optimize_vmemmap_key);
-}
-
 /*
  * If the feature of optimizing vmemmap pages associated with each HugeTLB
  * page is enabled, the head vmemmap page frame is reused and all of the tail
@@ -227,7 +221,8 @@ static __always_inline bool hugetlb_optimize_vmemmap_enabled(void)
  */
 static __always_inline const struct page *page_fixed_fake_head(const struct page *page)
 {
-	if (!hugetlb_optimize_vmemmap_enabled())
+	if (!static_branch_maybe(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
+				 &hugetlb_optimize_vmemmap_key))
 		return page;
 
 	/*
@@ -255,11 +250,6 @@ static inline const struct page *page_fixed_fake_head(const struct page *page)
 {
 	return page;
 }
-
-static inline bool hugetlb_optimize_vmemmap_enabled(void)
-{
-	return false;
-}
 #endif
 
 static __always_inline int page_is_fake_head(struct page *page)
diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
index 970c36b8935f..d1fea65fec98 100644
--- a/mm/hugetlb_vmemmap.c
+++ b/mm/hugetlb_vmemmap.c
@@ -124,7 +124,8 @@ void __init hugetlb_vmemmap_init(struct hstate *h)
 	BUILD_BUG_ON(__NR_USED_SUBPAGE >=
 		     RESERVE_VMEMMAP_SIZE / sizeof(struct page));
 
-	if (!hugetlb_optimize_vmemmap_enabled())
+	if (!static_branch_maybe(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
+				 &hugetlb_optimize_vmemmap_key))
 		return;
 
 	if (!is_power_of_2(sizeof(struct page))) {
-- 
2.11.0

[PATCH v12 6/7] sysctl: handle table->maxlen properly for proc_dobool

[Muchun Song]

Setting ->proc_handler to proc_dobool at the same time setting ->maxlen
to sizeof(int) is counter-intuitive, it is easy to make mistakes.  For
robustness, fix it by handling able->maxlen properly for proc_dobool in
__do_proc_dointvec().  In the next patch, we will use proc_dobool which
depends on this change.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Iurii Zaikin <yzaikin@google.com>
---
 fs/lockd/svc.c  |  2 +-
 kernel/sysctl.c | 22 ++++++++++++----------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index 59ef8a1f843f..6e48ee787f49 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -496,7 +496,7 @@ static struct ctl_table nlm_sysctls[] = {
 	{
 		.procname	= "nsm_use_hostnames",
 		.data		= &nsm_use_hostnames,
-		.maxlen		= sizeof(int),
+		.maxlen		= sizeof(nsm_use_hostnames),
 		.mode		= 0644,
 		.proc_handler	= proc_dobool,
 	},
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index e52b6e372c60..353fb9093012 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -428,6 +428,8 @@ static int do_proc_dobool_conv(bool *negp, unsigned long *lvalp,
 				int write, void *data)
 {
 	if (write) {
+		if (*negp || (*lvalp != 0 && *lvalp != 1))
+			return -EINVAL;
 		*(bool *)valp = *lvalp;
 	} else {
 		int val = *(bool *)valp;
@@ -489,17 +491,17 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
 			      int write, void *data),
 		  void *data)
 {
-	int *i, vleft, first = 1, err = 0;
+	int vleft, first = 1, err = 0, size;
 	size_t left;
 	char *p;
-	
+
 	if (!tbl_data || !table->maxlen || !*lenp || (*ppos && !write)) {
 		*lenp = 0;
 		return 0;
 	}
-	
-	i = (int *) tbl_data;
-	vleft = table->maxlen / sizeof(*i);
+
+	size = conv == do_proc_dobool_conv ? sizeof(bool) : sizeof(int);
+	vleft = table->maxlen / size;
 	left = *lenp;
 
 	if (!conv)
@@ -514,7 +516,7 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
 		p = buffer;
 	}
 
-	for (; left && vleft--; i++, first=0) {
+	for (; left && vleft--; tbl_data = (char *)tbl_data + size, first=0) {
 		unsigned long lval;
 		bool neg;
 
@@ -528,12 +530,12 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
 					     sizeof(proc_wspace_sep), NULL);
 			if (err)
 				break;
-			if (conv(&neg, &lval, i, 1, data)) {
+			if (conv(&neg, &lval, tbl_data, 1, data)) {
 				err = -EINVAL;
 				break;
 			}
 		} else {
-			if (conv(&neg, &lval, i, 0, data)) {
+			if (conv(&neg, &lval, tbl_data, 0, data)) {
 				err = -EINVAL;
 				break;
 			}
@@ -708,8 +710,8 @@ int do_proc_douintvec(struct ctl_table *table, int write,
  * @lenp: the size of the user buffer
  * @ppos: file position
  *
- * Reads/writes up to table->maxlen/sizeof(unsigned int) integer
- * values from/to the user buffer, treated as an ASCII string.
+ * Reads/writes up to table->maxlen/sizeof(bool) bool values from/to
+ * the user buffer, treated as an ASCII string.
  *
  * Returns 0 on success.
  */
-- 
2.11.0

[PATCH v12 7/7] mm: hugetlb_vmemmap: add hugetlb_optimize_vmemmap sysctl

[Muchun Song]

We must add hugetlb_free_vmemmap=on (or "off") to the boot cmdline and
reboot the server to enable or disable the feature of optimizing vmemmap
pages associated with HugeTLB pages.  However, rebooting usually takes a
long time.  So add a sysctl to enable or disable the feature at runtime
without rebooting.  Why we need this?  There are 3 use cases.

1) The feature of minimizing overhead of struct page associated with each
HugeTLB is disabled by default without passing "hugetlb_free_vmemmap=on"
to the boot cmdline. When we (ByteDance) deliver the servers to the
users who want to enable this feature, they have to configure the grub
(change boot cmdline) and reboot the servers, whereas rebooting usually
takes a long time (we have thousands of servers).  It's a very bad
experience for the users.  So we need a approach to enable this feature
after rebooting. This is a use case in our practical environment.

2) Some use cases are that HugeTLB pages are allocated 'on the fly'
instead of being pulled from the HugeTLB pool, those workloads would be
affected with this feature enabled.  Those workloads could be identified
by the characteristics of they never explicitly allocating huge pages
with 'nr_hugepages' but only set 'nr_overcommit_hugepages' and then let
the pages be allocated from the buddy allocator at fault time.  We can
confirm it is a real use case from the commit 099730d67417.  For those
workloads, the page fault time could be ~2x slower than before. We
suspect those users want to disable this feature if the system has enabled
this before and they don't think the memory savings benefit is enough to
make up for the performance drop.

3) If the workload which wants vmemmap pages to be optimized and the
workload which wants to set 'nr_overcommit_hugepages' and does not want
the extera overhead at fault time when the overcommitted pages be
allocated from the buddy allocator are deployed in the same server.
The user could enable this feature and set 'nr_hugepages' and
'nr_overcommit_hugepages', then disable the feature.  In this case,
the overcommited HugeTLB pages will not encounter the extra overhead
at fault time.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 Documentation/admin-guide/sysctl/vm.rst | 38 ++++++++++++++++++++
 include/linux/page-flags.h              |  6 ++--
 mm/hugetlb_vmemmap.c                    | 61 ++++++++++++++++++++++-----------
 3 files changed, 81 insertions(+), 24 deletions(-)

diff --git a/Documentation/admin-guide/sysctl/vm.rst b/Documentation/admin-guide/sysctl/vm.rst
index 747e325ebcd0..d7374a1e8ac9 100644
--- a/Documentation/admin-guide/sysctl/vm.rst
+++ b/Documentation/admin-guide/sysctl/vm.rst
@@ -562,6 +562,44 @@ Change the minimum size of the hugepage pool.
 See Documentation/admin-guide/mm/hugetlbpage.rst
 
 
+hugetlb_optimize_vmemmap
+========================
+
+This knob is not available when the size of 'struct page' (a structure defined
+in include/linux/mm_types.h) is not power of two (an unusual system config could
+result in this).
+
+Enable (set to 1) or disable (set to 0) the feature of optimizing vmemmap pages
+associated with each HugeTLB page.
+
+Once enabled, the vmemmap pages of subsequent allocation of HugeTLB pages from
+buddy allocator will be optimized (7 pages per 2MB HugeTLB page and 4095 pages
+per 1GB HugeTLB page), whereas already allocated HugeTLB pages will not be
+optimized.  When those optimized HugeTLB pages are freed from the HugeTLB pool
+to the buddy allocator, the vmemmap pages representing that range needs to be
+remapped again and the vmemmap pages discarded earlier need to be rellocated
+again.  If your use case is that HugeTLB pages are allocated 'on the fly' (e.g.
+never explicitly allocating HugeTLB pages with 'nr_hugepages' but only set
+'nr_overcommit_hugepages', those overcommitted HugeTLB pages are allocated 'on
+the fly') instead of being pulled from the HugeTLB pool, you should weigh the
+benefits of memory savings against the more overhead (~2x slower than before)
+of allocation or freeing HugeTLB pages between the HugeTLB pool and the buddy
+allocator.  Another behavior to note is that if the system is under heavy memory
+pressure, it could prevent the user from freeing HugeTLB pages from the HugeTLB
+pool to the buddy allocator since the allocation of vmemmap pages could be
+failed, you have to retry later if your system encounter this situation.
+
+Once disabled, the vmemmap pages of subsequent allocation of HugeTLB pages from
+buddy allocator will not be optimized meaning the extra overhead at allocation
+time from buddy allocator disappears, whereas already optimized HugeTLB pages
+will not be affected.  If you want to make sure there are no optimized HugeTLB
+pages, you can set "nr_hugepages" to 0 first and then disable this.  Note that
+writing 0 to nr_hugepages will make any "in use" HugeTLB pages become surplus
+pages.  So, those surplus pages are still optimized until they are no longer
+in use.  You would need to wait for those surplus pages to be released before
+there are no optimized pages in the system.
+
+
 nr_hugepages_mempolicy
 ======================
 
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 404f4ede17f5..07d8d444d9f1 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -200,8 +200,7 @@ enum pageflags {
 #ifndef __GENERATING_BOUNDS_H
 
 #ifdef CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP
-DECLARE_STATIC_KEY_MAYBE(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
-			 hugetlb_optimize_vmemmap_key);
+DECLARE_STATIC_KEY_FALSE(hugetlb_optimize_vmemmap_key);
 
 /*
  * If the feature of optimizing vmemmap pages associated with each HugeTLB
@@ -221,8 +220,7 @@ DECLARE_STATIC_KEY_MAYBE(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
  */
 static __always_inline const struct page *page_fixed_fake_head(const struct page *page)
 {
-	if (!static_branch_maybe(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
-				 &hugetlb_optimize_vmemmap_key))
+	if (!static_branch_unlikely(&hugetlb_optimize_vmemmap_key))
 		return page;
 
 	/*
diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
index d1fea65fec98..02862f117c2b 100644
--- a/mm/hugetlb_vmemmap.c
+++ b/mm/hugetlb_vmemmap.c
@@ -22,23 +22,15 @@
 #define RESERVE_VMEMMAP_NR		1U
 #define RESERVE_VMEMMAP_SIZE		(RESERVE_VMEMMAP_NR << PAGE_SHIFT)
 
-DEFINE_STATIC_KEY_MAYBE(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
-			hugetlb_optimize_vmemmap_key);
+DEFINE_STATIC_KEY_FALSE(hugetlb_optimize_vmemmap_key);
 EXPORT_SYMBOL(hugetlb_optimize_vmemmap_key);
 
+static bool optimize_vmemmap_enabled =
+	IS_ENABLED(CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON);
+
 static int __init hugetlb_vmemmap_early_param(char *buf)
 {
-	bool enable;
-
-	if (kstrtobool(buf, &enable))
-		return -EINVAL;
-
-	if (enable)
-		static_branch_enable(&hugetlb_optimize_vmemmap_key);
-	else
-		static_branch_disable(&hugetlb_optimize_vmemmap_key);
-
-	return 0;
+	return kstrtobool(buf, &optimize_vmemmap_enabled);
 }
 early_param("hugetlb_free_vmemmap", hugetlb_vmemmap_early_param);
 
@@ -69,8 +61,10 @@ int hugetlb_vmemmap_alloc(struct hstate *h, struct page *head)
 	 */
 	ret = vmemmap_remap_alloc(vmemmap_addr, vmemmap_end, vmemmap_reuse,
 				  GFP_KERNEL | __GFP_NORETRY | __GFP_THISNODE);
-	if (!ret)
+	if (!ret) {
 		ClearHPageVmemmapOptimized(head);
+		static_branch_dec(&hugetlb_optimize_vmemmap_key);
+	}
 
 	return ret;
 }
@@ -81,6 +75,9 @@ static unsigned int optimizable_vmemmap_pages(struct hstate *h,
 	unsigned long pfn = page_to_pfn(head);
 	unsigned long end = pfn + pages_per_huge_page(h);
 
+	if (!READ_ONCE(optimize_vmemmap_enabled))
+		return 0;
+
 	for (; pfn < end; pfn += PAGES_PER_SECTION) {
 		if (section_cannot_optimize_vmemmap(__pfn_to_section(pfn)))
 			return 0;
@@ -98,6 +95,8 @@ void hugetlb_vmemmap_free(struct hstate *h, struct page *head)
 	if (!vmemmap_pages)
 		return;
 
+	static_branch_inc(&hugetlb_optimize_vmemmap_key);
+
 	vmemmap_addr	+= RESERVE_VMEMMAP_SIZE;
 	vmemmap_end	= vmemmap_addr + (vmemmap_pages << PAGE_SHIFT);
 	vmemmap_reuse	= vmemmap_addr - PAGE_SIZE;
@@ -107,7 +106,9 @@ void hugetlb_vmemmap_free(struct hstate *h, struct page *head)
 	 * to the page which @vmemmap_reuse is mapped to, then free the pages
 	 * which the range [@vmemmap_addr, @vmemmap_end] is mapped to.
 	 */
-	if (!vmemmap_remap_free(vmemmap_addr, vmemmap_end, vmemmap_reuse))
+	if (vmemmap_remap_free(vmemmap_addr, vmemmap_end, vmemmap_reuse))
+		static_branch_dec(&hugetlb_optimize_vmemmap_key);
+	else
 		SetHPageVmemmapOptimized(head);
 }
 
@@ -124,13 +125,8 @@ void __init hugetlb_vmemmap_init(struct hstate *h)
 	BUILD_BUG_ON(__NR_USED_SUBPAGE >=
 		     RESERVE_VMEMMAP_SIZE / sizeof(struct page));
 
-	if (!static_branch_maybe(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
-				 &hugetlb_optimize_vmemmap_key))
-		return;
-
 	if (!is_power_of_2(sizeof(struct page))) {
 		pr_warn_once("cannot optimize vmemmap pages because \"struct page\" crosses page boundaries\n");
-		static_branch_disable(&hugetlb_optimize_vmemmap_key);
 		return;
 	}
 
@@ -149,3 +145,28 @@ void __init hugetlb_vmemmap_init(struct hstate *h)
 	pr_info("can optimize %d vmemmap pages for %s\n",
 		h->optimize_vmemmap_pages, h->name);
 }
+
+#ifdef CONFIG_PROC_SYSCTL
+static struct ctl_table hugetlb_vmemmap_sysctls[] = {
+	{
+		.procname	= "hugetlb_optimize_vmemmap",
+		.data		= &optimize_vmemmap_enabled,
+		.maxlen		= sizeof(optimize_vmemmap_enabled),
+		.mode		= 0644,
+		.proc_handler	= proc_dobool,
+	},
+	{ }
+};
+
+static int __init hugetlb_vmemmap_sysctls_init(void)
+{
+	/*
+	 * If "struct page" crosses page boundaries, the vmemmap pages cannot
+	 * be optimized.
+	 */
+	if (is_power_of_2(sizeof(struct page)))
+		register_sysctl_init("vm", hugetlb_vmemmap_sysctls);
+	return 0;
+}
+late_initcall(hugetlb_vmemmap_sysctls_init);
+#endif /* CONFIG_PROC_SYSCTL */
-- 
2.11.0

Re: [PATCH v12 4/7] mm: hotplug: introduce SECTION_CANNOT_OPTIMIZE_VMEMMAP

[Oscar Salvador]

On Mon, May 16, 2022 at 06:22:08PM +0800, Muchun Song wrote:
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -913,6 +913,13 @@ int __meminit sparse_add_section(int nid, unsigned long start_pfn,
>  	ms = __nr_to_section(section_nr);
>  	set_section_nid(section_nr, nid);
>  	__section_mark_present(ms, section_nr);
> +	/*
> +	 * Mark whole section as non-optimizable once there is a subsection
> +	 * whose vmemmap pages are allocated from alternative allocator. The
> +	 * early section is always optimizable.
> +	 */
> +	if (!early_section(ms) && altmap)
> +		section_mark_cannot_optimize_vmemmap(ms);

Because no one expects those sections to be removed?
IIRC, early_section + altmap only happened in case of sub-section pmem
scenario? I guess my question is: can we really have early_sections coming
from alternative allocator?

I think this should be spelled out more.


-- 
Oscar Salvador
SUSE Labs

Re: [PATCH v12 4/7] mm: hotplug: introduce SECTION_CANNOT_OPTIMIZE_VMEMMAP

[Muchun Song]

On Mon, May 16, 2022 at 12:38:46PM +0200, Oscar Salvador wrote:
> On Mon, May 16, 2022 at 06:22:08PM +0800, Muchun Song wrote:
> > --- a/mm/sparse.c
> > +++ b/mm/sparse.c
> > @@ -913,6 +913,13 @@ int __meminit sparse_add_section(int nid, unsigned long start_pfn,
> >  	ms = __nr_to_section(section_nr);
> >  	set_section_nid(section_nr, nid);
> >  	__section_mark_present(ms, section_nr);
> > +	/*
> > +	 * Mark whole section as non-optimizable once there is a subsection
> > +	 * whose vmemmap pages are allocated from alternative allocator. The
> > +	 * early section is always optimizable.
> > +	 */
> > +	if (!early_section(ms) && altmap)
> > +		section_mark_cannot_optimize_vmemmap(ms);
> 
> Because no one expects those sections to be removed?
> IIRC, early_section + altmap only happened in case of sub-section pmem
> scenario?

Right. The commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")
has more information.

> I guess my question is: can we really have early_sections coming
> from alternative allocator?
>

We can't. The early section does not consider partially being
populated currently.

> I think this should be spelled out more.

I think you mean add some comments here to describe the case
of early_section + altmap, right?

Thanks.

Re: [PATCH v12 0/7] add hugetlb_optimize_vmemmap sysctl

[Andrew Morton]

On Mon, 16 May 2022 18:22:04 +0800 Muchun Song <songmuchun@bytedance.com> wrote:

> This series amis to add hugetlb_optimize_vmemmap sysctl to enable or disable
> the feature of optimizing vmemmap pages associated with HugeTLB pages.
> 
> v12:
>   - Add patch 3 and patch 4 to handle the coexistence of hugetlb_free_vmemmap
>     and memory_hotplug.memmap_on_memory (David).
>   - Remove Reviewed-by from Mike in the last parch since it is changed.

v11 is in mm-stable now, so please send patches against that.  AFAICT
that will be quite straightforward.

Re: [PATCH v12 0/7] add hugetlb_optimize_vmemmap sysctl

[Muchun Song]

On Mon, May 16, 2022 at 11:46:20AM -0700, Andrew Morton wrote:
> On Mon, 16 May 2022 18:22:04 +0800 Muchun Song <songmuchun@bytedance.com> wrote:
> 
> > This series amis to add hugetlb_optimize_vmemmap sysctl to enable or disable
> > the feature of optimizing vmemmap pages associated with HugeTLB pages.
> > 
> > v12:
> >   - Add patch 3 and patch 4 to handle the coexistence of hugetlb_free_vmemmap
> >     and memory_hotplug.memmap_on_memory (David).
> >   - Remove Reviewed-by from Mike in the last parch since it is changed.
> 
> v11 is in mm-stable now, so please send patches against that.  AFAICT
> that will be quite straightforward.
>

IIUC, the mm-stable is introduced recently, and we consider those commits
in mm-stable are stable (meanning the commit sha1 is stable as well).
I'll send a new patchset based on mm-stable.

Thanks.

Re: [PATCH v12 1/7] mm: hugetlb_vmemmap: disable hugetlb_optimize_vmemmap when struct page crosses page boundaries

[Oscar Salvador]

On Mon, May 16, 2022 at 06:22:05PM +0800, Muchun Song wrote:
> If the size of "struct page" is not the power of two but with the feature
> of minimizing overhead of struct page associated with each HugeTLB is
> enabled, then the vmemmap pages of HugeTLB will be corrupted after
> remapping (panic is about to happen in theory).  But this only exists when
> !CONFIG_MEMCG && !CONFIG_SLUB on x86_64.  However, it is not a conventional
> configuration nowadays.  So it is not a real word issue, just the result
> of a code review.
> 
> But we cannot prevent anyone from configuring that combined configure.
> This hugetlb_optimize_vmemmap should be disable in this case to fix this
> issue.
> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
> Acked-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

-- 
Oscar Salvador
SUSE Labs

Re: [PATCH v12 2/7] mm: hugetlb_vmemmap: use kstrtobool for hugetlb_vmemmap param parsing

[Oscar Salvador]

On Mon, May 16, 2022 at 06:22:06PM +0800, Muchun Song wrote:
> Use kstrtobool rather than open coding "on" and "off" parsing in
> mm/hugetlb_vmemmap.c,  which is more powerful to handle all kinds
> of parameters like 'Yy1Nn0' or [oO][NnFf] for "on" and "off".
> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
> Acked-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>


-- 
Oscar Salvador
SUSE Labs

Re: [PATCH v12 3/7] mm: memory_hotplug: enumerate all supported section flags

[Oscar Salvador]

On Mon, May 16, 2022 at 06:22:07PM +0800, Muchun Song wrote:
> We are almost running out of free slots, only one bit is available in the

I would be more precise about what are we running out of. Free slots of
what?

> worst case (powerpc with 256k pages).  However, there are still some free
> slots on other architectures (e.g. x86_64 has 10 bits available, arm64
> has 8 bits available with worst case of 64K pages).  We have hard coded
> those numbers in code, it is inconvenient to use those bits on other
> architectures except powerpc.  So transfer those section flags to
> enumeration to make it easy to add new section flags in the future. Also,
> move SECTION_TAINT_ZONE_DEVICE into the scope of CONFIG_ZONE_DEVICE
> to save a bit on non-zone-device case.
> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
...
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1418,16 +1418,37 @@ extern size_t mem_section_usage_size(void);
>   *      (equal SECTION_SIZE_BITS - PAGE_SHIFT), and the
>   *      worst combination is powerpc with 256k pages,
>   *      which results in PFN_SECTION_SHIFT equal 6.
> - * To sum it up, at least 6 bits are available.
> + * To sum it up, at least 6 bits are available on all architectures.
> + * However, we can exceed 6 bits on some other architectures except
> + * powerpc (e.g. 15 bits are available on x86_64, 13 bits are available
> + * with the worst case of 64K pages on arm64) if we make sure the
> + * exceeded bit is not applicable to powerpc.
>   */
> -#define SECTION_MARKED_PRESENT		(1UL<<0)
> -#define SECTION_HAS_MEM_MAP		(1UL<<1)
> -#define SECTION_IS_ONLINE		(1UL<<2)
> -#define SECTION_IS_EARLY		(1UL<<3)
> -#define SECTION_TAINT_ZONE_DEVICE	(1UL<<4)
> -#define SECTION_MAP_LAST_BIT		(1UL<<5)
> +#define ENUM_SECTION_FLAG(MAPPER)						\
> +	MAPPER(MARKED_PRESENT)							\
> +	MAPPER(HAS_MEM_MAP)							\
> +	MAPPER(IS_ONLINE)							\
> +	MAPPER(IS_EARLY)							\
> +	MAPPER(TAINT_ZONE_DEVICE, CONFIG_ZONE_DEVICE)				\
> +	MAPPER(MAP_LAST_BIT)
> +
> +#define __SECTION_SHIFT_FLAG_MAPPER_0(x)
> +#define __SECTION_SHIFT_FLAG_MAPPER_1(x)	SECTION_##x##_SHIFT,
> +#define __SECTION_SHIFT_FLAG_MAPPER(x, ...)	\
> +	__PASTE(__SECTION_SHIFT_FLAG_MAPPER_, IS_ENABLED(__VA_ARGS__))(x)
> +
> +#define __SECTION_FLAG_MAPPER_0(x)
> +#define __SECTION_FLAG_MAPPER_1(x)		SECTION_##x = BIT(SECTION_##x##_SHIFT),
> +#define __SECTION_FLAG_MAPPER(x, ...)		\
> +	__PASTE(__SECTION_FLAG_MAPPER_, IS_ENABLED(__VA_ARGS__))(x)
> +
> +enum {
> +	ENUM_SECTION_FLAG(__SECTION_SHIFT_FLAG_MAPPER)
> +	ENUM_SECTION_FLAG(__SECTION_FLAG_MAPPER)
> +};
> +
>  #define SECTION_MAP_MASK		(~(SECTION_MAP_LAST_BIT-1))
> -#define SECTION_NID_SHIFT		6
> +#define SECTION_NID_SHIFT		SECTION_MAP_LAST_BIT_SHIFT

Is this really worth the extra code? And it might be me that I am not
familiar with all this magic, but it looks overcomplicated.
Maybe some comments here and there help clarifying what it is going on
here.

>  static inline struct page *__section_mem_map_addr(struct mem_section *section)
>  {
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 111684878fd9..aef3f041dec7 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -655,12 +655,18 @@ static void __meminit resize_pgdat_range(struct pglist_data *pgdat, unsigned lon
>  
>  }
>  
> +#ifdef CONFIG_ZONE_DEVICE
>  static void section_taint_zone_device(unsigned long pfn)
>  {
>  	struct mem_section *ms = __pfn_to_section(pfn);
>  
>  	ms->section_mem_map |= SECTION_TAINT_ZONE_DEVICE;
>  }
> +#else
> +static inline void section_taint_zone_device(unsigned long pfn)
> +{
> +}
> +#endif
>  
>  /*
>   * Associate the pfn range with the given zone, initializing the memmaps
> -- 
> 2.11.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

Re: [PATCH v12 4/7] mm: hotplug: introduce SECTION_CANNOT_OPTIMIZE_VMEMMAP

[Oscar Salvador]

On Mon, May 16, 2022 at 08:03:49PM +0800, Muchun Song wrote:
> On Mon, May 16, 2022 at 12:38:46PM +0200, Oscar Salvador wrote:
> > On Mon, May 16, 2022 at 06:22:08PM +0800, Muchun Song wrote:
> > > --- a/mm/sparse.c
> > > +++ b/mm/sparse.c
> > > @@ -913,6 +913,13 @@ int __meminit sparse_add_section(int nid, unsigned long start_pfn,
> > >  	ms = __nr_to_section(section_nr);
> > >  	set_section_nid(section_nr, nid);
> > >  	__section_mark_present(ms, section_nr);
> > > +	/*
> > > +	 * Mark whole section as non-optimizable once there is a subsection
> > > +	 * whose vmemmap pages are allocated from alternative allocator. The
> > > +	 * early section is always optimizable.
> > > +	 */
> > > +	if (!early_section(ms) && altmap)
> > > +		section_mark_cannot_optimize_vmemmap(ms);
> > 
> > Because no one expects those sections to be removed?
> > IIRC, early_section + altmap only happened in case of sub-section pmem
> > scenario?
> 
> Right. The commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")
> has more information.
> 
> > I guess my question is: can we really have early_sections coming
> > from alternative allocator?
> >
> 
> We can't. The early section does not consider partially being
> populated currently.

Then, IIUC, we can forget about the early_section() check?


-- 
Oscar Salvador
SUSE Labs

Re: [PATCH v12 7/7] mm: hugetlb_vmemmap: add hugetlb_optimize_vmemmap sysctl

[Oscar Salvador]

On Mon, May 16, 2022 at 06:22:11PM +0800, Muchun Song wrote:
> We must add hugetlb_free_vmemmap=on (or "off") to the boot cmdline and
> reboot the server to enable or disable the feature of optimizing vmemmap
> pages associated with HugeTLB pages.  However, rebooting usually takes a
> long time.  So add a sysctl to enable or disable the feature at runtime
> without rebooting.  Why we need this?  There are 3 use cases.
> 
> 1) The feature of minimizing overhead of struct page associated with each
> HugeTLB is disabled by default without passing "hugetlb_free_vmemmap=on"
> to the boot cmdline. When we (ByteDance) deliver the servers to the
> users who want to enable this feature, they have to configure the grub
> (change boot cmdline) and reboot the servers, whereas rebooting usually
> takes a long time (we have thousands of servers).  It's a very bad
> experience for the users.  So we need a approach to enable this feature
> after rebooting. This is a use case in our practical environment.
> 
> 2) Some use cases are that HugeTLB pages are allocated 'on the fly'
> instead of being pulled from the HugeTLB pool, those workloads would be
> affected with this feature enabled.  Those workloads could be identified
> by the characteristics of they never explicitly allocating huge pages
> with 'nr_hugepages' but only set 'nr_overcommit_hugepages' and then let
> the pages be allocated from the buddy allocator at fault time.  We can
> confirm it is a real use case from the commit 099730d67417.  For those
> workloads, the page fault time could be ~2x slower than before. We
> suspect those users want to disable this feature if the system has enabled
> this before and they don't think the memory savings benefit is enough to
> make up for the performance drop.
> 
> 3) If the workload which wants vmemmap pages to be optimized and the
> workload which wants to set 'nr_overcommit_hugepages' and does not want
> the extera overhead at fault time when the overcommitted pages be
> allocated from the buddy allocator are deployed in the same server.
> The user could enable this feature and set 'nr_hugepages' and
> 'nr_overcommit_hugepages', then disable the feature.  In this case,
> the overcommited HugeTLB pages will not encounter the extra overhead
> at fault time.

I am having issues parsing point 3), specially the first part.
IIUC, you are saying we have two kind of different workloads:

- one that wants to have hugetlb vmemmap pages optimized
- one that wants to allocate hugetlb pages at fault time rather than
  allocating them via /proc/..., but does not want to suffer the
  overhead of optimizing the vmemmap pages when faulting them

Then you say the user could enable the optimization and allocate
those pages via nr_hugepages, and then disable the feature.
So, when we fault in those pages, the pages are already in the
pool, right? And are already optimized.

> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  Documentation/admin-guide/sysctl/vm.rst | 38 ++++++++++++++++++++
>  include/linux/page-flags.h              |  6 ++--
>  mm/hugetlb_vmemmap.c                    | 61 ++++++++++++++++++++++-----------
>  3 files changed, 81 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/admin-guide/sysctl/vm.rst b/Documentation/admin-guide/sysctl/vm.rst
> index 747e325ebcd0..d7374a1e8ac9 100644
> --- a/Documentation/admin-guide/sysctl/vm.rst
> +++ b/Documentation/admin-guide/sysctl/vm.rst
> @@ -562,6 +562,44 @@ Change the minimum size of the hugepage pool.
>  See Documentation/admin-guide/mm/hugetlbpage.rst
>  
>  
> +hugetlb_optimize_vmemmap
> +========================
> +
> +This knob is not available when the size of 'struct page' (a structure defined
> +in include/linux/mm_types.h) is not power of two (an unusual system config could
> +result in this).
> +
> +Enable (set to 1) or disable (set to 0) the feature of optimizing vmemmap pages
> +associated with each HugeTLB page.
> +
> +Once enabled, the vmemmap pages of subsequent allocation of HugeTLB pages from
> +buddy allocator will be optimized (7 pages per 2MB HugeTLB page and 4095 pages
> +per 1GB HugeTLB page), whereas already allocated HugeTLB pages will not be
> +optimized.  When those optimized HugeTLB pages are freed from the HugeTLB pool
> +to the buddy allocator, the vmemmap pages representing that range needs to be
> +remapped again and the vmemmap pages discarded earlier need to be rellocated
> +again.  If your use case is that HugeTLB pages are allocated 'on the fly' (e.g.
> +never explicitly allocating HugeTLB pages with 'nr_hugepages' but only set
> +'nr_overcommit_hugepages', those overcommitted HugeTLB pages are allocated 'on
> +the fly') instead of being pulled from the HugeTLB pool, you should weigh the
> +benefits of memory savings against the more overhead (~2x slower than before)
> +of allocation or freeing HugeTLB pages between the HugeTLB pool and the buddy
> +allocator.  Another behavior to note is that if the system is under heavy memory
> +pressure, it could prevent the user from freeing HugeTLB pages from the HugeTLB
> +pool to the buddy allocator since the allocation of vmemmap pages could be
> +failed, you have to retry later if your system encounter this situation.
> +
> +Once disabled, the vmemmap pages of subsequent allocation of HugeTLB pages from
> +buddy allocator will not be optimized meaning the extra overhead at allocation
> +time from buddy allocator disappears, whereas already optimized HugeTLB pages
> +will not be affected.  If you want to make sure there are no optimized HugeTLB
> +pages, you can set "nr_hugepages" to 0 first and then disable this.  Note that
> +writing 0 to nr_hugepages will make any "in use" HugeTLB pages become surplus
> +pages.  So, those surplus pages are still optimized until they are no longer
> +in use.  You would need to wait for those surplus pages to be released before
> +there are no optimized pages in the system.
> +
> +
>  nr_hugepages_mempolicy
>  ======================
>  
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 404f4ede17f5..07d8d444d9f1 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -200,8 +200,7 @@ enum pageflags {
>  #ifndef __GENERATING_BOUNDS_H
>  
>  #ifdef CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP
> -DECLARE_STATIC_KEY_MAYBE(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
> -			 hugetlb_optimize_vmemmap_key);
> +DECLARE_STATIC_KEY_FALSE(hugetlb_optimize_vmemmap_key);
>  
>  /*
>   * If the feature of optimizing vmemmap pages associated with each HugeTLB
> @@ -221,8 +220,7 @@ DECLARE_STATIC_KEY_MAYBE(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
>   */
>  static __always_inline const struct page *page_fixed_fake_head(const struct page *page)
>  {
> -	if (!static_branch_maybe(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
> -				 &hugetlb_optimize_vmemmap_key))
> +	if (!static_branch_unlikely(&hugetlb_optimize_vmemmap_key))
>  		return page;
>  
>  	/*
> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index d1fea65fec98..02862f117c2b 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c
> @@ -22,23 +22,15 @@
>  #define RESERVE_VMEMMAP_NR		1U
>  #define RESERVE_VMEMMAP_SIZE		(RESERVE_VMEMMAP_NR << PAGE_SHIFT)
>  
> -DEFINE_STATIC_KEY_MAYBE(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
> -			hugetlb_optimize_vmemmap_key);
> +DEFINE_STATIC_KEY_FALSE(hugetlb_optimize_vmemmap_key);
>  EXPORT_SYMBOL(hugetlb_optimize_vmemmap_key);
>  
> +static bool optimize_vmemmap_enabled =
> +	IS_ENABLED(CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON);
> +
>  static int __init hugetlb_vmemmap_early_param(char *buf)
>  {
> -	bool enable;
> -
> -	if (kstrtobool(buf, &enable))
> -		return -EINVAL;
> -
> -	if (enable)
> -		static_branch_enable(&hugetlb_optimize_vmemmap_key);
> -	else
> -		static_branch_disable(&hugetlb_optimize_vmemmap_key);
> -
> -	return 0;
> +	return kstrtobool(buf, &optimize_vmemmap_enabled);
>  }
>  early_param("hugetlb_free_vmemmap", hugetlb_vmemmap_early_param);
>  
> @@ -69,8 +61,10 @@ int hugetlb_vmemmap_alloc(struct hstate *h, struct page *head)
>  	 */
>  	ret = vmemmap_remap_alloc(vmemmap_addr, vmemmap_end, vmemmap_reuse,
>  				  GFP_KERNEL | __GFP_NORETRY | __GFP_THISNODE);
> -	if (!ret)
> +	if (!ret) {
>  		ClearHPageVmemmapOptimized(head);
> +		static_branch_dec(&hugetlb_optimize_vmemmap_key);
> +	}
>  
>  	return ret;
>  }
> @@ -81,6 +75,9 @@ static unsigned int optimizable_vmemmap_pages(struct hstate *h,
>  	unsigned long pfn = page_to_pfn(head);
>  	unsigned long end = pfn + pages_per_huge_page(h);
>  
> +	if (!READ_ONCE(optimize_vmemmap_enabled))
> +		return 0;
> +
>  	for (; pfn < end; pfn += PAGES_PER_SECTION) {
>  		if (section_cannot_optimize_vmemmap(__pfn_to_section(pfn)))
>  			return 0;
> @@ -98,6 +95,8 @@ void hugetlb_vmemmap_free(struct hstate *h, struct page *head)
>  	if (!vmemmap_pages)
>  		return;
>  
> +	static_branch_inc(&hugetlb_optimize_vmemmap_key);
> +
>  	vmemmap_addr	+= RESERVE_VMEMMAP_SIZE;
>  	vmemmap_end	= vmemmap_addr + (vmemmap_pages << PAGE_SHIFT);
>  	vmemmap_reuse	= vmemmap_addr - PAGE_SIZE;
> @@ -107,7 +106,9 @@ void hugetlb_vmemmap_free(struct hstate *h, struct page *head)
>  	 * to the page which @vmemmap_reuse is mapped to, then free the pages
>  	 * which the range [@vmemmap_addr, @vmemmap_end] is mapped to.
>  	 */
> -	if (!vmemmap_remap_free(vmemmap_addr, vmemmap_end, vmemmap_reuse))
> +	if (vmemmap_remap_free(vmemmap_addr, vmemmap_end, vmemmap_reuse))
> +		static_branch_dec(&hugetlb_optimize_vmemmap_key);
> +	else
>  		SetHPageVmemmapOptimized(head);
>  }
>  
> @@ -124,13 +125,8 @@ void __init hugetlb_vmemmap_init(struct hstate *h)
>  	BUILD_BUG_ON(__NR_USED_SUBPAGE >=
>  		     RESERVE_VMEMMAP_SIZE / sizeof(struct page));
>  
> -	if (!static_branch_maybe(CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON,
> -				 &hugetlb_optimize_vmemmap_key))
> -		return;
> -
>  	if (!is_power_of_2(sizeof(struct page))) {
>  		pr_warn_once("cannot optimize vmemmap pages because \"struct page\" crosses page boundaries\n");
> -		static_branch_disable(&hugetlb_optimize_vmemmap_key);
>  		return;
>  	}
>  
> @@ -149,3 +145,28 @@ void __init hugetlb_vmemmap_init(struct hstate *h)
>  	pr_info("can optimize %d vmemmap pages for %s\n",
>  		h->optimize_vmemmap_pages, h->name);
>  }
> +
> +#ifdef CONFIG_PROC_SYSCTL
> +static struct ctl_table hugetlb_vmemmap_sysctls[] = {
> +	{
> +		.procname	= "hugetlb_optimize_vmemmap",
> +		.data		= &optimize_vmemmap_enabled,
> +		.maxlen		= sizeof(optimize_vmemmap_enabled),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dobool,
> +	},
> +	{ }
> +};
> +
> +static int __init hugetlb_vmemmap_sysctls_init(void)
> +{
> +	/*
> +	 * If "struct page" crosses page boundaries, the vmemmap pages cannot
> +	 * be optimized.
> +	 */
> +	if (is_power_of_2(sizeof(struct page)))
> +		register_sysctl_init("vm", hugetlb_vmemmap_sysctls);
> +	return 0;
> +}
> +late_initcall(hugetlb_vmemmap_sysctls_init);
> +#endif /* CONFIG_PROC_SYSCTL */
> -- 
> 2.11.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

Re: [PATCH v12 4/7] mm: hotplug: introduce SECTION_CANNOT_OPTIMIZE_VMEMMAP

[Muchun Song]

On Tue, May 17, 2022 at 09:52:36AM +0200, Oscar Salvador wrote:
> On Mon, May 16, 2022 at 08:03:49PM +0800, Muchun Song wrote:
> > On Mon, May 16, 2022 at 12:38:46PM +0200, Oscar Salvador wrote:
> > > On Mon, May 16, 2022 at 06:22:08PM +0800, Muchun Song wrote:
> > > > --- a/mm/sparse.c
> > > > +++ b/mm/sparse.c
> > > > @@ -913,6 +913,13 @@ int __meminit sparse_add_section(int nid, unsigned long start_pfn,
> > > >  	ms = __nr_to_section(section_nr);
> > > >  	set_section_nid(section_nr, nid);
> > > >  	__section_mark_present(ms, section_nr);
> > > > +	/*
> > > > +	 * Mark whole section as non-optimizable once there is a subsection
> > > > +	 * whose vmemmap pages are allocated from alternative allocator. The
> > > > +	 * early section is always optimizable.
> > > > +	 */
> > > > +	if (!early_section(ms) && altmap)
> > > > +		section_mark_cannot_optimize_vmemmap(ms);
> > > 
> > > Because no one expects those sections to be removed?
> > > IIRC, early_section + altmap only happened in case of sub-section pmem
> > > scenario?
> > 
> > Right. The commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")
> > has more information.
> > 
> > > I guess my question is: can we really have early_sections coming
> > > from alternative allocator?
> > >
> > 
> > We can't. The early section does not consider partially being
> > populated currently.
> 
> Then, IIUC, we can forget about the early_section() check?
>

Sorry for the confusing. I mean early_section() should be checked.
I could find a comment in section_activate, that says:

	/*
	 * The early init code does not consider partially populated
	 * initial sections, it simply assumes that memory will never be
	 * referenced.  If we hot-add memory into such a section then we
	 * do not need to populate the memmap and can simply reuse what
	 * is already there.
	 */
	if (nr_pages < PAGES_PER_SECTION && early_section(ms))
		return pfn_to_page(pfn);
 
We can see that we could hot-add a sub-section within a early section.
So I think early_section + altmap could happened in this case, then
we could not drop that check. Right?

Thanks.

Re: [PATCH v12 3/7] mm: memory_hotplug: enumerate all supported section flags

[Muchun Song]

On Tue, May 17, 2022 at 09:47:48AM +0200, Oscar Salvador wrote:
> On Mon, May 16, 2022 at 06:22:07PM +0800, Muchun Song wrote:
> > We are almost running out of free slots, only one bit is available in the
> 
> I would be more precise about what are we running out of. Free slots of
> what?
> 
> > worst case (powerpc with 256k pages).  However, there are still some free
> > slots on other architectures (e.g. x86_64 has 10 bits available, arm64
> > has 8 bits available with worst case of 64K pages).  We have hard coded
> > those numbers in code, it is inconvenient to use those bits on other
> > architectures except powerpc.  So transfer those section flags to
> > enumeration to make it easy to add new section flags in the future. Also,
> > move SECTION_TAINT_ZONE_DEVICE into the scope of CONFIG_ZONE_DEVICE
> > to save a bit on non-zone-device case.
> > 
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > ---
> ...
> > --- a/include/linux/mmzone.h
> > +++ b/include/linux/mmzone.h
> > @@ -1418,16 +1418,37 @@ extern size_t mem_section_usage_size(void);
> >   *      (equal SECTION_SIZE_BITS - PAGE_SHIFT), and the
> >   *      worst combination is powerpc with 256k pages,
> >   *      which results in PFN_SECTION_SHIFT equal 6.
> > - * To sum it up, at least 6 bits are available.
> > + * To sum it up, at least 6 bits are available on all architectures.
> > + * However, we can exceed 6 bits on some other architectures except
> > + * powerpc (e.g. 15 bits are available on x86_64, 13 bits are available
> > + * with the worst case of 64K pages on arm64) if we make sure the
> > + * exceeded bit is not applicable to powerpc.
> >   */
> > -#define SECTION_MARKED_PRESENT		(1UL<<0)
> > -#define SECTION_HAS_MEM_MAP		(1UL<<1)
> > -#define SECTION_IS_ONLINE		(1UL<<2)
> > -#define SECTION_IS_EARLY		(1UL<<3)
> > -#define SECTION_TAINT_ZONE_DEVICE	(1UL<<4)
> > -#define SECTION_MAP_LAST_BIT		(1UL<<5)
> > +#define ENUM_SECTION_FLAG(MAPPER)						\
> > +	MAPPER(MARKED_PRESENT)							\
> > +	MAPPER(HAS_MEM_MAP)							\
> > +	MAPPER(IS_ONLINE)							\
> > +	MAPPER(IS_EARLY)							\
> > +	MAPPER(TAINT_ZONE_DEVICE, CONFIG_ZONE_DEVICE)				\
> > +	MAPPER(MAP_LAST_BIT)
> > +
> > +#define __SECTION_SHIFT_FLAG_MAPPER_0(x)
> > +#define __SECTION_SHIFT_FLAG_MAPPER_1(x)	SECTION_##x##_SHIFT,
> > +#define __SECTION_SHIFT_FLAG_MAPPER(x, ...)	\
> > +	__PASTE(__SECTION_SHIFT_FLAG_MAPPER_, IS_ENABLED(__VA_ARGS__))(x)
> > +
> > +#define __SECTION_FLAG_MAPPER_0(x)
> > +#define __SECTION_FLAG_MAPPER_1(x)		SECTION_##x = BIT(SECTION_##x##_SHIFT),
> > +#define __SECTION_FLAG_MAPPER(x, ...)		\
> > +	__PASTE(__SECTION_FLAG_MAPPER_, IS_ENABLED(__VA_ARGS__))(x)
> > +
> > +enum {
> > +	ENUM_SECTION_FLAG(__SECTION_SHIFT_FLAG_MAPPER)
> > +	ENUM_SECTION_FLAG(__SECTION_FLAG_MAPPER)
> > +};
> > +
> >  #define SECTION_MAP_MASK		(~(SECTION_MAP_LAST_BIT-1))
> > -#define SECTION_NID_SHIFT		6
> > +#define SECTION_NID_SHIFT		SECTION_MAP_LAST_BIT_SHIFT
> 
> Is this really worth the extra code? And it might be me that I am not
> familiar with all this magic, but it looks overcomplicated.
> Maybe some comments here and there help clarifying what it is going on
> here.
>

Yeah, it's a little complicated. All the magic aims to generate
two enumeration from one MAPPER(xxx, config), one is SECTION_xxx_SHIFT,
another is SECTION_xxx = BIT(SECTION_xxx_SHIFT) if the 'config' is
configured. If we want to add a new flag, like the follow patch, just
one line could do that.

  MAPPER(CANNOT_OPTIMIZE_VMEMMAP, CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP)

Without those magic, we have to add 4 lines like follows to do the
similar thing.

  #ifdef CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP
  	SECTION_CANNOT_OPTIMIZE_VMEMMAP_SHIFT,
  #define SECTION_CANNOT_OPTIMIZE_VMEMMAP BIT(SECTION_CANNOT_OPTIMIZE_VMEMMAP_SHIFT)
  #endif

I admit it is more clear but not simplified as above approach.

Both two approaches are fine to me. If we choose the simplified
one, I agree with you I should add more comments to explain
what happens here.

Thanks.

 

Re: [PATCH v12 7/7] mm: hugetlb_vmemmap: add hugetlb_optimize_vmemmap sysctl

[Muchun Song]

On Tue, May 17, 2022 at 10:06:51AM +0200, Oscar Salvador wrote:
> On Mon, May 16, 2022 at 06:22:11PM +0800, Muchun Song wrote:
> > We must add hugetlb_free_vmemmap=on (or "off") to the boot cmdline and
> > reboot the server to enable or disable the feature of optimizing vmemmap
> > pages associated with HugeTLB pages.  However, rebooting usually takes a
> > long time.  So add a sysctl to enable or disable the feature at runtime
> > without rebooting.  Why we need this?  There are 3 use cases.
> > 
> > 1) The feature of minimizing overhead of struct page associated with each
> > HugeTLB is disabled by default without passing "hugetlb_free_vmemmap=on"
> > to the boot cmdline. When we (ByteDance) deliver the servers to the
> > users who want to enable this feature, they have to configure the grub
> > (change boot cmdline) and reboot the servers, whereas rebooting usually
> > takes a long time (we have thousands of servers).  It's a very bad
> > experience for the users.  So we need a approach to enable this feature
> > after rebooting. This is a use case in our practical environment.
> > 
> > 2) Some use cases are that HugeTLB pages are allocated 'on the fly'
> > instead of being pulled from the HugeTLB pool, those workloads would be
> > affected with this feature enabled.  Those workloads could be identified
> > by the characteristics of they never explicitly allocating huge pages
> > with 'nr_hugepages' but only set 'nr_overcommit_hugepages' and then let
> > the pages be allocated from the buddy allocator at fault time.  We can
> > confirm it is a real use case from the commit 099730d67417.  For those
> > workloads, the page fault time could be ~2x slower than before. We
> > suspect those users want to disable this feature if the system has enabled
> > this before and they don't think the memory savings benefit is enough to
> > make up for the performance drop.
> > 
> > 3) If the workload which wants vmemmap pages to be optimized and the
> > workload which wants to set 'nr_overcommit_hugepages' and does not want
> > the extera overhead at fault time when the overcommitted pages be
> > allocated from the buddy allocator are deployed in the same server.
> > The user could enable this feature and set 'nr_hugepages' and
> > 'nr_overcommit_hugepages', then disable the feature.  In this case,
> > the overcommited HugeTLB pages will not encounter the extra overhead
> > at fault time.
> 
> I am having issues parsing point 3), specially the first part.
> IIUC, you are saying we have two kind of different workloads:
> 
> - one that wants to have hugetlb vmemmap pages optimized
> - one that wants to allocate hugetlb pages at fault time rather than
>   allocating them via /proc/..., but does not want to suffer the
>   overhead of optimizing the vmemmap pages when faulting them

I need to clarify this workload, the one that does not want to
suffer the overhead of optimizing the vmemmap pages when faulting
them instead of wanting to allocate hugetlb pages at fault time.
It is different from the one in the case 2). This one usually
configures 'nr_overcommit_hugepages' as well as 'nr_hugepages',
if it does not want to suffer the overhead of optimizing the
vmemmap pages when faulting pages (must be overcommitted pages),
then they could follow the steps mentioned above.

> 
> Then you say the user could enable the optimization and allocate
> those pages via nr_hugepages, and then disable the feature.
> So, when we fault in those pages, the pages are already in the
> pool, right? And are already optimized.
>

I mean the overcommitted pages (it could be allocated at fault
time) as explained above.

Thanks.