diff mbox series

[v2,5/6] mm: move allocation of gigantic hstates to the start of mm_core_init

Message ID 20230730151606.2871391-6-usama.arif@bytedance.com
State New
Headers
Series mm/memblock: Skip prep and initialization of struct pages freed later by HVO |

Commit Message

Usama Arif July 30, 2023, 3:16 p.m. UTC 
  Whether the initialization of tail struct pages of a hugepage
happens or not will become dependent on the commandline
parameter hugetlb_free_vmemmap in the future. Hence,
hugetlb_hstate_alloc_pages needs to be after command line parameters
are parsed and the start of mm_core_init is a good point.

Signed-off-by: Usama Arif <usama.arif@bytedance.com>
---
 mm/hugetlb.c  | 18 ++++++++++--------
 mm/internal.h |  9 +++++++++
 mm/mm_init.c  |  6 ++++++
 3 files changed, 25 insertions(+), 8 deletions(-)

Comments

kernel test robot July 30, 2023, 4:49 p.m. UTC | #1 
Hi Usama,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Usama-Arif/mm-hugetlb-Skip-prep-of-tail-pages-when-HVO-is-enabled/20230730-231750
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20230730151606.2871391-6-usama.arif%40bytedance.com
patch subject: [v2 5/6] mm: move allocation of gigantic hstates to the start of mm_core_init
config: csky-randconfig-r034-20230730 (https://download.01.org/0day-ci/archive/20230731/202307310053.hx4wsFnM-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230731/202307310053.hx4wsFnM-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307310053.hx4wsFnM-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from mm/readahead.c:132:
>> mm/internal.h:1143:1: error: expected identifier or '(' before '{' token
    1143 | {
         | ^
>> mm/internal.h:1142:27: warning: 'hugetlb_hstate_alloc_gigantic_pages' declared 'static' but never defined [-Wunused-function]
    1142 | static inline void __init hugetlb_hstate_alloc_gigantic_pages(void);
         |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   In file included from mm/mm_init.c:30:
>> mm/internal.h:1143:1: error: expected identifier or '(' before '{' token
    1143 | {
         | ^
>> mm/internal.h:1142:27: warning: 'hugetlb_hstate_alloc_gigantic_pages' used but never defined
    1142 | static inline void __init hugetlb_hstate_alloc_gigantic_pages(void);
         |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   In file included from lib/vsprintf.c:49:
>> lib/../mm/internal.h:1143:1: error: expected identifier or '(' before '{' token
    1143 | {
         | ^
   lib/vsprintf.c: In function 'va_format':
   lib/vsprintf.c:1681:9: warning: function 'va_format' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
    1681 |         buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
         |         ^~~
   lib/../mm/internal.h: At top level:
>> lib/../mm/internal.h:1142:27: warning: 'hugetlb_hstate_alloc_gigantic_pages' declared 'static' but never defined [-Wunused-function]
    1142 | static inline void __init hugetlb_hstate_alloc_gigantic_pages(void);
         |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +1143 mm/internal.h

  1138	
  1139	#ifdef CONFIG_HUGETLBFS
  1140	void __init hugetlb_hstate_alloc_gigantic_pages(void);
  1141	#else
> 1142	static inline void __init hugetlb_hstate_alloc_gigantic_pages(void);
> 1143	{
  1144	}
  1145	#endif /* CONFIG_HUGETLBFS */
  1146
Muchun Song Aug. 1, 2023, 3:07 a.m. UTC | #2 
On 2023/7/30 23:16, Usama Arif wrote:
> Whether the initialization of tail struct pages of a hugepage
> happens or not will become dependent on the commandline
> parameter hugetlb_free_vmemmap in the future. Hence,
> hugetlb_hstate_alloc_pages needs to be after command line parameters
> are parsed and the start of mm_core_init is a good point.
>
> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
> ---
>   mm/hugetlb.c  | 18 ++++++++++--------
>   mm/internal.h |  9 +++++++++
>   mm/mm_init.c  |  6 ++++++
>   3 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 541c07b6d60f..bf60545496d7 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4400,14 +4400,6 @@ static int __init hugepages_setup(char *s)
>   		}
>   	}
>   
> -	/*
> -	 * Global state is always initialized later in hugetlb_init.
> -	 * But we need to allocate gigantic hstates here early to still
> -	 * use the bootmem allocator.
> -	 */
> -	if (hugetlb_max_hstate && hstate_is_gigantic(parsed_hstate))
> -		hugetlb_hstate_alloc_pages(parsed_hstate);
> -
>   	last_mhp = mhp;
>   
>   	return 1;
> @@ -4419,6 +4411,16 @@ static int __init hugepages_setup(char *s)
>   }
>   __setup("hugepages=", hugepages_setup);
>   
> +void __init hugetlb_hstate_alloc_gigantic_pages(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < HUGE_MAX_HSTATE; i++) {
> +		if (hstate_is_gigantic(&hstates[i]))
> +			hugetlb_hstate_alloc_pages(&hstates[i]);
> +	}
> +}
> +
>   /*
>    * hugepagesz command line processing
>    * A specific huge page size can only be specified once with hugepagesz.
> diff --git a/mm/internal.h b/mm/internal.h
> index a7d9e980429a..692bb1136a39 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1102,4 +1102,13 @@ struct vma_prepare {
>   	struct vm_area_struct *remove;
>   	struct vm_area_struct *remove2;
>   };
> +
> +#ifdef CONFIG_HUGETLBFS
> +void __init hugetlb_hstate_alloc_gigantic_pages(void);
> +#else
> +static inline void __init hugetlb_hstate_alloc_gigantic_pages(void);
> +{
> +}
> +#endif /* CONFIG_HUGETLBFS */
> +
>   #endif	/* __MM_INTERNAL_H */
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index a1963c3322af..f2751ccd7d99 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -26,6 +26,7 @@
>   #include <linux/pgtable.h>
>   #include <linux/swap.h>
>   #include <linux/cma.h>
> +#include <linux/hugetlb.h>
>   #include "internal.h"
>   #include "slab.h"
>   #include "shuffle.h"
> @@ -2768,6 +2769,11 @@ static void __init mem_init_print_info(void)
>    */
>   void __init mm_core_init(void)
>   {
> +	/*
> +	 * We need to allocate gigantic hstates here early to still use the bootmem
> +	 * allocator. Non gigantic hstates are initialized later in hugetlb_init.
> +	 */
> +	hugetlb_hstate_alloc_gigantic_pages();

Actually, I don't want to add hugetlb related information into a
common code. You just want to make hugetlb_hstate_alloc_gigantic_page()
happen after cmdline has been pased, maybe we could review this
cleanup [1], but it need some changes to satisfy your need. E.g. adding
"hugetlb_free_vmemmap" cmdline parsing in hugetlb_process_arg(). Then
we do not need to change mm/mm_init.c.

[1] 
https://lore.kernel.org/linux-mm/20220616071827.3480-1-songmuchun@bytedance.com/
>   	/* Initializations relying on SMP setup */
>   	build_all_zonelists(NULL);
>   	page_alloc_init_cpuhp();
diff mbox series

Patch

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 541c07b6d60f..bf60545496d7 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4400,14 +4400,6 @@  static int __init hugepages_setup(char *s)
 		}
 	}
 
