[PATCH 00/16] A few cleanup patches for mm

[PATCH 00/16] A few cleanup patches for mm

[Miaohe Lin]

Hi everyone,
This series contains a few cleanup patches to remove the obsolete
comments and functions, use helper macro to improve readability and
so on. More details can be found in the respective changelogs.
Thanks!

Miaohe Lin (16):
  mm/page_alloc: ensure kswapd doesn't accidentally go to sleep
  mm/page_alloc: make zone_pcp_update() static
  mm: remove obsolete macro NR_PCP_ORDER_MASK
  mm/page_alloc: remove obsolete comment in zone_statistics()
  mm/page_alloc: add __init annotations to
    init_mem_debugging_and_hardening()
  mm/page_alloc: fix freeing static percpu memory
  mm: remove obsolete pgdat_is_empty()
  mm/page_alloc: add missing is_migrate_isolate() check in
    set_page_guard()
  mm/page_alloc: use local variable zone_idx directly
  mm, memory_hotplug: remove obsolete generic_free_nodedata()
  mm/page_alloc: make boot_nodestats static
  mm/page_alloc: use helper macro SZ_1{K,M}
  mm/page_alloc: init local variable buddy_pfn
  mm/page_alloc: use costly_order in WARN_ON_ONCE_GFP()
  mm/page_alloc: remove obsolete gfpflags_normal_context()
  mm/page_alloc: fix obsolete comment in deferred_pfn_valid()

 include/linux/gfp.h            | 23 -------------
 include/linux/memory_hotplug.h |  8 -----
 include/linux/mm.h             |  2 +-
 include/linux/mmzone.h         |  6 ----
 mm/internal.h                  |  3 --
 mm/page_alloc.c                | 60 ++++++++++++++++------------------
 6 files changed, 30 insertions(+), 72 deletions(-)

-- 
2.23.0

[PATCH 01/16] mm/page_alloc: ensure kswapd doesn't accidentally go to sleep

[Miaohe Lin]

If ALLOC_KSWAPD is set, wake_all_kswapds() will be called to ensure
kswapd doesn't accidentally go to sleep. But when reserve_flags is
set, alloc_flags will be overwritten and ALLOC_KSWAPD is thus lost.
Preserve the ALLOC_KSWAPD flag in alloc_flags to ensure kswapd won't
go to sleep accidentally.

Fixes: 0a79cdad5eb2 ("mm: use alloc_flags to record if kswapd can wake")
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/page_alloc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ba5c9402a5cb..4b97a03fa2dd 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5147,7 +5147,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 
 	reserve_flags = __gfp_pfmemalloc_flags(gfp_mask);
 	if (reserve_flags)
-		alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, reserve_flags);
+		alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, reserve_flags) |
+					  (alloc_flags & ALLOC_KSWAPD);
 
 	/*
 	 * Reset the nodemask and zonelist iterators if memory policies can be
-- 
2.23.0

Re: [PATCH 01/16] mm/page_alloc: ensure kswapd doesn't accidentally go to sleep

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> If ALLOC_KSWAPD is set, wake_all_kswapds() will be called to ensure
> kswapd doesn't accidentally go to sleep. But when reserve_flags is
> set, alloc_flags will be overwritten and ALLOC_KSWAPD is thus lost.
> Preserve the ALLOC_KSWAPD flag in alloc_flags to ensure kswapd won't
> go to sleep accidentally.
> 
> Fixes: 0a79cdad5eb2 ("mm: use alloc_flags to record if kswapd can wake")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/page_alloc.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index ba5c9402a5cb..4b97a03fa2dd 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5147,7 +5147,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>   
>   	reserve_flags = __gfp_pfmemalloc_flags(gfp_mask);
>   	if (reserve_flags)
> -		alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, reserve_flags);
> +		alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, reserve_flags) |
> +					  (alloc_flags & ALLOC_KSWAPD);
>   
>   	/*
>   	 * Reset the nodemask and zonelist iterators if memory policies can be

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 01/16] mm/page_alloc: ensure kswapd doesn't accidentally go to sleep

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> If ALLOC_KSWAPD is set, wake_all_kswapds() will be called to ensure
> kswapd doesn't accidentally go to sleep. But when reserve_flags is
> set, alloc_flags will be overwritten and ALLOC_KSWAPD is thus lost.
> Preserve the ALLOC_KSWAPD flag in alloc_flags to ensure kswapd won't
> go to sleep accidentally.

Currently wake_all_kswapds() gets skipped subsequently if ALLOC_KSWAPD
is lost, but this only happens when the 'retry:' loops is taken ?

> 
> Fixes: 0a79cdad5eb2 ("mm: use alloc_flags to record if kswapd can wake")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/page_alloc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index ba5c9402a5cb..4b97a03fa2dd 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5147,7 +5147,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  
>  	reserve_flags = __gfp_pfmemalloc_flags(gfp_mask);
>  	if (reserve_flags)
> -		alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, reserve_flags);
> +		alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, reserve_flags) |
> +					  (alloc_flags & ALLOC_KSWAPD);
>  
>  	/*
>  	 * Reset the nodemask and zonelist iterators if memory policies can be

Re: [PATCH 01/16] mm/page_alloc: ensure kswapd doesn't accidentally go to sleep

[Matthew Wilcox]

On Tue, Sep 13, 2022 at 12:32:50PM +0530, Anshuman Khandual wrote:
> 
> 
> On 9/9/22 14:54, Miaohe Lin wrote:
> > If ALLOC_KSWAPD is set, wake_all_kswapds() will be called to ensure
> > kswapd doesn't accidentally go to sleep. But when reserve_flags is
> > set, alloc_flags will be overwritten and ALLOC_KSWAPD is thus lost.
> > Preserve the ALLOC_KSWAPD flag in alloc_flags to ensure kswapd won't
> > go to sleep accidentally.
> 
> Currently wake_all_kswapds() gets skipped subsequently if ALLOC_KSWAPD
> is lost, but this only happens when the 'retry:' loops is taken ?

Right, but see the comment:

        /* Ensure kswapd doesn't accidentally go to sleep as long as we loop */

and that is not currently true.  I think that was an inadvertent change.
Particularly since the changelog for 0a79cdad5eb2 says "No functional
change".

> > 
> > Fixes: 0a79cdad5eb2 ("mm: use alloc_flags to record if kswapd can wake")
> > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> > ---
> >  mm/page_alloc.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index ba5c9402a5cb..4b97a03fa2dd 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -5147,7 +5147,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> >  
> >  	reserve_flags = __gfp_pfmemalloc_flags(gfp_mask);
> >  	if (reserve_flags)
> > -		alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, reserve_flags);
> > +		alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, reserve_flags) |
> > +					  (alloc_flags & ALLOC_KSWAPD);
> >  
> >  	/*
> >  	 * Reset the nodemask and zonelist iterators if memory policies can be
> 

Re: [PATCH 01/16] mm/page_alloc: ensure kswapd doesn't accidentally go to sleep

[Anshuman Khandual]

On 9/13/22 13:28, Matthew Wilcox wrote:
> On Tue, Sep 13, 2022 at 12:32:50PM +0530, Anshuman Khandual wrote:
>>
>> On 9/9/22 14:54, Miaohe Lin wrote:
>>> If ALLOC_KSWAPD is set, wake_all_kswapds() will be called to ensure
>>> kswapd doesn't accidentally go to sleep. But when reserve_flags is
>>> set, alloc_flags will be overwritten and ALLOC_KSWAPD is thus lost.
>>> Preserve the ALLOC_KSWAPD flag in alloc_flags to ensure kswapd won't
>>> go to sleep accidentally.
>> Currently wake_all_kswapds() gets skipped subsequently if ALLOC_KSWAPD
>> is lost, but this only happens when the 'retry:' loops is taken ?
> Right, but see the comment:
> 
>         /* Ensure kswapd doesn't accidentally go to sleep as long as we loop */
> 
> and that is not currently true.  I think that was an inadvertent change.
> Particularly since the changelog for 0a79cdad5eb2 says "No functional
> change".

Got it, thanks for the explanation.

