Re: [PATCH v7 4/4] hugetlb: allow to free gigantic pages regardless of the configuration

From: Alex Ghiti
Date: Mon Mar 18 2019 - 03:01:03 EST

On 3/17/19 2:31 PM, christophe leroy wrote:


Le 17/03/2019 Ã 17:28, Alexandre Ghiti a ÃcritÂ:
On systems without CONTIG_ALLOC activated but that support gigantic pages,
boottime reserved gigantic pages can not be freed at all. This patch
simply enables the possibility to hand back those pages to memory
allocator.

Signed-off-by: Alexandre Ghiti <alex@xxxxxxxx>
Acked-by: David S. Miller <davem@xxxxxxxxxxxxx> [sparc]
---
 arch/arm64/Kconfig | 2 +-
 arch/arm64/include/asm/hugetlb.h | 4 --
 arch/powerpc/include/asm/book3s/64/hugetlb.h | 7 ---
 arch/powerpc/platforms/Kconfig.cputype | 2 +-
 arch/s390/Kconfig | 2 +-
 arch/s390/include/asm/hugetlb.h | 3 --
 arch/sh/Kconfig | 2 +-
 arch/sparc/Kconfig | 2 +-
 arch/x86/Kconfig | 2 +-
 arch/x86/include/asm/hugetlb.h | 4 --
 include/asm-generic/hugetlb.h | 14 +++++
 include/linux/gfp.h | 2 +-
 mm/hugetlb.c | 54 ++++++++++++++------
 mm/page_alloc.c | 4 +-
 14 files changed, 61 insertions(+), 43 deletions(-)


[...]

diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h
index 71d7b77eea50..aaf14974ee5f 100644
--- a/include/asm-generic/hugetlb.h
+++ b/include/asm-generic/hugetlb.h
@@ -126,4 +126,18 @@ static inline pte_t huge_ptep_get(pte_t *ptep)
 }
 #endif
 +#ifndef __HAVE_ARCH_GIGANTIC_PAGE_RUNTIME_SUPPORTED
+#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
+static inline bool gigantic_page_runtime_supported(void)
+{
+ÂÂÂ return true;
+}
+#else
+static inline bool gigantic_page_runtime_supported(void)
+{
+ÂÂÂ return false;
+}
+#endif /* CONFIG_ARCH_HAS_GIGANTIC_PAGE */

What about the following instead:

static inline bool gigantic_page_runtime_supported(void)
{
ÂÂÂÂreturn IS_ENABLED(CONFIG_ARCH_HAS_GIGANTIC_PAGE);
}


Totally, it already was like that in v2 or v3...



+#endif /* __HAVE_ARCH_GIGANTIC_PAGE_RUNTIME_SUPPORTED */
+
 #endif /* _ASM_GENERIC_HUGETLB_H */
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 1f1ad9aeebb9..58ea44bf75de 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -589,8 +589,8 @@ static inline bool pm_suspended_storage(void)
 /* The below functions must be run on a range from a single zone. */
 extern int alloc_contig_range(unsigned long start, unsigned long end,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ unsigned migratetype, gfp_t gfp_mask);
-extern void free_contig_range(unsigned long pfn, unsigned nr_pages);
 #endif
+extern void free_contig_range(unsigned long pfn, unsigned int nr_pages);

'extern' is unneeded and should be avoided (iaw checkpatch)


Ok, I did fix a checkpatch warning here, but did not notice the 'extern' one.


Thanks for your time,


Alex


