[PATCH v4 1/3] mm/memblock: introduce a new helper memblock_estimated_nr_free_pages()

[PATCH v4 1/3] mm/memblock: introduce a new helper memblock_estimated_nr_free_pages()

[Wei Yang]

During bootup, system may need the number of free pages in the whole system
to do some calculation before all pages are freed to buddy system. Usually
this number is get from totalram_pages(). Since we plan to move the free
pages accounting in __free_pages_core(), this value may not represent
total free pages at the early stage, especially when
CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled.

Instead of using raw memblock api, let's introduce a new helper for user
to get the estimated number of free pages from memblock point of view.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
CC: David Hildenbrand <david@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>

---
v4: adjust comment per david's suggestion
---
 include/linux/memblock.h |  1 +
 mm/memblock.c            | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 40c62aca36ec..7d1c32b3dc12 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -486,6 +486,7 @@ static inline __init_memblock bool memblock_bottom_up(void)
 
 phys_addr_t memblock_phys_mem_size(void);
 phys_addr_t memblock_reserved_size(void);
+unsigned long memblock_estimated_nr_pages(void);
 phys_addr_t memblock_start_of_DRAM(void);
 phys_addr_t memblock_end_of_DRAM(void);
 void memblock_enforce_memory_limit(phys_addr_t memory_limit);
diff --git a/mm/memblock.c b/mm/memblock.c
index e81fb68f7f88..26162902789f 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1729,6 +1729,26 @@ phys_addr_t __init_memblock memblock_reserved_size(void)
 	return memblock.reserved.total_size;
 }
 