Re: [PATCH 01/16] mm/page_alloc: ensure kswapd doesn't accidentally go to sleep

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:36PM +0800, Miaohe Lin wrote:
> If ALLOC_KSWAPD is set, wake_all_kswapds() will be called to ensure
> kswapd doesn't accidentally go to sleep. But when reserve_flags is
> set, alloc_flags will be overwritten and ALLOC_KSWAPD is thus lost.
> Preserve the ALLOC_KSWAPD flag in alloc_flags to ensure kswapd won't
> go to sleep accidentally.
> 
> Fixes: 0a79cdad5eb2 ("mm: use alloc_flags to record if kswapd can wake")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/page_alloc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index ba5c9402a5cb..4b97a03fa2dd 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5147,7 +5147,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  
>  	reserve_flags = __gfp_pfmemalloc_flags(gfp_mask);
>  	if (reserve_flags)
> -		alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, reserve_flags);
> +		alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, reserve_flags) |
> +					  (alloc_flags & ALLOC_KSWAPD);
>  
>  	/*
>  	 * Reset the nodemask and zonelist iterators if memory policies can be
> -- 
> 2.23.0
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 02/16] mm/page_alloc: make zone_pcp_update() static

[Miaohe Lin]

Since commit b92ca18e8ca5 ("mm/page_alloc: disassociate the pcp->high
from pcp->batch"), zone_pcp_update() is only used in mm/page_alloc.c.
Move zone_pcp_update() up to avoid forward declaration and then make
it static. No functional change intended.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/internal.h   |  1 -
 mm/page_alloc.c | 22 +++++++++++-----------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index 4e5f3d0d5538..2424fcde6538 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -367,7 +367,6 @@ extern int user_min_free_kbytes;
 extern void free_unref_page(struct page *page, unsigned int order);
 extern void free_unref_page_list(struct list_head *list);
 
-extern void zone_pcp_update(struct zone *zone, int cpu_online);
 extern void zone_pcp_reset(struct zone *zone);
 extern void zone_pcp_disable(struct zone *zone);
 extern void zone_pcp_enable(struct zone *zone);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4b97a03fa2dd..2deac2fe3372 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7238,6 +7238,17 @@ void __meminit setup_zone_pageset(struct zone *zone)
 	zone_set_pageset_high_and_batch(zone, 0);
 }
 
+/*
+ * The zone indicated has a new number of managed_pages; batch sizes and percpu
+ * page high values need to be recalculated.
+ */
+static void zone_pcp_update(struct zone *zone, int cpu_online)
+{
+	mutex_lock(&pcp_batch_high_lock);
+	zone_set_pageset_high_and_batch(zone, cpu_online);
+	mutex_unlock(&pcp_batch_high_lock);
+}
+
 /*
  * Allocate per cpu pagesets and initialize them.
  * Before this call only boot pagesets were available.
@@ -9469,17 +9480,6 @@ void free_contig_range(unsigned long pfn, unsigned long nr_pages)
 }
 EXPORT_SYMBOL(free_contig_range);
 
-/*
- * The zone indicated has a new number of managed_pages; batch sizes and percpu
- * page high values need to be recalculated.
- */
-void zone_pcp_update(struct zone *zone, int cpu_online)
-{
-	mutex_lock(&pcp_batch_high_lock);
-	zone_set_pageset_high_and_batch(zone, cpu_online);
-	mutex_unlock(&pcp_batch_high_lock);
-}
-
 /*
  * Effectively disable pcplists for the zone by setting the high limit to 0
  * and draining all cpus. A concurrent page freeing on another CPU that's about
-- 
2.23.0

Re: [PATCH 02/16] mm/page_alloc: make zone_pcp_update() static

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> Since commit b92ca18e8ca5 ("mm/page_alloc: disassociate the pcp->high
> from pcp->batch"), zone_pcp_update() is only used in mm/page_alloc.c.
> Move zone_pcp_update() up to avoid forward declaration and then make
> it static. No functional change intended.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/internal.h   |  1 -
>   mm/page_alloc.c | 22 +++++++++++-----------
>   2 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 4e5f3d0d5538..2424fcde6538 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -367,7 +367,6 @@ extern int user_min_free_kbytes;
>   extern void free_unref_page(struct page *page, unsigned int order);
>   extern void free_unref_page_list(struct list_head *list);
>   
> -extern void zone_pcp_update(struct zone *zone, int cpu_online);
>   extern void zone_pcp_reset(struct zone *zone);
>   extern void zone_pcp_disable(struct zone *zone);
>   extern void zone_pcp_enable(struct zone *zone);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 4b97a03fa2dd..2deac2fe3372 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7238,6 +7238,17 @@ void __meminit setup_zone_pageset(struct zone *zone)
>   	zone_set_pageset_high_and_batch(zone, 0);
>   }
>   
> +/*
> + * The zone indicated has a new number of managed_pages; batch sizes and percpu
> + * page high values need to be recalculated.
> + */
> +static void zone_pcp_update(struct zone *zone, int cpu_online)
> +{
> +	mutex_lock(&pcp_batch_high_lock);
> +	zone_set_pageset_high_and_batch(zone, cpu_online);
> +	mutex_unlock(&pcp_batch_high_lock);
> +}
> +
>   /*
>    * Allocate per cpu pagesets and initialize them.
>    * Before this call only boot pagesets were available.
> @@ -9469,17 +9480,6 @@ void free_contig_range(unsigned long pfn, unsigned long nr_pages)
>   }
>   EXPORT_SYMBOL(free_contig_range);
>   
> -/*
> - * The zone indicated has a new number of managed_pages; batch sizes and percpu
> - * page high values need to be recalculated.
> - */
> -void zone_pcp_update(struct zone *zone, int cpu_online)
> -{
> -	mutex_lock(&pcp_batch_high_lock);
> -	zone_set_pageset_high_and_batch(zone, cpu_online);
> -	mutex_unlock(&pcp_batch_high_lock);
> -}
> -
>   /*
>    * Effectively disable pcplists for the zone by setting the high limit to 0
>    * and draining all cpus. A concurrent page freeing on another CPU that's about

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 02/16] mm/page_alloc: make zone_pcp_update() static

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> Since commit b92ca18e8ca5 ("mm/page_alloc: disassociate the pcp->high
> from pcp->batch"), zone_pcp_update() is only used in mm/page_alloc.c.
> Move zone_pcp_update() up to avoid forward declaration and then make
> it static. No functional change intended.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  mm/internal.h   |  1 -
>  mm/page_alloc.c | 22 +++++++++++-----------
>  2 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 4e5f3d0d5538..2424fcde6538 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -367,7 +367,6 @@ extern int user_min_free_kbytes;
>  extern void free_unref_page(struct page *page, unsigned int order);
>  extern void free_unref_page_list(struct list_head *list);
>  
> -extern void zone_pcp_update(struct zone *zone, int cpu_online);
>  extern void zone_pcp_reset(struct zone *zone);
>  extern void zone_pcp_disable(struct zone *zone);
>  extern void zone_pcp_enable(struct zone *zone);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 4b97a03fa2dd..2deac2fe3372 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7238,6 +7238,17 @@ void __meminit setup_zone_pageset(struct zone *zone)
>  	zone_set_pageset_high_and_batch(zone, 0);
>  }
>  
> +/*
> + * The zone indicated has a new number of managed_pages; batch sizes and percpu
> + * page high values need to be recalculated.
> + */
> +static void zone_pcp_update(struct zone *zone, int cpu_online)
> +{
> +	mutex_lock(&pcp_batch_high_lock);
> +	zone_set_pageset_high_and_batch(zone, cpu_online);
> +	mutex_unlock(&pcp_batch_high_lock);
> +}
> +
>  /*
>   * Allocate per cpu pagesets and initialize them.
>   * Before this call only boot pagesets were available.
> @@ -9469,17 +9480,6 @@ void free_contig_range(unsigned long pfn, unsigned long nr_pages)
>  }
>  EXPORT_SYMBOL(free_contig_range);
>  
> -/*
> - * The zone indicated has a new number of managed_pages; batch sizes and percpu
> - * page high values need to be recalculated.
> - */
> -void zone_pcp_update(struct zone *zone, int cpu_online)
> -{
> -	mutex_lock(&pcp_batch_high_lock);
> -	zone_set_pageset_high_and_batch(zone, cpu_online);
> -	mutex_unlock(&pcp_batch_high_lock);
> -}
> -
>  /*
>   * Effectively disable pcplists for the zone by setting the high limit to 0
>   * and draining all cpus. A concurrent page freeing on another CPU that's about

Re: [PATCH 02/16] mm/page_alloc: make zone_pcp_update() static

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:37PM +0800, Miaohe Lin wrote:
> Since commit b92ca18e8ca5 ("mm/page_alloc: disassociate the pcp->high
> from pcp->batch"), zone_pcp_update() is only used in mm/page_alloc.c.
> Move zone_pcp_update() up to avoid forward declaration and then make
> it static. No functional change intended.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>


-- 
Oscar Salvador
SUSE Labs

[PATCH 03/16] mm: remove obsolete macro NR_PCP_ORDER_MASK

[Miaohe Lin]

Since commit 8b10b465d0e1 ("mm/page_alloc: free pages in a single pass
during bulk free"), it's not used anymore. Remove it.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/mmzone.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index d907a29cc429..6950179a20a2 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -569,7 +569,6 @@ enum zone_watermarks {
  * in the least significant bits.
  */
 #define NR_PCP_ORDER_WIDTH 8
-#define NR_PCP_ORDER_MASK ((1<<NR_PCP_ORDER_WIDTH) - 1)
 
 #define min_wmark_pages(z) (z->_watermark[WMARK_MIN] + z->watermark_boost)
 #define low_wmark_pages(z) (z->_watermark[WMARK_LOW] + z->watermark_boost)
-- 
2.23.0

Re: [PATCH 03/16] mm: remove obsolete macro NR_PCP_ORDER_MASK

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> Since commit 8b10b465d0e1 ("mm/page_alloc: free pages in a single pass
> during bulk free"), it's not used anymore. Remove it.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   include/linux/mmzone.h | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index d907a29cc429..6950179a20a2 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -569,7 +569,6 @@ enum zone_watermarks {
>    * in the least significant bits.
>    */
>   #define NR_PCP_ORDER_WIDTH 8
> -#define NR_PCP_ORDER_MASK ((1<<NR_PCP_ORDER_WIDTH) - 1)
>   
>   #define min_wmark_pages(z) (z->_watermark[WMARK_MIN] + z->watermark_boost)
>   #define low_wmark_pages(z) (z->_watermark[WMARK_LOW] + z->watermark_boost)

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 03/16] mm: remove obsolete macro NR_PCP_ORDER_MASK

[Matthew Wilcox]