-	/*
-	 * Global state is always initialized later in hugetlb_init.
-	 * But we need to allocate gigantic hstates here early to still
-	 * use the bootmem allocator.
-	 */
-	if (hugetlb_max_hstate && hstate_is_gigantic(parsed_hstate))
-		hugetlb_hstate_alloc_pages(parsed_hstate);
-
 	last_mhp = mhp;
 
 	return 1;
@@ -4419,6 +4411,16 @@  static int __init hugepages_setup(char *s)
 }
 __setup("hugepages=", hugepages_setup);
 
+void __init hugetlb_hstate_alloc_gigantic_pages(void)
+{
+	int i;
+
+	for (i = 0; i < HUGE_MAX_HSTATE; i++) {
+		if (hstate_is_gigantic(&hstates[i]))
+			hugetlb_hstate_alloc_pages(&hstates[i]);
+	}
+}
+
 /*
  * hugepagesz command line processing
  * A specific huge page size can only be specified once with hugepagesz.
diff --git a/mm/internal.h b/mm/internal.h
index a7d9e980429a..692bb1136a39 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1102,4 +1102,13 @@  struct vma_prepare {
 	struct vm_area_struct *remove;
 	struct vm_area_struct *remove2;
 };
+
+#ifdef CONFIG_HUGETLBFS
+void __init hugetlb_hstate_alloc_gigantic_pages(void);
+#else
+static inline void __init hugetlb_hstate_alloc_gigantic_pages(void);
+{
+}
+#endif /* CONFIG_HUGETLBFS */
+
 #endif	/* __MM_INTERNAL_H */
diff --git a/mm/mm_init.c b/mm/mm_init.c
index a1963c3322af..f2751ccd7d99 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -26,6 +26,7 @@ 
 #include <linux/pgtable.h>
 #include <linux/swap.h>
 #include <linux/cma.h>
+#include <linux/hugetlb.h>
 #include "internal.h"
 #include "slab.h"
 #include "shuffle.h"
@@ -2768,6 +2769,11 @@  static void __init mem_init_print_info(void)
  */
 void __init mm_core_init(void)
 {
+	/*
+	 * We need to allocate gigantic hstates here early to still use the bootmem
+	 * allocator. Non gigantic hstates are initialized later in hugetlb_init.
+	 */
+	hugetlb_hstate_alloc_gigantic_pages();
 	/* Initializations relying on SMP setup */
 	build_all_zonelists(NULL);
 	page_alloc_init_cpuhp();