diff mbox series

[v3,1/3] mm/memblock: introduce a new helper memblock_estimated_nr_pages()

Message ID 20240703005151.28712-1-richard.weiyang@gmail.com (mailing list archive)
State New
Headers show
Series [v3,1/3] mm/memblock: introduce a new helper memblock_estimated_nr_pages() | expand

Commit Message

Wei Yang July 3, 2024, 12:51 a.m. UTC
Instead of using raw memblock api, we wrap a new helper for user.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 include/linux/memblock.h |  1 +
 mm/memblock.c            | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

Comments

Mike Rapoport July 5, 2024, 9:09 a.m. UTC | #1
On Wed, Jul 03, 2024 at 12:51:49AM +0000, Wei Yang wrote:
> Instead of using raw memblock api, we wrap a new helper for user.

The changelog should be more elaborate and explain why this function is
needed.
 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---
>  include/linux/memblock.h |  1 +
>  mm/memblock.c            | 19 +++++++++++++++++++
>  2 files changed, 20 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..c1f1aac0459f 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1729,6 +1729,25 @@ phys_addr_t __init_memblock memblock_reserved_size(void)
>  	return memblock.reserved.total_size;
>  }
>  
> +/**
> + * memblock_estimated_nr_pages - return number of pages from memblock point of
> + * view

This function returns the estimate for free pages, not the number of pages
in RAM.

How about memblock_estimated_nr_free_pages()?

> + * some calculation before all pages are freed to buddy system, especially
> + * when CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled.

I'm failing to parse this sentence. The return value here won't depend on
CONFIG_DEFERRED_STRUCT_PAGE_INIT.

> + *
> + * At this point, we can get this information from memblock. Since the system
> + * state is not settle down and address alignment, the value is an estimation.
> + *
> + * Return:
> + * An estimated number of pages from memblock point of view.

                            ^ free

> + */
> +unsigned long __init memblock_estimated_nr_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
>
David Hildenbrand July 5, 2024, 10:16 a.m. UTC | #2
On 05.07.24 11:09, Mike Rapoport wrote:
> On Wed, Jul 03, 2024 at 12:51:49AM +0000, Wei Yang wrote:
>> Instead of using raw memblock api, we wrap a new helper for user.
> 
> The changelog should be more elaborate and explain why this function is
> needed.
>   
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> ---
>>   include/linux/memblock.h |  1 +
>>   mm/memblock.c            | 19 +++++++++++++++++++
>>   2 files changed, 20 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..c1f1aac0459f 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -1729,6 +1729,25 @@ phys_addr_t __init_memblock memblock_reserved_size(void)
>>   	return memblock.reserved.total_size;
>>   }
>>   
>> +/**
>> + * memblock_estimated_nr_pages - return number of pages from memblock point of
>> + * view
> 
> This function returns the estimate for free pages, not the number of pages
> in RAM.
> 
> How about memblock_estimated_nr_free_pages()?

I was just about to comment exactly that. Agreed!
Wei Yang July 6, 2024, 1:28 a.m. UTC | #3
On Fri, Jul 05, 2024 at 12:09:48PM +0300, Mike Rapoport wrote:
>On Wed, Jul 03, 2024 at 12:51:49AM +0000, Wei Yang wrote:
>> Instead of using raw memblock api, we wrap a new helper for user.
>
>The changelog should be more elaborate and explain why this function is
>needed.
> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> ---
>>  include/linux/memblock.h |  1 +
>>  mm/memblock.c            | 19 +++++++++++++++++++
>>  2 files changed, 20 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..c1f1aac0459f 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -1729,6 +1729,25 @@ phys_addr_t __init_memblock memblock_reserved_size(void)
>>  	return memblock.reserved.total_size;
>>  }
>>  
>> +/**
>> + * memblock_estimated_nr_pages - return number of pages from memblock point of
>> + * view
>
>This function returns the estimate for free pages, not the number of pages
>in RAM.
>
>How about memblock_estimated_nr_free_pages()?
>
>> + * some calculation before all pages are freed to buddy system, especially
>> + * when CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled.
>
>I'm failing to parse this sentence. The return value here won't depend on
>CONFIG_DEFERRED_STRUCT_PAGE_INIT.
>
>> + *
>> + * At this point, we can get this information from memblock. Since the system
>> + * state is not settle down and address alignment, the value is an estimation.
>> + *
>> + * Return:
>> + * An estimated number of pages from memblock point of view.
>
>                            ^ free
>
>> + */
>> +unsigned long __init memblock_estimated_nr_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
>> 

Thanks for review. Is this one looks better?

Subject: [PATCH] mm/memblock: introduce a new helper memblock_estimated_nr_free_pages()

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>
---
 include/linux/memblock.h |  1 +
 mm/memblock.c            | 22 ++++++++++++++++++++++
 2 files changed, 23 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..00decc42e02b 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1729,6 +1729,28 @@ 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, 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.