On Fri, Sep 09, 2022 at 05:24:38PM +0800, Miaohe Lin wrote:
> Since commit 8b10b465d0e1 ("mm/page_alloc: free pages in a single pass
> during bulk free"), it's not used anymore. Remove it.

Yes, but this patch needs to go further.  The comment is no longer true
(after the patch you mention), so need to also delete these lines:

-/*
- * Shift to encode migratetype and order in the same integer, with order
- * in the least significant bits.
- */
-#define NR_PCP_ORDER_WIDTH 8

and the line:
-                BUILD_BUG_ON(MAX_ORDER >= (1<<NR_PCP_ORDER_WIDTH));

Re: [PATCH 03/16] mm: remove obsolete macro NR_PCP_ORDER_MASK

[Miaohe Lin]

On 2022/9/10 3:37, Matthew Wilcox wrote:
> On Fri, Sep 09, 2022 at 05:24:38PM +0800, Miaohe Lin wrote:
>> Since commit 8b10b465d0e1 ("mm/page_alloc: free pages in a single pass
>> during bulk free"), it's not used anymore. Remove it.
> 
> Yes, but this patch needs to go further.  The comment is no longer true
> (after the patch you mention), so need to also delete these lines:
> 
> -/*
> - * Shift to encode migratetype and order in the same integer, with order
> - * in the least significant bits.
> - */
> -#define NR_PCP_ORDER_WIDTH 8
> 
> and the line:
> -                BUILD_BUG_ON(MAX_ORDER >= (1<<NR_PCP_ORDER_WIDTH));
> 

Agree. Many thanks for pointing this out. Will do it in next version.

Thanks,
Miaohe Lin

> 
> .
> 

Re: [PATCH 03/16] mm: remove obsolete macro NR_PCP_ORDER_MASK

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> Since commit 8b10b465d0e1 ("mm/page_alloc: free pages in a single pass
> during bulk free"), it's not used anymore. Remove it.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

With those additional changes suggested by Matthew earlier

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  include/linux/mmzone.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index d907a29cc429..6950179a20a2 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -569,7 +569,6 @@ enum zone_watermarks {
>   * in the least significant bits.
>   */
>  #define NR_PCP_ORDER_WIDTH 8
> -#define NR_PCP_ORDER_MASK ((1<<NR_PCP_ORDER_WIDTH) - 1)
>  
>  #define min_wmark_pages(z) (z->_watermark[WMARK_MIN] + z->watermark_boost)
>  #define low_wmark_pages(z) (z->_watermark[WMARK_LOW] + z->watermark_boost)

Re: [PATCH 03/16] mm: remove obsolete macro NR_PCP_ORDER_MASK

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:38PM +0800, Miaohe Lin wrote:
> Since commit 8b10b465d0e1 ("mm/page_alloc: free pages in a single pass
> during bulk free"), it's not used anymore. Remove it.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

with the changes from Matthew applied:

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  include/linux/mmzone.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index d907a29cc429..6950179a20a2 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -569,7 +569,6 @@ enum zone_watermarks {
>   * in the least significant bits.
>   */
>  #define NR_PCP_ORDER_WIDTH 8
> -#define NR_PCP_ORDER_MASK ((1<<NR_PCP_ORDER_WIDTH) - 1)
>  
>  #define min_wmark_pages(z) (z->_watermark[WMARK_MIN] + z->watermark_boost)
>  #define low_wmark_pages(z) (z->_watermark[WMARK_LOW] + z->watermark_boost)
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 04/16] mm/page_alloc: remove obsolete comment in zone_statistics()

[Miaohe Lin]

Since commit 43c95bcc51e4 ("mm/page_alloc: reduce duration that IRQs
are disabled for VM counters"), zone_statistics() is not called with
interrupts disabled. Update the corresponding comment.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/page_alloc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2deac2fe3372..c81f06d8a4f5 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3671,8 +3671,6 @@ void __putback_isolated_page(struct page *page, unsigned int order, int mt)
 
 /*
  * Update NUMA hit/miss statistics
- *
- * Must be called with interrupts disabled.
  */
 static inline void zone_statistics(struct zone *preferred_zone, struct zone *z,
 				   long nr_account)
-- 
2.23.0

Re: [PATCH 04/16] mm/page_alloc: remove obsolete comment in zone_statistics()

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> Since commit 43c95bcc51e4 ("mm/page_alloc: reduce duration that IRQs
> are disabled for VM counters"), zone_statistics() is not called with
> interrupts disabled. Update the corresponding comment.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/page_alloc.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2deac2fe3372..c81f06d8a4f5 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3671,8 +3671,6 @@ void __putback_isolated_page(struct page *page, unsigned int order, int mt)
>   
>   /*
>    * Update NUMA hit/miss statistics
> - *
> - * Must be called with interrupts disabled.
>    */
>   static inline void zone_statistics(struct zone *preferred_zone, struct zone *z,
>   				   long nr_account)


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

-- 
Thanks,

David / dhildenb

Re: [PATCH 04/16] mm/page_alloc: remove obsolete comment in zone_statistics()

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> Since commit 43c95bcc51e4 ("mm/page_alloc: reduce duration that IRQs
> are disabled for VM counters"), zone_statistics() is not called with
> interrupts disabled. Update the corresponding comment.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  mm/page_alloc.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2deac2fe3372..c81f06d8a4f5 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3671,8 +3671,6 @@ void __putback_isolated_page(struct page *page, unsigned int order, int mt)
>  
>  /*
>   * Update NUMA hit/miss statistics
> - *
> - * Must be called with interrupts disabled.
>   */
>  static inline void zone_statistics(struct zone *preferred_zone, struct zone *z,
>  				   long nr_account)

Re: [PATCH 04/16] mm/page_alloc: remove obsolete comment in zone_statistics()

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:39PM +0800, Miaohe Lin wrote:
> Since commit 43c95bcc51e4 ("mm/page_alloc: reduce duration that IRQs
> are disabled for VM counters"), zone_statistics() is not called with
> interrupts disabled. Update the corresponding comment.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/page_alloc.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2deac2fe3372..c81f06d8a4f5 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3671,8 +3671,6 @@ void __putback_isolated_page(struct page *page, unsigned int order, int mt)
>  
>  /*
>   * Update NUMA hit/miss statistics
> - *
> - * Must be called with interrupts disabled.
>   */
>  static inline void zone_statistics(struct zone *preferred_zone, struct zone *z,
>  				   long nr_account)
> -- 
> 2.23.0
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 05/16] mm/page_alloc: add __init annotations to init_mem_debugging_and_hardening()

[Miaohe Lin]

It's only called by mm_init(). Add __init annotations to it.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/mm.h | 2 +-
 mm/page_alloc.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 364bcadb4d20..c2277f5aba9e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3093,7 +3093,7 @@ extern int apply_to_existing_page_range(struct mm_struct *mm,
 				   unsigned long address, unsigned long size,
 				   pte_fn_t fn, void *data);
 
-extern void init_mem_debugging_and_hardening(void);
+extern void __init init_mem_debugging_and_hardening(void);
 #ifdef CONFIG_PAGE_POISONING
 extern void __kernel_poison_pages(struct page *page, int numpages);
 extern void __kernel_unpoison_pages(struct page *page, int numpages);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c81f06d8a4f5..3497919f4ef5 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -903,7 +903,7 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
  * order of appearance. So we need to first gather the full picture of what was
  * enabled, and then make decisions.
  */
-void init_mem_debugging_and_hardening(void)
+void __init init_mem_debugging_and_hardening(void)
 {
 	bool page_poisoning_requested = false;
 
-- 
2.23.0

Re: [PATCH 05/16] mm/page_alloc: add __init annotations to init_mem_debugging_and_hardening()

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> It's only called by mm_init(). Add __init annotations to it.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   include/linux/mm.h | 2 +-
>   mm/page_alloc.c    | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 364bcadb4d20..c2277f5aba9e 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3093,7 +3093,7 @@ extern int apply_to_existing_page_range(struct mm_struct *mm,
>   				   unsigned long address, unsigned long size,
>   				   pte_fn_t fn, void *data);
>   
> -extern void init_mem_debugging_and_hardening(void);
> +extern void __init init_mem_debugging_and_hardening(void);
>   #ifdef CONFIG_PAGE_POISONING
>   extern void __kernel_poison_pages(struct page *page, int numpages);
>   extern void __kernel_unpoison_pages(struct page *page, int numpages);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index c81f06d8a4f5..3497919f4ef5 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -903,7 +903,7 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
>    * order of appearance. So we need to first gather the full picture of what was
>    * enabled, and then make decisions.
>    */
> -void init_mem_debugging_and_hardening(void)
> +void __init init_mem_debugging_and_hardening(void)
>   {
>   	bool page_poisoning_requested = false;
>   

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 05/16] mm/page_alloc: add __init annotations to init_mem_debugging_and_hardening()

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> It's only called by mm_init(). Add __init annotations to it.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  include/linux/mm.h | 2 +-
>  mm/page_alloc.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 364bcadb4d20..c2277f5aba9e 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3093,7 +3093,7 @@ extern int apply_to_existing_page_range(struct mm_struct *mm,
>  				   unsigned long address, unsigned long size,
>  				   pte_fn_t fn, void *data);
>  
> -extern void init_mem_debugging_and_hardening(void);
> +extern void __init init_mem_debugging_and_hardening(void);
>  #ifdef CONFIG_PAGE_POISONING
>  extern void __kernel_poison_pages(struct page *page, int numpages);
>  extern void __kernel_unpoison_pages(struct page *page, int numpages);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index c81f06d8a4f5..3497919f4ef5 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -903,7 +903,7 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
>   * order of appearance. So we need to first gather the full picture of what was
>   * enabled, and then make decisions.
>   */
> -void init_mem_debugging_and_hardening(void)
> +void __init init_mem_debugging_and_hardening(void)
>  {
>  	bool page_poisoning_requested = false;
>  

Re: [PATCH 05/16] mm/page_alloc: add __init annotations to init_mem_debugging_and_hardening()

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:40PM +0800, Miaohe Lin wrote:
> It's only called by mm_init(). Add __init annotations to it.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>


-- 
Oscar Salvador
SUSE Labs

[PATCH 06/16] mm/page_alloc: fix freeing static percpu memory

[Miaohe Lin]

The size of struct per_cpu_zonestat can be 0 on !SMP && !NUMA. In that
case, zone->per_cpu_zonestats will always equal to boot_zonestats. But
in zone_pcp_reset(), zone->per_cpu_zonestats is freed via free_percpu()
directly without checking against boot_zonestats first. boot_zonestats
will be released by free_percpu() unexpectedly.

Fixes: 28f836b6777b ("mm/page_alloc: split per cpu page lists and zone stats")
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/page_alloc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3497919f4ef5..a35ef385d906 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -9510,9 +9510,11 @@ void zone_pcp_reset(struct zone *zone)
 			drain_zonestat(zone, pzstats);
 		}
 		free_percpu(zone->per_cpu_pageset);
-		free_percpu(zone->per_cpu_zonestats);
 		zone->per_cpu_pageset = &boot_pageset;
-		zone->per_cpu_zonestats = &boot_zonestats;
+		if (zone->per_cpu_zonestats != &boot_zonestats) {
+			free_percpu(zone->per_cpu_zonestats);
+			zone->per_cpu_zonestats = &boot_zonestats;
+		}
 	}
 }
 
-- 
2.23.0

Re: [PATCH 06/16] mm/page_alloc: fix freeing static percpu memory

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> The size of struct per_cpu_zonestat can be 0 on !SMP && !NUMA. In that
> case, zone->per_cpu_zonestats will always equal to boot_zonestats. But
> in zone_pcp_reset(), zone->per_cpu_zonestats is freed via free_percpu()
> directly without checking against boot_zonestats first. boot_zonestats
> will be released by free_percpu() unexpectedly.
> 
> Fixes: 28f836b6777b ("mm/page_alloc: split per cpu page lists and zone stats")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/page_alloc.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3497919f4ef5..a35ef385d906 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -9510,9 +9510,11 @@ void zone_pcp_reset(struct zone *zone)
>   			drain_zonestat(zone, pzstats);
>   		}
>   		free_percpu(zone->per_cpu_pageset);
> -		free_percpu(zone->per_cpu_zonestats);
>   		zone->per_cpu_pageset = &boot_pageset;
> -		zone->per_cpu_zonestats = &boot_zonestats;
> +		if (zone->per_cpu_zonestats != &boot_zonestats) {
> +			free_percpu(zone->per_cpu_zonestats);
> +			zone->per_cpu_zonestats = &boot_zonestats;
> +		}
>   	}
>   }
>   

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 06/16] mm/page_alloc: fix freeing static percpu memory

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:41PM +0800, Miaohe Lin wrote:
> The size of struct per_cpu_zonestat can be 0 on !SMP && !NUMA. In that
> case, zone->per_cpu_zonestats will always equal to boot_zonestats. But
> in zone_pcp_reset(), zone->per_cpu_zonestats is freed via free_percpu()
> directly without checking against boot_zonestats first. boot_zonestats
> will be released by free_percpu() unexpectedly.
> 
> Fixes: 28f836b6777b ("mm/page_alloc: split per cpu page lists and zone stats")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>


-- 
Oscar Salvador
SUSE Labs

[PATCH 07/16] mm: remove obsolete pgdat_is_empty()

[Miaohe Lin]

There's no caller. Remove it.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/mmzone.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 6950179a20a2..ab17a0e6f26f 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1247,11 +1247,6 @@ static inline unsigned long pgdat_end_pfn(pg_data_t *pgdat)
 	return pgdat->node_start_pfn + pgdat->node_spanned_pages;
 }
 
-static inline bool pgdat_is_empty(pg_data_t *pgdat)
-{
-	return !pgdat->node_start_pfn && !pgdat->node_spanned_pages;
-}
-
 #include <linux/memory_hotplug.h>
 
 void build_all_zonelists(pg_data_t *pgdat);
-- 
2.23.0

Re: [PATCH 07/16] mm: remove obsolete pgdat_is_empty()

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> There's no caller. Remove it.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   include/linux/mmzone.h | 5 -----
>   1 file changed, 5 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 6950179a20a2..ab17a0e6f26f 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1247,11 +1247,6 @@ static inline unsigned long pgdat_end_pfn(pg_data_t *pgdat)
>   	return pgdat->node_start_pfn + pgdat->node_spanned_pages;
>   }
>   
> -static inline bool pgdat_is_empty(pg_data_t *pgdat)
> -{
> -	return !pgdat->node_start_pfn && !pgdat->node_spanned_pages;
> -}
> -
>   #include <linux/memory_hotplug.h>
>   
>   void build_all_zonelists(pg_data_t *pgdat);

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 07/16] mm: remove obsolete pgdat_is_empty()

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> There's no caller. Remove it.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  include/linux/mmzone.h | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 6950179a20a2..ab17a0e6f26f 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1247,11 +1247,6 @@ static inline unsigned long pgdat_end_pfn(pg_data_t *pgdat)
>  	return pgdat->node_start_pfn + pgdat->node_spanned_pages;
>  }
>  
> -static inline bool pgdat_is_empty(pg_data_t *pgdat)
> -{
> -	return !pgdat->node_start_pfn && !pgdat->node_spanned_pages;
> -}
> -
>  #include <linux/memory_hotplug.h>
>  
>  void build_all_zonelists(pg_data_t *pgdat);

