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

[PATCH v6 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>

---
v6: fix memblock test
v5: cleanup the stale name
v4: adjust comment per david's suggestion
---
 include/linux/memblock.h  |  1 +
 mm/memblock.c             | 17 +++++++++++++++++
 tools/include/linux/pfn.h |  1 +
 3 files changed, 19 insertions(+)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index fc4d75c6cec3..673d5cae7c81 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -467,6 +467,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_free_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 3b9dc2d89b8a..213057603b65 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1731,6 +1731,23 @@ 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.
+ *
+ * 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)
 {
diff --git a/tools/include/linux/pfn.h b/tools/include/linux/pfn.h
index 7512a58189eb..f77a30d70152 100644
--- a/tools/include/linux/pfn.h
+++ b/tools/include/linux/pfn.h
@@ -7,4 +7,5 @@
 #define PFN_UP(x)	(((x) + PAGE_SIZE - 1) >> PAGE_SHIFT)
 #define PFN_DOWN(x)	((x) >> PAGE_SHIFT)
 #define PFN_PHYS(x)	((phys_addr_t)(x) << PAGE_SHIFT)
+#define PHYS_PFN(x)	((unsigned long)((x) >> PAGE_SHIFT))
 #endif
-- 
2.34.1

[PATCH v6 2/3] kernel/fork.c: get estimated free pages by memblock api

[Wei Yang]

Instead of getting estimated free pages from memblock directly, we have
introduced an API, memblock_estimated_nr_free_pages(), which is more
friendly for users.

Just replace it with new API, no functional change.

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index cc760491f201..d99f148d818b 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -997,7 +997,7 @@ void __init __weak arch_task_cache_init(void) { }
 static void __init set_max_threads(unsigned int max_threads_suggested)
 {
 	u64 threads;
-	unsigned long nr_pages = PHYS_PFN(memblock_phys_mem_size() - memblock_reserved_size());
+	unsigned long nr_pages = memblock_estimated_nr_free_pages();
 
 	/*
 	 * The number of threads shall be limited such that the thread
-- 
2.34.1

Re: [PATCH v6 2/3] kernel/fork.c: get estimated free pages by memblock api

[David Hildenbrand]

On 08.08.24 02:14, Wei Yang wrote:
> Instead of getting estimated free pages from memblock directly, we have
> introduced an API, memblock_estimated_nr_free_pages(), which is more
> friendly for users.
> 
> Just replace it with new API, no functional change.
> 
> 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>
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

[PATCH v6 3/3] s390/mm: get estimated free pages by memblock api

[Wei Yang]

Instead of getting estimated free pages from memblock directly, we have
introduced an API, memblock_estimated_nr_free_pages(), which is more
friendly for users.

Just replace it with new API, no functional change.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
CC: Mike Rapoport (IBM) <rppt@kernel.org>
CC: David Hildenbrand <david@redhat.com>
---
 arch/s390/mm/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index e3d258f9e726..651344206294 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -62,7 +62,7 @@ EXPORT_SYMBOL(zero_page_mask);
 
 static void __init setup_zero_pages(void)
 {
-	unsigned long total_pages = PHYS_PFN(memblock_phys_mem_size() - memblock_reserved_size());
+	unsigned long total_pages = memblock_estimated_nr_free_pages();
 	unsigned int order;
 	struct page *page;
 	int i;
-- 
2.34.1

Re: [PATCH v6 3/3] s390/mm: get estimated free pages by memblock api

[David Hildenbrand]

On 08.08.24 02:14, Wei Yang wrote:
> Instead of getting estimated free pages from memblock directly, we have
> introduced an API, memblock_estimated_nr_free_pages(), which is more
> friendly for users.
> 
> Just replace it with new API, no functional change.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> CC: Mike Rapoport (IBM) <rppt@kernel.org>
> CC: David Hildenbrand <david@redhat.com>
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

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

[Alexander Gordeev]

On Thu, Aug 08, 2024 at 12:14:13AM +0000, Wei Yang wrote:

Hi Wei,

...
> + * 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());
> +}

This could possibly be short on up to two pages due to lack of alignment.
The current uses are okay, but since you make it generic it probably matters.

Also, the returned value is not an estimation. Meaning the function name
is rather unfortunate AFAICT.

> +#define PHYS_PFN(x)	((unsigned long)((x) >> PAGE_SHIFT))

Thanks!

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

[Wei Yang]

On Thu, Aug 08, 2024 at 12:06:38PM +0200, Alexander Gordeev wrote:
>On Thu, Aug 08, 2024 at 12:14:13AM +0000, Wei Yang wrote:
>
>Hi Wei,
>
>...
>> + * 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());
>> +}
>
>This could possibly be short on up to two pages due to lack of alignment.
>The current uses are okay, but since you make it generic it probably matters.
>

I don't follow, would you mind giving more detail?

>Also, the returned value is not an estimation. Meaning the function name
>is rather unfortunate AFAICT.

From my point of view, this is an estimation for two reasons:

* value from memblock_xxx is not page size aligned
* reserved memory maybe released during boot stage

It is not that easy to get the exact number of free pages here. Do I miss
something?

>
>> +#define PHYS_PFN(x)	((unsigned long)((x) >> PAGE_SHIFT))
>
>Thanks!

-- 
Wei Yang
Help you, Help me

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

[Alexander Gordeev]

On Thu, Aug 08, 2024 at 02:24:05PM +0000, Wei Yang wrote:
> >> + * 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());
> >> +}
> >
> >This could possibly be short on up to two pages due to lack of alignment.
> >The current uses are okay, but since you make it generic it probably matters.
> >
> 
> I don't follow, would you mind giving more detail?

memblock_estimated_nr_free_pages() returns number of pages, not bytes.
Yet, both memblock_phys_mem_size() and memblock_reserved_size() return
a value which is not aligned on PAGE_SIZE. Therefore, the result of
PHYS_PFN() applied to the difference between the two functions might
be short on one (two?) page(s).

> >Also, the returned value is not an estimation. Meaning the function name
> >is rather unfortunate AFAICT.
> 
> From my point of view, this is an estimation for two reasons:
> 
> * value from memblock_xxx is not page size aligned
> * reserved memory maybe released during boot stage
> 
> It is not that easy to get the exact number of free pages here. Do I miss
> something?

No, with this reasoning it makes sense to me.

> -- 
> Wei Yang
> Help you, Help me

Thank you for the clarification!

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

[Mike Rapoport]

From: Mike Rapoport (Microsoft) <rppt@kernel.org>

On Thu, 08 Aug 2024 00:14:13 +0000, 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.
> 
> [...]

Applied to for-next branch of memblock.git tree, thanks!

[1/3] mm/memblock: introduce a new helper memblock_estimated_nr_free_pages()
      commit: d0f8a8973f265f6a276f99d091af99edfb2b87de
[2/3] kernel/fork.c: get estimated free pages by memblock api
      commit: 0910bf0ef85c5404aac94394cb31e076e4eb03f1
[3/3] s390/mm: get estimated free pages by memblock api
      commit: cb088e38aab4c7e9ce711c18c66e851c8f4227bb

tree: https://git.kernel.org/pub/scm/linux/kernel/git/rppt/memblock
branch: for-next

--
Sincerely yours,
Mike.