+/**
+ * memblock_estimated_nr_free_pages - return estimated number of free pages
+ * from memblock point of view
+ *
+ * During bootup, subsystems might need a rough estimate of the number of free
+ * pages in the whole system, before precise numbers are available from the
+ * buddy. Especially with CONFIG_DEFERRED_STRUCT_PAGE_INIT, the numbers
+ * obtained from the buddy might be very imprecise during bootup.
+ *
+ * While we can get the estimated number of free pages from memblock, which is
+ * good enough for estimation.
+ *
+ * Return:
+ * An estimated number of free pages from memblock point of view.
+ */
+unsigned long __init memblock_estimated_nr_free_pages(void)
+{
+	return PHYS_PFN(memblock_phys_mem_size() - memblock_reserved_size());
+}
+
 /* lowest address */
 phys_addr_t __init_memblock memblock_start_of_DRAM(void)
 {
-- 
2.34.1

[PATCH v4 2/3] kernel/fork.c: get totalram_pages from memblock to calculate max_threads

[Wei Yang]

Since we plan to move the accounting into __free_pages_core(),
totalram_pages may not represent the total usable pages on system
at this point when defer_init is enabled.

Instead we can get the total estimated pages from memblock directly.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
CC: Mike Rapoport (IBM) <rppt@kernel.org>
CC: David Hildenbrand <david@redhat.com>
CC: Oleg Nesterov <oleg@redhat.com>
---
 kernel/fork.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 99076dbe27d8..e80e889543f3 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -44,6 +44,7 @@
 #include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/mm_inline.h>
+#include <linux/memblock.h>
 #include <linux/nsproxy.h>
 #include <linux/capability.h>
 #include <linux/cpu.h>
@@ -999,7 +1000,7 @@ void __init __weak arch_task_cache_init(void) { }
 static void set_max_threads(unsigned int max_threads_suggested)
 {
 	u64 threads;
-	unsigned long nr_pages = totalram_pages();
+	unsigned long nr_pages = memblock_estimated_nr_pages();
 
 	/*
 	 * The number of threads shall be limited such that the thread
-- 
2.34.1

Re: [PATCH v4 2/3] kernel/fork.c: get totalram_pages from memblock to calculate max_threads

[David Hildenbrand]

On 08.07.24 03:00, Wei Yang wrote:
> Since we plan to move the accounting into __free_pages_core(),
> totalram_pages may not represent the total usable pages on system
> at this point when defer_init is enabled.
> 
> Instead we can get the total estimated pages from memblock directly.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> CC: Mike Rapoport (IBM) <rppt@kernel.org>
> CC: David Hildenbrand <david@redhat.com>
> CC: Oleg Nesterov <oleg@redhat.com>
> ---
>   kernel/fork.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 99076dbe27d8..e80e889543f3 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -44,6 +44,7 @@
>   #include <linux/fs.h>
>   #include <linux/mm.h>
>   #include <linux/mm_inline.h>
> +#include <linux/memblock.h>
>   #include <linux/nsproxy.h>
>   #include <linux/capability.h>
>   #include <linux/cpu.h>
> @@ -999,7 +1000,7 @@ void __init __weak arch_task_cache_init(void) { }
>   static void set_max_threads(unsigned int max_threads_suggested)
>   {
>   	u64 threads;
> -	unsigned long nr_pages = totalram_pages();
> +	unsigned long nr_pages = memblock_estimated_nr_pages();

Stale name

-- 
Cheers,

David / dhildenb

[PATCH v4 3/3] kernel/fork.c: put set_max_threads()/task_struct_whitelist() in __init section

[Wei Yang]

Function set_max_threads()/task_struct_whitelist() is only used by
fork_init() during bootup.

Let's add __init tag to them.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Suggested-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/fork.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index e80e889543f3..da0b4545d380 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -997,7 +997,7 @@ void __init __weak arch_task_cache_init(void) { }
 /*
  * set_max_threads
  */
-static void set_max_threads(unsigned int max_threads_suggested)
+static void __init set_max_threads(unsigned int max_threads_suggested)
 {
 	u64 threads;
 	unsigned long nr_pages = memblock_estimated_nr_pages();
@@ -1023,7 +1023,7 @@ static void set_max_threads(unsigned int max_threads_suggested)
 int arch_task_struct_size __read_mostly;
 #endif
 
-static void task_struct_whitelist(unsigned long *offset, unsigned long *size)
+static void __init task_struct_whitelist(unsigned long *offset, unsigned long *size)
 {
 	/* Fetch thread_struct whitelist for the architecture. */
 	arch_thread_struct_whitelist(offset, size);
-- 
2.34.1

Re: [PATCH v4 1/3] mm/memblock: introduce a new helper memblock_estimated_nr_free_pages()

[David Hildenbrand]

On 08.07.24 03:00, Wei Yang wrote:
> During bootup, system may need the number of free pages in the whole system
> to do some calculation before all pages are freed to buddy system. Usually
> this number is get from totalram_pages(). Since we plan to move the free
> pages accounting in __free_pages_core(), this value may not represent
> total free pages at the early stage, especially when
> CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled.
> 
> Instead of using raw memblock api, let's introduce a new helper for user
> to get the estimated number of free pages from memblock point of view.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> CC: David Hildenbrand <david@redhat.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> 
> ---
> v4: adjust comment per david's suggestion
> ---
>   include/linux/memblock.h |  1 +
>   mm/memblock.c            | 20 ++++++++++++++++++++
>   2 files changed, 21 insertions(+)
> 
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 40c62aca36ec..7d1c32b3dc12 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -486,6 +486,7 @@ static inline __init_memblock bool memblock_bottom_up(void)
>   
>   phys_addr_t memblock_phys_mem_size(void);
>   phys_addr_t memblock_reserved_size(void);
> +unsigned long memblock_estimated_nr_pages(void);

^ stale name

>   phys_addr_t memblock_start_of_DRAM(void);
>   phys_addr_t memblock_end_of_DRAM(void);
>   void memblock_enforce_memory_limit(phys_addr_t memory_limit);
> diff --git a/mm/memblock.c b/mm/memblock.c
> index e81fb68f7f88..26162902789f 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1729,6 +1729,26 @@ phys_addr_t __init_memblock memblock_reserved_size(void)
>   	return memblock.reserved.total_size;
>   }
>   
> +/**
> + * memblock_estimated_nr_free_pages - return estimated number of free pages
> + * from memblock point of view
> + *
> + * During bootup, subsystems might need a rough estimate of the number of free
> + * pages in the whole system, before precise numbers are available from the
> + * buddy. Especially with CONFIG_DEFERRED_STRUCT_PAGE_INIT, the numbers
> + * obtained from the buddy might be very imprecise during bootup.
> + *
> + * While we can get the estimated number of free pages from memblock, which is
> + * good enough for estimation.

I'd drop that sentence.


Apart form that LGTM

-- 
Cheers,

David / dhildenb

Re: [PATCH v4 1/3] mm/memblock: introduce a new helper memblock_estimated_nr_free_pages()

[Wei Yang]

On Mon, Jul 08, 2024 at 09:56:21AM +0200, David Hildenbrand wrote:
>On 08.07.24 03:00, Wei Yang wrote:
>> During bootup, system may need the number of free pages in the whole system
>> to do some calculation before all pages are freed to buddy system. Usually
>> this number is get from totalram_pages(). Since we plan to move the free
>> pages accounting in __free_pages_core(), this value may not represent
>> total free pages at the early stage, especially when
>> CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled.
>> 
>> Instead of using raw memblock api, let's introduce a new helper for user
>> to get the estimated number of free pages from memblock point of view.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> CC: David Hildenbrand <david@redhat.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> 
>> ---
>> v4: adjust comment per david's suggestion
>> ---
>>   include/linux/memblock.h |  1 +
>>   mm/memblock.c            | 20 ++++++++++++++++++++
>>   2 files changed, 21 insertions(+)
>> 
>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
>> index 40c62aca36ec..7d1c32b3dc12 100644
>> --- a/include/linux/memblock.h
>> +++ b/include/linux/memblock.h
>> @@ -486,6 +486,7 @@ static inline __init_memblock bool memblock_bottom_up(void)
>>   phys_addr_t memblock_phys_mem_size(void);
>>   phys_addr_t memblock_reserved_size(void);
>> +unsigned long memblock_estimated_nr_pages(void);
>
>^ stale name

My bad.

>
>>   phys_addr_t memblock_start_of_DRAM(void);
>>   phys_addr_t memblock_end_of_DRAM(void);
>>   void memblock_enforce_memory_limit(phys_addr_t memory_limit);
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index e81fb68f7f88..26162902789f 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -1729,6 +1729,26 @@ phys_addr_t __init_memblock memblock_reserved_size(void)
>>   	return memblock.reserved.total_size;
>>   }
>> +/**
>> + * memblock_estimated_nr_free_pages - return estimated number of free pages
>> + * from memblock point of view
>> + *
>> + * During bootup, subsystems might need a rough estimate of the number of free
>> + * pages in the whole system, before precise numbers are available from the
>> + * buddy. Especially with CONFIG_DEFERRED_STRUCT_PAGE_INIT, the numbers
>> + * obtained from the buddy might be very imprecise during bootup.
>> + *
>> + * While we can get the estimated number of free pages from memblock, which is
>> + * good enough for estimation.
>
>I'd drop that sentence.
>

Will update it.

>
>Apart form that LGTM
>
>-- 
>Cheers,
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me