Re: [PATCH 07/16] mm: remove obsolete pgdat_is_empty()

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:42PM +0800, Miaohe Lin wrote:
> There's no caller. Remove it.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

 

-- 
Oscar Salvador
SUSE Labs

[PATCH 08/16] mm/page_alloc: add missing is_migrate_isolate() check in set_page_guard()

[Miaohe Lin]

In MIGRATE_ISOLATE case, zone freepage state shouldn't be modified as
caller will take care of it. Add missing is_migrate_isolate() here to
avoid possible unbalanced freepage state.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/page_alloc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a35ef385d906..94baf33da865 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -873,7 +873,8 @@ static inline bool set_page_guard(struct zone *zone, struct page *page,
 	INIT_LIST_HEAD(&page->buddy_list);
 	set_page_private(page, order);
 	/* Guard pages are not available for any usage */
-	__mod_zone_freepage_state(zone, -(1 << order), migratetype);
+	if (!is_migrate_isolate(migratetype))
+		__mod_zone_freepage_state(zone, -(1 << order), migratetype);
 
 	return true;
 }
-- 
2.23.0

Re: [PATCH 08/16] mm/page_alloc: add missing is_migrate_isolate() check in set_page_guard()

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> In MIGRATE_ISOLATE case, zone freepage state shouldn't be modified as
> caller will take care of it. Add missing is_migrate_isolate() here to
> avoid possible unbalanced freepage state.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/page_alloc.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a35ef385d906..94baf33da865 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -873,7 +873,8 @@ static inline bool set_page_guard(struct zone *zone, struct page *page,
>   	INIT_LIST_HEAD(&page->buddy_list);
>   	set_page_private(page, order);
>   	/* Guard pages are not available for any usage */
> -	__mod_zone_freepage_state(zone, -(1 << order), migratetype);
> +	if (!is_migrate_isolate(migratetype))
> +		__mod_zone_freepage_state(zone, -(1 << order), migratetype);
>   
>   	return true;
>   }

Do we have a fixes: tag for this one?

Can it even happen that the pageblock is isolated when we end up in this 
function? IIUC, we'd have an allocation in an isolated pageblock, which 
would be wrong already?

-- 
Thanks,

David / dhildenb

Re: [PATCH 08/16] mm/page_alloc: add missing is_migrate_isolate() check in set_page_guard()

[Miaohe Lin]

On 2022/9/9 19:31, David Hildenbrand wrote:
> On 09.09.22 11:24, Miaohe Lin wrote:
>> In MIGRATE_ISOLATE case, zone freepage state shouldn't be modified as
>> caller will take care of it. Add missing is_migrate_isolate() here to
>> avoid possible unbalanced freepage state.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>   mm/page_alloc.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index a35ef385d906..94baf33da865 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -873,7 +873,8 @@ static inline bool set_page_guard(struct zone *zone, struct page *page,
>>       INIT_LIST_HEAD(&page->buddy_list);
>>       set_page_private(page, order);
>>       /* Guard pages are not available for any usage */
>> -    __mod_zone_freepage_state(zone, -(1 << order), migratetype);
>> +    if (!is_migrate_isolate(migratetype))
>> +        __mod_zone_freepage_state(zone, -(1 << order), migratetype);
>>         return true;
>>   }
> 
> Do we have a fixes: tag for this one?
> 
> Can it even happen that the pageblock is isolated when we end up in this function? IIUC, we'd have an allocation in an isolated pageblock, which would be wrong already?

For "normal" page allocation case, migratetype can't be MIGRATE_ISOLATE. So it's fine. But when called from take_page_off_buddy(), the issue
could be triggered as it breaks the assumption in the set_page_guard (that migratetype can't be MIGRATE_ISOLATE). So the fixes tag might be:

	Fixes: 06be6ff3d2ec ("mm,hwpoison: rework soft offline for free pages")

Or am I miss something?

Many thanks for your review and comment in this series, David. :)

Thanks,
Miaohe Lin

Re: [PATCH 08/16] mm/page_alloc: add missing is_migrate_isolate() check in set_page_guard()

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:43PM +0800, Miaohe Lin wrote:
> In MIGRATE_ISOLATE case, zone freepage state shouldn't be modified as
> caller will take care of it. Add missing is_migrate_isolate() here to
> avoid possible unbalanced freepage state.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Ok, I gave it some thought and I guess it's safe to assume that this would happen if
someone isolates the block, and then we face an MCE failure/soft-offline on a page
within that block.

take_page_off_buddy
 break_down_buddy_pages
  set_page_guard

will trigger __mod_zone_freepage_state(), which already had been triggered back
when the block was isolated.

I think the changelog could grow fatter to better explain the issue.

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/page_alloc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a35ef385d906..94baf33da865 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -873,7 +873,8 @@ static inline bool set_page_guard(struct zone *zone, struct page *page,
>  	INIT_LIST_HEAD(&page->buddy_list);
>  	set_page_private(page, order);
>  	/* Guard pages are not available for any usage */
> -	__mod_zone_freepage_state(zone, -(1 << order), migratetype);
> +	if (!is_migrate_isolate(migratetype))
> +		__mod_zone_freepage_state(zone, -(1 << order), migratetype);
>  
>  	return true;
>  }
> -- 
> 2.23.0
> 

-- 
Oscar Salvador
SUSE Labs

Re: [PATCH 08/16] mm/page_alloc: add missing is_migrate_isolate() check in set_page_guard()

[Miaohe Lin]

On 2022/9/15 15:26, Oscar Salvador wrote:
> On Fri, Sep 09, 2022 at 05:24:43PM +0800, Miaohe Lin wrote:
>> In MIGRATE_ISOLATE case, zone freepage state shouldn't be modified as
>> caller will take care of it. Add missing is_migrate_isolate() here to
>> avoid possible unbalanced freepage state.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> 
> Ok, I gave it some thought and I guess it's safe to assume that this would happen if
> someone isolates the block, and then we face an MCE failure/soft-offline on a page
> within that block.
> 
> take_page_off_buddy
>  break_down_buddy_pages
>   set_page_guard
> 
> will trigger __mod_zone_freepage_state(), which already had been triggered back
> when the block was isolated.
> 
> I think the changelog could grow fatter to better explain the issue.