+ *
+ * While we can get the estimated number of free pages from memblock, which is
+ * good enough for the calculation.
+ *
+ * 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)
 {
David Hildenbrand July 7, 2024, 8:17 a.m. UTC | #4
On 06.07.24 03:28, Wei Yang wrote:
> On Fri, Jul 05, 2024 at 12:09:48PM +0300, Mike Rapoport wrote:
>> On Wed, Jul 03, 2024 at 12:51:49AM +0000, Wei Yang wrote:
>>> Instead of using raw memblock api, we wrap a new helper for user.
>>
>> The changelog should be more elaborate and explain why this function is
>> needed.
>>
>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>> ---
>>>   include/linux/memblock.h |  1 +
>>>   mm/memblock.c            | 19 +++++++++++++++++++
>>>   2 files changed, 20 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..c1f1aac0459f 100644
>>> --- a/mm/memblock.c
>>> +++ b/mm/memblock.c
>>> @@ -1729,6 +1729,25 @@ phys_addr_t __init_memblock memblock_reserved_size(void)
>>>   	return memblock.reserved.total_size;
>>>   }
>>>   
>>> +/**
>>> + * memblock_estimated_nr_pages - return number of pages from memblock point of
>>> + * view
>>
>> This function returns the estimate for free pages, not the number of pages
>> in RAM.
>>
>> How about memblock_estimated_nr_free_pages()?
>>
>>> + * some calculation before all pages are freed to buddy system, especially
>>> + * when CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled.
>>
>> I'm failing to parse this sentence. The return value here won't depend on
>> CONFIG_DEFERRED_STRUCT_PAGE_INIT.
>>
>>> + *
>>> + * At this point, we can get this information from memblock. Since the system
>>> + * state is not settle down and address alignment, the value is an estimation.
>>> + *
>>> + * Return:
>>> + * An estimated number of pages from memblock point of view.
>>
>>                             ^ free
>>
>>> + */
>>> +unsigned long __init memblock_estimated_nr_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
>>>
> 
> Thanks for review. Is this one looks better?
> 
> Subject: [PATCH] mm/memblock: introduce a new helper memblock_estimated_nr_free_pages()
> 
> 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>
> ---
>   include/linux/memblock.h |  1 +
>   mm/memblock.c            | 22 ++++++++++++++++++++++
>   2 files changed, 23 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..00decc42e02b 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1729,6 +1729,28 @@ 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, 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.

These historical details should be dropped. "Since we plan ..." will 
easily get outdated soon.

* 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.

?

Reviewed-by: David Hildenbrand <david@redhat.com>
Wei Yang July 8, 2024, 12:36 a.m. UTC | #5
On Sun, Jul 07, 2024 at 09:03:05PM +0200, Oleg Nesterov wrote:
>As I have already said, I can't review this patch.
>
>But I am curious, why set_max_threads() is the only user of the new helper?
>Say, should files_maxfiles_init() use it too or not? Perhaps the changelog
>can explain this?
>

files_maxfiles_init() is called in two places:

  * vfs_caches_init()
  * page_alloc_init_late()

And in page_alloc_init_late(), it is called after deferred_init_memmap(). This
means the totalram_pages() here return the real free pages at this point. So
it is not necessary to touch it now.