Christophe

  #ifdef CONFIG_CMA
 /* CMA stuff */
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index afef61656c1e..4e55aa38704f 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1058,6 +1058,7 @@ static void free_gigantic_page(struct page *page, unsigned int order)
ÂÂÂÂÂ free_contig_range(page_to_pfn(page), 1 << order);
 }
 +#ifdef CONFIG_CONTIG_ALLOC
 static int __alloc_gigantic_page(unsigned long start_pfn,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ unsigned long nr_pages, gfp_t gfp_mask)
 {
@@ -1142,11 +1143,20 @@ static struct page *alloc_gigantic_page(struct hstate *h, gfp_t gfp_mask,
  static void prep_new_huge_page(struct hstate *h, struct page *page, int nid);
 static void prep_compound_gigantic_page(struct page *page, unsigned int order);
+#else /* !CONFIG_CONTIG_ALLOC */
+static struct page *alloc_gigantic_page(struct hstate *h, gfp_t gfp_mask,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ int nid, nodemask_t *nodemask)
+{
+ÂÂÂ return NULL;
+}
+#endif /* CONFIG_CONTIG_ALLOC */
  #else /* !CONFIG_ARCH_HAS_GIGANTIC_PAGE */
-static inline bool gigantic_page_supported(void) { return false; }
 static struct page *alloc_gigantic_page(struct hstate *h, gfp_t gfp_mask,
-ÂÂÂÂÂÂÂ int nid, nodemask_t *nodemask) { return NULL; }
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ int nid, nodemask_t *nodemask)
+{
+ÂÂÂ return NULL;
+}
 static inline void free_gigantic_page(struct page *page, unsigned int order) { }
 static inline void destroy_compound_gigantic_page(struct page *page,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ unsigned int order) { }
@@ -1156,7 +1166,7 @@ static void update_and_free_page(struct hstate *h, struct page *page)
 {
ÂÂÂÂÂ int i;
 - if (hstate_is_gigantic(h) && !gigantic_page_supported())
+ÂÂÂ if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
ÂÂÂÂÂÂÂÂÂ return;
 Â h->nr_huge_pages--;
@@ -2276,13 +2286,27 @@ static int adjust_pool_surplus(struct hstate *h, nodemask_t *nodes_allowed,
 }
  #define persistent_huge_pages(h) (h->nr_huge_pages - h->surplus_huge_pages)
-static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count,
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ nodemask_t *nodes_allowed)
+static int set_max_huge_pages(struct hstate *h, unsigned long count,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ nodemask_t *nodes_allowed)
 {
ÂÂÂÂÂ unsigned long min_count, ret;
 - if (hstate_is_gigantic(h) && !gigantic_page_supported())
-ÂÂÂÂÂÂÂ return h->max_huge_pages;
+ÂÂÂ spin_lock(&hugetlb_lock);
+
+ÂÂÂ /*
+ÂÂÂÂ * Gigantic pages runtime allocation depend on the capability for large
+ÂÂÂÂ * page range allocation.
+ÂÂÂÂ * If the system does not provide this feature, return an error when
+ÂÂÂÂ * the user tries to allocate gigantic pages but let the user free the
+ÂÂÂÂ * boottime allocated gigantic pages.
+ÂÂÂÂ */
+ÂÂÂ if (hstate_is_gigantic(h) && !IS_ENABLED(CONFIG_CONTIG_ALLOC)) {
+ÂÂÂÂÂÂÂ if (count > persistent_huge_pages(h)) {
+ÂÂÂÂÂÂÂÂÂÂÂ spin_unlock(&hugetlb_lock);
+ÂÂÂÂÂÂÂÂÂÂÂ return -EINVAL;
+ÂÂÂÂÂÂÂ }
+ÂÂÂÂÂÂÂ /* Fall through to decrease pool */
+ÂÂÂ }
 Â /*
ÂÂÂÂÂÂ * Increase the pool size
@@ -2295,7 +2319,6 @@ static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count,
ÂÂÂÂÂÂ * pool might be one hugepage larger than it needs to be, but
ÂÂÂÂÂÂ * within all the constraints specified by the sysctls.
ÂÂÂÂÂÂ */
-ÂÂÂ spin_lock(&hugetlb_lock);
ÂÂÂÂÂ while (h->surplus_huge_pages && count > persistent_huge_pages(h)) {
ÂÂÂÂÂÂÂÂÂ if (!adjust_pool_surplus(h, nodes_allowed, -1))
ÂÂÂÂÂÂÂÂÂÂÂÂÂ break;
@@ -2350,9 +2373,10 @@ static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count,
ÂÂÂÂÂÂÂÂÂÂÂÂÂ break;
ÂÂÂÂÂ }
 out:
-ÂÂÂ ret = persistent_huge_pages(h);
+ÂÂÂ h->max_huge_pages = persistent_huge_pages(h);
ÂÂÂÂÂ spin_unlock(&hugetlb_lock);
-ÂÂÂ return ret;
+
+ÂÂÂ return 0;
 }
  #define HSTATE_ATTR_RO(_name) \
@@ -2404,7 +2428,7 @@ static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
ÂÂÂÂÂ int err;
ÂÂÂÂÂ NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | __GFP_NORETRY);
 - if (hstate_is_gigantic(h) && !gigantic_page_supported()) {
+ÂÂÂ if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported()) {
ÂÂÂÂÂÂÂÂÂ err = -EINVAL;
ÂÂÂÂÂÂÂÂÂ goto out;
ÂÂÂÂÂ }
@@ -2428,15 +2452,13 @@ static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
ÂÂÂÂÂ } else
ÂÂÂÂÂÂÂÂÂ nodes_allowed = &node_states[N_MEMORY];
 - h->max_huge_pages = set_max_huge_pages(h, count, nodes_allowed);
+ÂÂÂ err = set_max_huge_pages(h, count, nodes_allowed);
 +out:
ÂÂÂÂÂ if (nodes_allowed != &node_states[N_MEMORY])
ÂÂÂÂÂÂÂÂÂ NODEMASK_FREE(nodes_allowed);
 - return len;
-out:
-ÂÂÂ NODEMASK_FREE(nodes_allowed);
-ÂÂÂ return err;
+ÂÂÂ return err ? err : len;
 }
  static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ac9c45ffb344..a4547d90fa7a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8234,8 +8234,9 @@ int alloc_contig_range(unsigned long start, unsigned long end,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ pfn_max_align_up(end), migratetype);
ÂÂÂÂÂ return ret;
 }
+#endif /* CONFIG_CONTIG_ALLOC */
 -void free_contig_range(unsigned long pfn, unsigned nr_pages)
+void free_contig_range(unsigned long pfn, unsigned int nr_pages)
 {
ÂÂÂÂÂ unsigned int count = 0;
 @@ -8247,7 +8248,6 @@ void free_contig_range(unsigned long pfn, unsigned nr_pages)
ÂÂÂÂÂ }
ÂÂÂÂÂ WARN(count != 0, "%d pages are still in use!\n", count);
 }
-#endif
  #ifdef CONFIG_MEMORY_HOTPLUG
 /*


---
L'absence de virus dans ce courrier Ãlectronique a Ãtà vÃrifiÃe par le logiciel antivirus Avast.
https://www.avast.com/antivirus