That will be really helpful. Will do it in v2.

> 
> Reviewed-by: Oscar Salvador <osalvador@suse.de>

Many thanks for your suggestion and review.

Thanks,
Miaohe Lin

> 
>> ---
>>  mm/page_alloc.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index a35ef385d906..94baf33da865 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -873,7 +873,8 @@ static inline bool set_page_guard(struct zone *zone, struct page *page,
>>  	INIT_LIST_HEAD(&page->buddy_list);
>>  	set_page_private(page, order);
>>  	/* Guard pages are not available for any usage */
>> -	__mod_zone_freepage_state(zone, -(1 << order), migratetype);
>> +	if (!is_migrate_isolate(migratetype))
>> +		__mod_zone_freepage_state(zone, -(1 << order), migratetype);
>>  
>>  	return true;
>>  }
>> -- 
>> 2.23.0
>>
> 

[PATCH 09/16] mm/page_alloc: use local variable zone_idx directly

[Miaohe Lin]

Use local variable zone_idx directly since it holds the exact value of
zone_idx(). No functional change intended.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 94baf33da865..3417232afa45 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6878,7 +6878,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
 	unsigned long start = jiffies;
 	int nid = pgdat->node_id;
 
-	if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
+	if (WARN_ON_ONCE(!pgmap || zone_idx != ZONE_DEVICE))
 		return;
 
 	/*
-- 
2.23.0

Re: [PATCH 09/16] mm/page_alloc: use local variable zone_idx directly

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> Use local variable zone_idx directly since it holds the exact value of
> zone_idx(). No functional change intended.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/page_alloc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 94baf33da865..3417232afa45 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6878,7 +6878,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>   	unsigned long start = jiffies;
>   	int nid = pgdat->node_id;
>   
> -	if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
> +	if (WARN_ON_ONCE(!pgmap || zone_idx != ZONE_DEVICE))
>   		return;
>   
>   	/*

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 09/16] mm/page_alloc: use local variable zone_idx directly

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> Use local variable zone_idx directly since it holds the exact value of
> zone_idx(). No functional change intended.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 94baf33da865..3417232afa45 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6878,7 +6878,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  	unsigned long start = jiffies;
>  	int nid = pgdat->node_id;
>  
> -	if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
> +	if (WARN_ON_ONCE(!pgmap || zone_idx != ZONE_DEVICE))
>  		return;
>  
>  	/*

Re: [PATCH 09/16] mm/page_alloc: use local variable zone_idx directly

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:44PM +0800, Miaohe Lin wrote:
> Use local variable zone_idx directly since it holds the exact value of
> zone_idx(). No functional change intended.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

By the way, zone_idx being "int" should be enough?

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 94baf33da865..3417232afa45 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6878,7 +6878,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  	unsigned long start = jiffies;
>  	int nid = pgdat->node_id;
>  
> -	if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
> +	if (WARN_ON_ONCE(!pgmap || zone_idx != ZONE_DEVICE))
>  		return;
>  
>  	/*
> -- 
> 2.23.0
> 

-- 
Oscar Salvador
SUSE Labs

Re: [PATCH 09/16] mm/page_alloc: use local variable zone_idx directly

[Miaohe Lin]

On 2022/9/15 15:36, Oscar Salvador wrote:
> On Fri, Sep 09, 2022 at 05:24:44PM +0800, Miaohe Lin wrote:
>> Use local variable zone_idx directly since it holds the exact value of
>> zone_idx(). No functional change intended.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> 
> By the way, zone_idx being "int" should be enough?

Yes, it should be. But zone_idx is used as "unsigned long" around this function,
e.g. __init_zone_device_page expects zone_idx as "unsigned long", so it should be
better to keep it as is ?

> 
> Reviewed-by: Oscar Salvador <osalvador@suse.de>

Many thanks for your review and comment in this series.

Thanks,
Miaohe Lin


> 
>> ---
>>  mm/page_alloc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 94baf33da865..3417232afa45 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -6878,7 +6878,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>>  	unsigned long start = jiffies;
>>  	int nid = pgdat->node_id;
>>  
>> -	if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
>> +	if (WARN_ON_ONCE(!pgmap || zone_idx != ZONE_DEVICE))
>>  		return;
>>  
>>  	/*
>> -- 
>> 2.23.0
>>
> 

[PATCH 10/16] mm, memory_hotplug: remove obsolete generic_free_nodedata()

[Miaohe Lin]

Commit 390511e1476e ("mm, memory_hotplug: drop arch_free_nodedata")
drops the last caller of generic_free_nodedata(). Remove it too.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/memory_hotplug.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 51052969dbfe..9fcbf5706595 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -43,11 +43,6 @@ extern void arch_refresh_nodedata(int nid, pg_data_t *pgdat);
 ({								\
 	memblock_alloc(sizeof(*pgdat), SMP_CACHE_BYTES);	\
 })
-/*
- * This definition is just for error path in node hotadd.
- * For node hotremove, we have to replace this.
- */
-#define generic_free_nodedata(pgdat)	kfree(pgdat)
 
 extern pg_data_t *node_data[];
 static inline void arch_refresh_nodedata(int nid, pg_data_t *pgdat)
@@ -63,9 +58,6 @@ static inline pg_data_t *generic_alloc_nodedata(int nid)
 	BUG();
 	return NULL;
 }
-static inline void generic_free_nodedata(pg_data_t *pgdat)
-{
-}
 static inline void arch_refresh_nodedata(int nid, pg_data_t *pgdat)
 {
 }
-- 
2.23.0

Re: [PATCH 10/16] mm, memory_hotplug: remove obsolete generic_free_nodedata()

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> Commit 390511e1476e ("mm, memory_hotplug: drop arch_free_nodedata")
> drops the last caller of generic_free_nodedata(). Remove it too.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   include/linux/memory_hotplug.h | 8 --------
>   1 file changed, 8 deletions(-)
> 
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 51052969dbfe..9fcbf5706595 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -43,11 +43,6 @@ extern void arch_refresh_nodedata(int nid, pg_data_t *pgdat);
>   ({								\
>   	memblock_alloc(sizeof(*pgdat), SMP_CACHE_BYTES);	\
>   })
> -/*
> - * This definition is just for error path in node hotadd.
> - * For node hotremove, we have to replace this.
> - */
> -#define generic_free_nodedata(pgdat)	kfree(pgdat)
>   
>   extern pg_data_t *node_data[];
>   static inline void arch_refresh_nodedata(int nid, pg_data_t *pgdat)
> @@ -63,9 +58,6 @@ static inline pg_data_t *generic_alloc_nodedata(int nid)
>   	BUG();
>   	return NULL;
>   }
> -static inline void generic_free_nodedata(pg_data_t *pgdat)
> -{
> -}
>   static inline void arch_refresh_nodedata(int nid, pg_data_t *pgdat)
>   {
>   }


Reviewed-by: David Hildenbrand <david@redhat.com>
-- 
Thanks,

David / dhildenb

Re: [PATCH 10/16] mm, memory_hotplug: remove obsolete generic_free_nodedata()

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> Commit 390511e1476e ("mm, memory_hotplug: drop arch_free_nodedata")
> drops the last caller of generic_free_nodedata(). Remove it too.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  include/linux/memory_hotplug.h | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 51052969dbfe..9fcbf5706595 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -43,11 +43,6 @@ extern void arch_refresh_nodedata(int nid, pg_data_t *pgdat);
>  ({								\
>  	memblock_alloc(sizeof(*pgdat), SMP_CACHE_BYTES);	\
>  })
> -/*
> - * This definition is just for error path in node hotadd.
> - * For node hotremove, we have to replace this.
> - */
> -#define generic_free_nodedata(pgdat)	kfree(pgdat)
>  
>  extern pg_data_t *node_data[];
>  static inline void arch_refresh_nodedata(int nid, pg_data_t *pgdat)
> @@ -63,9 +58,6 @@ static inline pg_data_t *generic_alloc_nodedata(int nid)
>  	BUG();
>  	return NULL;
>  }
> -static inline void generic_free_nodedata(pg_data_t *pgdat)
> -{
> -}
>  static inline void arch_refresh_nodedata(int nid, pg_data_t *pgdat)
>  {
>  }

Re: [PATCH 10/16] mm, memory_hotplug: remove obsolete generic_free_nodedata()

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:45PM +0800, Miaohe Lin wrote:
> Commit 390511e1476e ("mm, memory_hotplug: drop arch_free_nodedata")
> drops the last caller of generic_free_nodedata(). Remove it too.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  include/linux/memory_hotplug.h | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 51052969dbfe..9fcbf5706595 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -43,11 +43,6 @@ extern void arch_refresh_nodedata(int nid, pg_data_t *pgdat);
>  ({								\
>  	memblock_alloc(sizeof(*pgdat), SMP_CACHE_BYTES);	\
>  })
> -/*
> - * This definition is just for error path in node hotadd.
> - * For node hotremove, we have to replace this.
> - */
> -#define generic_free_nodedata(pgdat)	kfree(pgdat)
>  
>  extern pg_data_t *node_data[];
>  static inline void arch_refresh_nodedata(int nid, pg_data_t *pgdat)
> @@ -63,9 +58,6 @@ static inline pg_data_t *generic_alloc_nodedata(int nid)
>  	BUG();
>  	return NULL;
>  }
> -static inline void generic_free_nodedata(pg_data_t *pgdat)
> -{
> -}
>  static inline void arch_refresh_nodedata(int nid, pg_data_t *pgdat)
>  {
>  }
> -- 
> 2.23.0
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 11/16] mm/page_alloc: make boot_nodestats static

[Miaohe Lin]

It's only used in mm/page_alloc.c now. Make it static.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/internal.h   | 2 --
 mm/page_alloc.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index 2424fcde6538..43a441d35baf 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -864,8 +864,6 @@ int migrate_device_coherent_page(struct page *page);
  */
 struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags);
 