>Thanks,
>
>Oleg.
>
>On 07/07, David Hildenbrand wrote:
>>
>> On 06.07.24 03:28, Wei Yang wrote:
>> >On Fri, Jul 05, 2024 at 12:09:48PM +0300, Mike Rapoport wrote:
>> >>On Wed, Jul 03, 2024 at 12:51:49AM +0000, Wei Yang wrote:
>> >>>Instead of using raw memblock api, we wrap a new helper for user.
>> >>
>> >>The changelog should be more elaborate and explain why this function is
>> >>needed.
>> >>
>> >>>Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> >>>---
>> >>>  include/linux/memblock.h |  1 +
>> >>>  mm/memblock.c            | 19 +++++++++++++++++++
>> >>>  2 files changed, 20 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..c1f1aac0459f 100644
>> >>>--- a/mm/memblock.c
>> >>>+++ b/mm/memblock.c
>> >>>@@ -1729,6 +1729,25 @@ phys_addr_t __init_memblock memblock_reserved_size(void)
>> >>>  	return memblock.reserved.total_size;
>> >>>  }
>> >>>+/**
>> >>>+ * memblock_estimated_nr_pages - return number of pages from memblock point of
>> >>>+ * view
>> >>
>> >>This function returns the estimate for free pages, not the number of pages
>> >>in RAM.
>> >>
>> >>How about memblock_estimated_nr_free_pages()?
>> >>
>> >>>+ * some calculation before all pages are freed to buddy system, especially
>> >>>+ * when CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled.
>> >>
>> >>I'm failing to parse this sentence. The return value here won't depend on
>> >>CONFIG_DEFERRED_STRUCT_PAGE_INIT.
>> >>
>> >>>+ *
>> >>>+ * At this point, we can get this information from memblock. Since the system
>> >>>+ * state is not settle down and address alignment, the value is an estimation.
>> >>>+ *
>> >>>+ * Return:
>> >>>+ * An estimated number of pages from memblock point of view.
>> >>
>> >>                            ^ free
>> >>
>> >>>+ */
>> >>>+unsigned long __init memblock_estimated_nr_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
>> >>>
>> >
>> >Thanks for review. Is this one looks better?
>> >
>> >Subject: [PATCH] mm/memblock: introduce a new helper memblock_estimated_nr_free_pages()
>> >
>> >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>
>> >---
>> >  include/linux/memblock.h |  1 +
>> >  mm/memblock.c            | 22 ++++++++++++++++++++++
>> >  2 files changed, 23 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..00decc42e02b 100644
>> >--- a/mm/memblock.c
>> >+++ b/mm/memblock.c
>> >@@ -1729,6 +1729,28 @@ 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, 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.
>> 
>> These historical details should be dropped. "Since we plan ..." will easily
>> get outdated soon.
>> 
>> * 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.
>> 
>> ?
>> 
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> 
>> -- 
>> Cheers,
>> 
>> David / dhildenb
>>
Wei Yang July 8, 2024, 12:39 a.m. UTC | #6
On Sun, Jul 07, 2024 at 10:17:27AM +0200, David Hildenbrand wrote:
>On 06.07.24 03:28, Wei Yang wrote:
>> On Fri, Jul 05, 2024 at 12:09:48PM +0300, Mike Rapoport wrote:
>> > On Wed, Jul 03, 2024 at 12:51:49AM +0000, Wei Yang wrote:
>> > > Instead of using raw memblock api, we wrap a new helper for user.
>> > 
>> > The changelog should be more elaborate and explain why this function is
>> > needed.
>> > 
>> > > Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> > > ---
>> > >   include/linux/memblock.h |  1 +
>> > >   mm/memblock.c            | 19 +++++++++++++++++++
>> > >   2 files changed, 20 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..c1f1aac0459f 100644
>> > > --- a/mm/memblock.c
>> > > +++ b/mm/memblock.c
>> > > @@ -1729,6 +1729,25 @@ phys_addr_t __init_memblock memblock_reserved_size(void)
>> > >   	return memblock.reserved.total_size;
>> > >   }
>> > > +/**
>> > > + * memblock_estimated_nr_pages - return number of pages from memblock point of
>> > > + * view
>> > 
>> > This function returns the estimate for free pages, not the number of pages
>> > in RAM.
>> > 
>> > How about memblock_estimated_nr_free_pages()?
>> > 
>> > > + * some calculation before all pages are freed to buddy system, especially
>> > > + * when CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled.
>> > 
>> > I'm failing to parse this sentence. The return value here won't depend on
>> > CONFIG_DEFERRED_STRUCT_PAGE_INIT.
>> > 
>> > > + *
>> > > + * At this point, we can get this information from memblock. Since the system
>> > > + * state is not settle down and address alignment, the value is an estimation.
>> > > + *
>> > > + * Return:
>> > > + * An estimated number of pages from memblock point of view.
>> > 
>> >                             ^ free
>> > 
>> > > + */
>> > > +unsigned long __init memblock_estimated_nr_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
>> > > 
>> 
>> Thanks for review. Is this one looks better?
>> 
>> Subject: [PATCH] mm/memblock: introduce a new helper memblock_estimated_nr_free_pages()
>> 
>> 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>
>> ---
>>   include/linux/memblock.h |  1 +
>>   mm/memblock.c            | 22 ++++++++++++++++++++++
>>   2 files changed, 23 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..00decc42e02b 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -1729,6 +1729,28 @@ 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, 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.
>
>These historical details should be dropped. "Since we plan ..." will easily
>get outdated soon.
>
>* 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.
>

Pretty better, thanks :-)

>?
>
>Reviewed-by: David Hildenbrand <david@redhat.com>
>
>-- 
>Cheers,
>
>David / dhildenb
diff mbox series

Patch

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..c1f1aac0459f 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1729,6 +1729,25 @@  phys_addr_t __init_memblock memblock_reserved_size(void)
 	return memblock.reserved.total_size;
 }
 
+/**
+ * memblock_estimated_nr_pages - return number of pages from memblock point of
+ * view
+ *
+ * During bootup, system may need number of pages in the whole system to do
+ * some calculation before all pages are freed to buddy system, especially
+ * when CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled.
+ *
+ * At this point, we can get this information from memblock. Since the system
+ * state is not settle down and address alignment, the value is an estimation.
+ *
+ * Return:
+ * An estimated number of pages from memblock point of view.
+ */
+unsigned long __init memblock_estimated_nr_pages(void)
+{
+	return PHYS_PFN(memblock_phys_mem_size() - memblock_reserved_size());
+}
+
 /* lowest address */
 phys_addr_t __init_memblock memblock_start_of_DRAM(void)
 {