-DECLARE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
-
 extern bool mirrored_kernelcore;
 
 static inline bool vma_soft_dirty_enabled(struct vm_area_struct *vma)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3417232afa45..7a8a6bb08a15 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6571,7 +6571,7 @@ static void per_cpu_pages_init(struct per_cpu_pages *pcp, struct per_cpu_zonesta
 #define BOOT_PAGESET_BATCH	1
 static DEFINE_PER_CPU(struct per_cpu_pages, boot_pageset);
 static DEFINE_PER_CPU(struct per_cpu_zonestat, boot_zonestats);
-DEFINE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
+static DEFINE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
 
 static void __build_all_zonelists(void *data)
 {
-- 
2.23.0

Re: [PATCH 11/16] mm/page_alloc: make boot_nodestats static

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> It's only used in mm/page_alloc.c now. Make it static.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/internal.h   | 2 --
>   mm/page_alloc.c | 2 +-
>   2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 2424fcde6538..43a441d35baf 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -864,8 +864,6 @@ int migrate_device_coherent_page(struct page *page);
>    */
>   struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags);
>   
> -DECLARE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
> -
>   extern bool mirrored_kernelcore;
>   
>   static inline bool vma_soft_dirty_enabled(struct vm_area_struct *vma)
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3417232afa45..7a8a6bb08a15 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6571,7 +6571,7 @@ static void per_cpu_pages_init(struct per_cpu_pages *pcp, struct per_cpu_zonesta
>   #define BOOT_PAGESET_BATCH	1
>   static DEFINE_PER_CPU(struct per_cpu_pages, boot_pageset);
>   static DEFINE_PER_CPU(struct per_cpu_zonestat, boot_zonestats);
> -DEFINE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
> +static DEFINE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
>   
>   static void __build_all_zonelists(void *data)
>   {

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 11/16] mm/page_alloc: make boot_nodestats static

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> It's only used in mm/page_alloc.c now. Make it static.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  mm/internal.h   | 2 --
>  mm/page_alloc.c | 2 +-
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 2424fcde6538..43a441d35baf 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -864,8 +864,6 @@ int migrate_device_coherent_page(struct page *page);
>   */
>  struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags);
>  
> -DECLARE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
> -
>  extern bool mirrored_kernelcore;
>  
>  static inline bool vma_soft_dirty_enabled(struct vm_area_struct *vma)
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3417232afa45..7a8a6bb08a15 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6571,7 +6571,7 @@ static void per_cpu_pages_init(struct per_cpu_pages *pcp, struct per_cpu_zonesta
>  #define BOOT_PAGESET_BATCH	1
>  static DEFINE_PER_CPU(struct per_cpu_pages, boot_pageset);
>  static DEFINE_PER_CPU(struct per_cpu_zonestat, boot_zonestats);
> -DEFINE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
> +static DEFINE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
>  
>  static void __build_all_zonelists(void *data)
>  {

Re: [PATCH 11/16] mm/page_alloc: make boot_nodestats static

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:46PM +0800, Miaohe Lin wrote:
> It's only used in mm/page_alloc.c now. Make it static.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/internal.h   | 2 --
>  mm/page_alloc.c | 2 +-
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 2424fcde6538..43a441d35baf 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -864,8 +864,6 @@ int migrate_device_coherent_page(struct page *page);
>   */
>  struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags);
>  
> -DECLARE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
> -
>  extern bool mirrored_kernelcore;
>  
>  static inline bool vma_soft_dirty_enabled(struct vm_area_struct *vma)
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3417232afa45..7a8a6bb08a15 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6571,7 +6571,7 @@ static void per_cpu_pages_init(struct per_cpu_pages *pcp, struct per_cpu_zonesta
>  #define BOOT_PAGESET_BATCH	1
>  static DEFINE_PER_CPU(struct per_cpu_pages, boot_pageset);
>  static DEFINE_PER_CPU(struct per_cpu_zonestat, boot_zonestats);
> -DEFINE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
> +static DEFINE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
>  
>  static void __build_all_zonelists(void *data)
>  {
> -- 
> 2.23.0
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 12/16] mm/page_alloc: use helper macro SZ_1{K,M}

[Miaohe Lin]

Use helper macro SZ_1K and SZ_1M to do the size conversion. Minor
readability improvement.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/page_alloc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 7a8a6bb08a15..e1c7f98cff96 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7053,7 +7053,7 @@ static int zone_batchsize(struct zone *zone)
 	 * size is striking a balance between allocation latency
 	 * and zone lock contention.
 	 */
-	batch = min(zone_managed_pages(zone) >> 10, (1024 * 1024) / PAGE_SIZE);
+	batch = min(zone_managed_pages(zone) >> 10, SZ_1M / PAGE_SIZE);
 	batch /= 4;		/* We effectively *= 4 below */
 	if (batch < 1)
 		batch = 1;
@@ -8528,8 +8528,8 @@ void __init mem_init_print_info(void)
 #endif
 		")\n",
 		K(nr_free_pages()), K(physpages),
-		codesize >> 10, datasize >> 10, rosize >> 10,
-		(init_data_size + init_code_size) >> 10, bss_size >> 10,
+		codesize / SZ_1K, datasize / SZ_1K, rosize / SZ_1K,
+		(init_data_size + init_code_size) / SZ_1K, bss_size / SZ_1K,
 		K(physpages - totalram_pages() - totalcma_pages),
 		K(totalcma_pages)
 #ifdef	CONFIG_HIGHMEM
@@ -9055,7 +9055,7 @@ void *__init alloc_large_system_hash(const char *tablename,
 
 		/* It isn't necessary when PAGE_SIZE >= 1MB */
 		if (PAGE_SHIFT < 20)
-			numentries = round_up(numentries, (1<<20)/PAGE_SIZE);
+			numentries = round_up(numentries, SZ_1M / PAGE_SIZE);
 
 #if __BITS_PER_LONG > 32
 		if (!high_limit) {
-- 
2.23.0

Re: [PATCH 12/16] mm/page_alloc: use helper macro SZ_1{K,M}

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> Use helper macro SZ_1K and SZ_1M to do the size conversion. Minor
> readability improvement.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/page_alloc.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 7a8a6bb08a15..e1c7f98cff96 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7053,7 +7053,7 @@ static int zone_batchsize(struct zone *zone)
>   	 * size is striking a balance between allocation latency
>   	 * and zone lock contention.
>   	 */
> -	batch = min(zone_managed_pages(zone) >> 10, (1024 * 1024) / PAGE_SIZE);
> +	batch = min(zone_managed_pages(zone) >> 10, SZ_1M / PAGE_SIZE);
>   	batch /= 4;		/* We effectively *= 4 below */
>   	if (batch < 1)
>   		batch = 1;
> @@ -8528,8 +8528,8 @@ void __init mem_init_print_info(void)
>   #endif
>   		")\n",
>   		K(nr_free_pages()), K(physpages),
> -		codesize >> 10, datasize >> 10, rosize >> 10,
> -		(init_data_size + init_code_size) >> 10, bss_size >> 10,
> +		codesize / SZ_1K, datasize / SZ_1K, rosize / SZ_1K,
> +		(init_data_size + init_code_size) / SZ_1K, bss_size / SZ_1K,
>   		K(physpages - totalram_pages() - totalcma_pages),
>   		K(totalcma_pages)
>   #ifdef	CONFIG_HIGHMEM
> @@ -9055,7 +9055,7 @@ void *__init alloc_large_system_hash(const char *tablename,
>   
>   		/* It isn't necessary when PAGE_SIZE >= 1MB */

Huh, how could we ever have that. Smells like dead code.

>   		if (PAGE_SHIFT < 20)

What about adjusting that as well? The it exactly matches the comment

if (PAGE_SIZE >= SZ_1M)

> -			numentries = round_up(numentries, (1<<20)/PAGE_SIZE);
> +			numentries = round_up(numentries, SZ_1M / PAGE_SIZE);
>   
>   #if __BITS_PER_LONG > 32
>   		if (!high_limit) {

-- 
Thanks,

David / dhildenb

Re: [PATCH 12/16] mm/page_alloc: use helper macro SZ_1{K,M}

[Matthew Wilcox]

On Fri, Sep 09, 2022 at 01:34:52PM +0200, David Hildenbrand wrote:
> On 09.09.22 11:24, Miaohe Lin wrote:
> > @@ -9055,7 +9055,7 @@ void *__init alloc_large_system_hash(const char *tablename,
> >   		/* It isn't necessary when PAGE_SIZE >= 1MB */
> 
> Huh, how could we ever have that. Smells like dead code.
> 
> >   		if (PAGE_SHIFT < 20)
> 
> What about adjusting that as well? The it exactly matches the comment
> 
> if (PAGE_SIZE >= SZ_1M)
> 
> > -			numentries = round_up(numentries, (1<<20)/PAGE_SIZE);
> > +			numentries = round_up(numentries, SZ_1M / PAGE_SIZE);

The git history provides some clues here.  See a7e833182a92.
But we do have an architecture which has ...

#ifdef CONFIG_PAGE_SIZE_1MB
#define PAGE_SHIFT 20
#define HEXAGON_L1_PTE_SIZE __HVM_PDE_S_1MB
#endif

I don't think it's an especially common config.

Re: [PATCH 12/16] mm/page_alloc: use helper macro SZ_1{K,M}

[Miaohe Lin]

On 2022/9/10 3:44, Matthew Wilcox wrote:
> On Fri, Sep 09, 2022 at 01:34:52PM +0200, David Hildenbrand wrote:
>> On 09.09.22 11:24, Miaohe Lin wrote:
>>> @@ -9055,7 +9055,7 @@ void *__init alloc_large_system_hash(const char *tablename,
>>>   		/* It isn't necessary when PAGE_SIZE >= 1MB */
>>
>> Huh, how could we ever have that. Smells like dead code.
>>
>>>   		if (PAGE_SHIFT < 20)
>>
>> What about adjusting that as well? The it exactly matches the comment
>>
>> if (PAGE_SIZE >= SZ_1M)

Looks good. Will do it in next version.

>>
>>> -			numentries = round_up(numentries, (1<<20)/PAGE_SIZE);
>>> +			numentries = round_up(numentries, SZ_1M / PAGE_SIZE);
> 
> The git history provides some clues here.  See a7e833182a92.
> But we do have an architecture which has ...
> 
> #ifdef CONFIG_PAGE_SIZE_1MB
> #define PAGE_SHIFT 20
> #define HEXAGON_L1_PTE_SIZE __HVM_PDE_S_1MB
> #endif
> 
> I don't think it's an especially common config.

Maybe commit a7e833182a92 fixed a theoretical bug. But IMHO, it might be better to keep the code even
if no architecture defines PAGE_SIZE >= 1MB. These codes would be eliminated at compiling time. And
once there're architectures with PAGE_SIZE >= 1MB, we still work. Any thoughts? Thanks both.

Thanks,
Miaohe Lin

[PATCH 13/16] mm/page_alloc: init local variable buddy_pfn

[Miaohe Lin]

The local variable buddy_pfn could be passed to buddy_merge_likely()
without initialization if the passed in order is MAX_ORDER - 1. This
looks buggy but buddy_pfn won't be used in this case as there's a
order >= MAX_ORDER - 2 check. Init buddy_pfn to 0 anyway to avoid
possible future misuse.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e1c7f98cff96..63ad25e86010 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1113,7 +1113,7 @@ static inline void __free_one_page(struct page *page,
 		int migratetype, fpi_t fpi_flags)
 {
 	struct capture_control *capc = task_capc(zone);
-	unsigned long buddy_pfn;
+	unsigned long buddy_pfn = 0;
 	unsigned long combined_pfn;
 	struct page *buddy;
 	bool to_tail;
-- 
2.23.0

Re: [PATCH 13/16] mm/page_alloc: init local variable buddy_pfn

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> The local variable buddy_pfn could be passed to buddy_merge_likely()
> without initialization if the passed in order is MAX_ORDER - 1. This
> looks buggy but buddy_pfn won't be used in this case as there's a
> order >= MAX_ORDER - 2 check. Init buddy_pfn to 0 anyway to avoid
> possible future misuse.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/page_alloc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index e1c7f98cff96..63ad25e86010 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1113,7 +1113,7 @@ static inline void __free_one_page(struct page *page,
>   		int migratetype, fpi_t fpi_flags)
>   {
>   	struct capture_control *capc = task_capc(zone);
> -	unsigned long buddy_pfn;
> +	unsigned long buddy_pfn = 0;
>   	unsigned long combined_pfn;
>   	struct page *buddy;
>   	bool to_tail;

Yeah, why not.

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 13/16] mm/page_alloc: init local variable buddy_pfn

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> The local variable buddy_pfn could be passed to buddy_merge_likely()
> without initialization if the passed in order is MAX_ORDER - 1. This
> looks buggy but buddy_pfn won't be used in this case as there's a
> order >= MAX_ORDER - 2 check. Init buddy_pfn to 0 anyway to avoid
> possible future misuse.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index e1c7f98cff96..63ad25e86010 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1113,7 +1113,7 @@ static inline void __free_one_page(struct page *page,
>  		int migratetype, fpi_t fpi_flags)
>  {
>  	struct capture_control *capc = task_capc(zone);
> -	unsigned long buddy_pfn;
> +	unsigned long buddy_pfn = 0;
>  	unsigned long combined_pfn;
>  	struct page *buddy;
>  	bool to_tail;

Re: [PATCH 13/16] mm/page_alloc: init local variable buddy_pfn

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:48PM +0800, Miaohe Lin wrote:
> The local variable buddy_pfn could be passed to buddy_merge_likely()
> without initialization if the passed in order is MAX_ORDER - 1. This
> looks buggy but buddy_pfn won't be used in this case as there's a
> order >= MAX_ORDER - 2 check. Init buddy_pfn to 0 anyway to avoid
> possible future misuse.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index e1c7f98cff96..63ad25e86010 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1113,7 +1113,7 @@ static inline void __free_one_page(struct page *page,
>  		int migratetype, fpi_t fpi_flags)
>  {
>  	struct capture_control *capc = task_capc(zone);
> -	unsigned long buddy_pfn;
> +	unsigned long buddy_pfn = 0;
>  	unsigned long combined_pfn;
>  	struct page *buddy;
>  	bool to_tail;
> -- 
> 2.23.0
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 14/16] mm/page_alloc: use costly_order in WARN_ON_ONCE_GFP()

[Miaohe Lin]

There's no need to check whether order > PAGE_ALLOC_COSTLY_ORDER again.
Minor readability improvement.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 63ad25e86010..262e8972e019 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5272,7 +5272,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 		 * so that we can identify them and convert them to something
 		 * else.
 		 */
-		WARN_ON_ONCE_GFP(order > PAGE_ALLOC_COSTLY_ORDER, gfp_mask);
+		WARN_ON_ONCE_GFP(costly_order, gfp_mask);
 
 		/*
 		 * Help non-failing allocations by giving them access to memory
-- 
2.23.0

Re: [PATCH 14/16] mm/page_alloc: use costly_order in WARN_ON_ONCE_GFP()

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> There's no need to check whether order > PAGE_ALLOC_COSTLY_ORDER again.
> Minor readability improvement.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/page_alloc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 63ad25e86010..262e8972e019 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5272,7 +5272,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>   		 * so that we can identify them and convert them to something
>   		 * else.
>   		 */
> -		WARN_ON_ONCE_GFP(order > PAGE_ALLOC_COSTLY_ORDER, gfp_mask);
> +		WARN_ON_ONCE_GFP(costly_order, gfp_mask);
>   
>   		/*
>   		 * Help non-failing allocations by giving them access to memory

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 14/16] mm/page_alloc: use costly_order in WARN_ON_ONCE_GFP()

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> There's no need to check whether order > PAGE_ALLOC_COSTLY_ORDER again.
> Minor readability improvement.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 63ad25e86010..262e8972e019 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5272,7 +5272,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  		 * so that we can identify them and convert them to something
>  		 * else.
>  		 */
> -		WARN_ON_ONCE_GFP(order > PAGE_ALLOC_COSTLY_ORDER, gfp_mask);
> +		WARN_ON_ONCE_GFP(costly_order, gfp_mask);
>  
>  		/*
>  		 * Help non-failing allocations by giving them access to memory

Re: [PATCH 14/16] mm/page_alloc: use costly_order in WARN_ON_ONCE_GFP()

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:49PM +0800, Miaohe Lin wrote:
> There's no need to check whether order > PAGE_ALLOC_COSTLY_ORDER again.
> Minor readability improvement.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 63ad25e86010..262e8972e019 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5272,7 +5272,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  		 * so that we can identify them and convert them to something
>  		 * else.
>  		 */
> -		WARN_ON_ONCE_GFP(order > PAGE_ALLOC_COSTLY_ORDER, gfp_mask);
> +		WARN_ON_ONCE_GFP(costly_order, gfp_mask);
>  
>  		/*
>  		 * Help non-failing allocations by giving them access to memory
> -- 
> 2.23.0
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 15/16] mm/page_alloc: remove obsolete gfpflags_normal_context()

[Miaohe Lin]

Since commit dacb5d8875cc ("tcp: fix page frag corruption on page
fault"), there's no caller of gfpflags_normal_context(). Remove it
as this helper is strictly tied to the sk page frag usage and there
won't be other user in the future.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/gfp.h | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index ea6cb9399152..ef4aea3b356e 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -36,29 +36,6 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
 	return !!(gfp_flags & __GFP_DIRECT_RECLAIM);
 }
 
-/**
- * gfpflags_normal_context - is gfp_flags a normal sleepable context?
- * @gfp_flags: gfp_flags to test
- *
- * Test whether @gfp_flags indicates that the allocation is from the
- * %current context and allowed to sleep.
- *
- * An allocation being allowed to block doesn't mean it owns the %current
- * context.  When direct reclaim path tries to allocate memory, the
- * allocation context is nested inside whatever %current was doing at the
- * time of the original allocation.  The nested allocation may be allowed
- * to block but modifying anything %current owns can corrupt the outer
- * context's expectations.
- *
- * %true result from this function indicates that the allocation context
- * can sleep and use anything that's associated with %current.
- */
-static inline bool gfpflags_normal_context(const gfp_t gfp_flags)
-{
-	return (gfp_flags & (__GFP_DIRECT_RECLAIM | __GFP_MEMALLOC)) ==
-		__GFP_DIRECT_RECLAIM;
-}
-
 #ifdef CONFIG_HIGHMEM
 #define OPT_ZONE_HIGHMEM ZONE_HIGHMEM
 #else
-- 
2.23.0

Re: [PATCH 15/16] mm/page_alloc: remove obsolete gfpflags_normal_context()

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> Since commit dacb5d8875cc ("tcp: fix page frag corruption on page
> fault"), there's no caller of gfpflags_normal_context(). Remove it
> as this helper is strictly tied to the sk page frag usage and there
> won't be other user in the future.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   include/linux/gfp.h | 23 -----------------------
>   1 file changed, 23 deletions(-)
> 
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index ea6cb9399152..ef4aea3b356e 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -36,29 +36,6 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
>   	return !!(gfp_flags & __GFP_DIRECT_RECLAIM);
>   }
>   
> -/**
> - * gfpflags_normal_context - is gfp_flags a normal sleepable context?
> - * @gfp_flags: gfp_flags to test
> - *
> - * Test whether @gfp_flags indicates that the allocation is from the
> - * %current context and allowed to sleep.
> - *
> - * An allocation being allowed to block doesn't mean it owns the %current
> - * context.  When direct reclaim path tries to allocate memory, the
> - * allocation context is nested inside whatever %current was doing at the
> - * time of the original allocation.  The nested allocation may be allowed
> - * to block but modifying anything %current owns can corrupt the outer
> - * context's expectations.
> - *
> - * %true result from this function indicates that the allocation context
> - * can sleep and use anything that's associated with %current.
> - */
> -static inline bool gfpflags_normal_context(const gfp_t gfp_flags)
> -{
> -	return (gfp_flags & (__GFP_DIRECT_RECLAIM | __GFP_MEMALLOC)) ==
> -		__GFP_DIRECT_RECLAIM;
> -}
> -
>   #ifdef CONFIG_HIGHMEM
>   #define OPT_ZONE_HIGHMEM ZONE_HIGHMEM
>   #else

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 15/16] mm/page_alloc: remove obsolete gfpflags_normal_context()

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> Since commit dacb5d8875cc ("tcp: fix page frag corruption on page
> fault"), there's no caller of gfpflags_normal_context(). Remove it
> as this helper is strictly tied to the sk page frag usage and there
> won't be other user in the future.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  include/linux/gfp.h | 23 -----------------------
>  1 file changed, 23 deletions(-)
> 
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index ea6cb9399152..ef4aea3b356e 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -36,29 +36,6 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
>  	return !!(gfp_flags & __GFP_DIRECT_RECLAIM);
>  }
>  
> -/**
> - * gfpflags_normal_context - is gfp_flags a normal sleepable context?
> - * @gfp_flags: gfp_flags to test
> - *
> - * Test whether @gfp_flags indicates that the allocation is from the
> - * %current context and allowed to sleep.
> - *
> - * An allocation being allowed to block doesn't mean it owns the %current
> - * context.  When direct reclaim path tries to allocate memory, the
> - * allocation context is nested inside whatever %current was doing at the
> - * time of the original allocation.  The nested allocation may be allowed
> - * to block but modifying anything %current owns can corrupt the outer
> - * context's expectations.
> - *
> - * %true result from this function indicates that the allocation context
> - * can sleep and use anything that's associated with %current.
> - */
> -static inline bool gfpflags_normal_context(const gfp_t gfp_flags)
> -{
> -	return (gfp_flags & (__GFP_DIRECT_RECLAIM | __GFP_MEMALLOC)) ==
> -		__GFP_DIRECT_RECLAIM;
> -}
> -
>  #ifdef CONFIG_HIGHMEM
>  #define OPT_ZONE_HIGHMEM ZONE_HIGHMEM
>  #else

Re: [PATCH 15/16] mm/page_alloc: remove obsolete gfpflags_normal_context()

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> Since commit dacb5d8875cc ("tcp: fix page frag corruption on page
> fault"), there's no caller of gfpflags_normal_context(). Remove it
> as this helper is strictly tied to the sk page frag usage and there
> won't be other user in the future.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  include/linux/gfp.h | 23 -----------------------
>  1 file changed, 23 deletions(-)
> 
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index ea6cb9399152..ef4aea3b356e 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -36,29 +36,6 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
>  	return !!(gfp_flags & __GFP_DIRECT_RECLAIM);
>  }
>  
> -/**
> - * gfpflags_normal_context - is gfp_flags a normal sleepable context?
> - * @gfp_flags: gfp_flags to test
> - *
> - * Test whether @gfp_flags indicates that the allocation is from the
> - * %current context and allowed to sleep.
> - *
> - * An allocation being allowed to block doesn't mean it owns the %current
> - * context.  When direct reclaim path tries to allocate memory, the
> - * allocation context is nested inside whatever %current was doing at the
> - * time of the original allocation.  The nested allocation may be allowed
> - * to block but modifying anything %current owns can corrupt the outer
> - * context's expectations.
> - *
> - * %true result from this function indicates that the allocation context
> - * can sleep and use anything that's associated with %current.
> - */
> -static inline bool gfpflags_normal_context(const gfp_t gfp_flags)
> -{
> -	return (gfp_flags & (__GFP_DIRECT_RECLAIM | __GFP_MEMALLOC)) ==
> -		__GFP_DIRECT_RECLAIM;
> -}
> -
>  #ifdef CONFIG_HIGHMEM
>  #define OPT_ZONE_HIGHMEM ZONE_HIGHMEM
>  #else

Re: [PATCH 15/16] mm/page_alloc: remove obsolete gfpflags_normal_context()

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:50PM +0800, Miaohe Lin wrote:
> Since commit dacb5d8875cc ("tcp: fix page frag corruption on page
> fault"), there's no caller of gfpflags_normal_context(). Remove it
> as this helper is strictly tied to the sk page frag usage and there
> won't be other user in the future.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  include/linux/gfp.h | 23 -----------------------
>  1 file changed, 23 deletions(-)
> 
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index ea6cb9399152..ef4aea3b356e 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -36,29 +36,6 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
>  	return !!(gfp_flags & __GFP_DIRECT_RECLAIM);
>  }
>  
> -/**
> - * gfpflags_normal_context - is gfp_flags a normal sleepable context?
> - * @gfp_flags: gfp_flags to test
> - *
> - * Test whether @gfp_flags indicates that the allocation is from the
> - * %current context and allowed to sleep.
> - *
> - * An allocation being allowed to block doesn't mean it owns the %current
> - * context.  When direct reclaim path tries to allocate memory, the
> - * allocation context is nested inside whatever %current was doing at the
> - * time of the original allocation.  The nested allocation may be allowed
> - * to block but modifying anything %current owns can corrupt the outer
> - * context's expectations.
> - *
> - * %true result from this function indicates that the allocation context
> - * can sleep and use anything that's associated with %current.
> - */
> -static inline bool gfpflags_normal_context(const gfp_t gfp_flags)
> -{
> -	return (gfp_flags & (__GFP_DIRECT_RECLAIM | __GFP_MEMALLOC)) ==
> -		__GFP_DIRECT_RECLAIM;
> -}
> -
>  #ifdef CONFIG_HIGHMEM
>  #define OPT_ZONE_HIGHMEM ZONE_HIGHMEM
>  #else
> -- 
> 2.23.0
> 

-- 
Oscar Salvador
SUSE Labs

[PATCH 16/16] mm/page_alloc: fix obsolete comment in deferred_pfn_valid()

[Miaohe Lin]

There are no architectures that can have holes in the memory map within
a pageblock since commit 859a85ddf90e ("mm: remove pfn_valid_within()
and CONFIG_HOLES_IN_ZONE"). Update the corresponding comment.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/page_alloc.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 262e8972e019..4cc0232020d2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1929,11 +1929,7 @@ static inline void __init pgdat_init_report_one_done(void)
 /*
  * Returns true if page needs to be initialized or freed to buddy allocator.
  *
- * First we check if pfn is valid on architectures where it is possible to have
- * holes within pageblock_nr_pages. On systems where it is not possible, this
- * function is optimized out.
- *
- * Then, we check if a current large page is valid by only checking the validity
+ * We check if a current large page is valid by only checking the validity
  * of the head pfn.
  */
 static inline bool __init deferred_pfn_valid(unsigned long pfn)
-- 
2.23.0

Re: [PATCH 16/16] mm/page_alloc: fix obsolete comment in deferred_pfn_valid()

[David Hildenbrand]

On 09.09.22 11:24, Miaohe Lin wrote:
> There are no architectures that can have holes in the memory map within
> a pageblock since commit 859a85ddf90e ("mm: remove pfn_valid_within()
> and CONFIG_HOLES_IN_ZONE"). Update the corresponding comment.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/page_alloc.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 262e8972e019..4cc0232020d2 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1929,11 +1929,7 @@ static inline void __init pgdat_init_report_one_done(void)
>   /*
>    * Returns true if page needs to be initialized or freed to buddy allocator.
>    *
> - * First we check if pfn is valid on architectures where it is possible to have
> - * holes within pageblock_nr_pages. On systems where it is not possible, this
> - * function is optimized out.
> - *
> - * Then, we check if a current large page is valid by only checking the validity
> + * We check if a current large page is valid by only checking the validity
>    * of the head pfn.
>    */
>   static inline bool __init deferred_pfn_valid(unsigned long pfn)

Reviewed-by: David Hildenbrand <david@redhat.com>
-- 
Thanks,

David / dhildenb

Re: [PATCH 16/16] mm/page_alloc: fix obsolete comment in deferred_pfn_valid()

[Anshuman Khandual]

On 9/9/22 14:54, Miaohe Lin wrote:
> There are no architectures that can have holes in the memory map within
> a pageblock since commit 859a85ddf90e ("mm: remove pfn_valid_within()
> and CONFIG_HOLES_IN_ZONE"). Update the corresponding comment.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>


> ---
>  mm/page_alloc.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 262e8972e019..4cc0232020d2 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1929,11 +1929,7 @@ static inline void __init pgdat_init_report_one_done(void)
>  /*
>   * Returns true if page needs to be initialized or freed to buddy allocator.
>   *
> - * First we check if pfn is valid on architectures where it is possible to have
> - * holes within pageblock_nr_pages. On systems where it is not possible, this
> - * function is optimized out.
> - *
> - * Then, we check if a current large page is valid by only checking the validity
> + * We check if a current large page is valid by only checking the validity
>   * of the head pfn.
>   */
>  static inline bool __init deferred_pfn_valid(unsigned long pfn)

Re: [PATCH 16/16] mm/page_alloc: fix obsolete comment in deferred_pfn_valid()

[Oscar Salvador]

On Fri, Sep 09, 2022 at 05:24:51PM +0800, Miaohe Lin wrote:
> There are no architectures that can have holes in the memory map within
> a pageblock since commit 859a85ddf90e ("mm: remove pfn_valid_within()
> and CONFIG_HOLES_IN_ZONE"). Update the corresponding comment.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/page_alloc.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 262e8972e019..4cc0232020d2 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1929,11 +1929,7 @@ static inline void __init pgdat_init_report_one_done(void)
>  /*
>   * Returns true if page needs to be initialized or freed to buddy allocator.
>   *
> - * First we check if pfn is valid on architectures where it is possible to have
> - * holes within pageblock_nr_pages. On systems where it is not possible, this
> - * function is optimized out.
> - *
> - * Then, we check if a current large page is valid by only checking the validity
> + * We check if a current large page is valid by only checking the validity
>   * of the head pfn.
>   */
>  static inline bool __init deferred_pfn_valid(unsigned long pfn)
> -- 
> 2.23.0
> 

-- 
Oscar Salvador
SUSE Labs