[PATCH v3 00/15] Allocate and free frozen pages

[PATCH v3 00/15] Allocate and free frozen pages

[Matthew Wilcox (Oracle)]

Slab does not need to use the page refcount at all, and it can avoid
an atomic operation on page free.  Hugetlb wants to delay setting the
refcount until it has assembled a complete gigantic page.  We already
have the ability to freeze a page (safely reduce its reference count to
0), so this patchset adds APIs to allocate and free pages which are in
a frozen state.

This patchset is also a step towards the Glorious Future in which struct
page doesn't have a refcount; the users which need a refcount will have
one in their per-allocation memdesc.

v3:
 - Rebase to next-20241114
 - Drop patch for free_the_page() as it no longer exists; instead rename
   free_unref_page() to free_frozen_pages()
 - Add patch to make alloc_pages_mpol() static
 - Drop slab patch since slab is gone
 - There are now many more callers of post_alloc_hook() and prep_new_page()
   than there used to be, but I reviewed the changes and believe they are
   all changed in the appropriate commit.
 - Adapt to _noprof

v2: https://lore.kernel.org/linux-mm/20220809171854.3725722-1-willy@infradead.org/

Matthew Wilcox (Oracle) (15):
  mm/page_alloc: Cache page_zone() result in free_unref_page()
  mm: Make alloc_pages_mpol() static
  mm/page_alloc: Export free_frozen_pages() instead of free_unref_page()
  mm/page_alloc: Move set_page_refcounted() to callers of
    post_alloc_hook()
  mm/page_alloc: Move set_page_refcounted() to callers of
    prep_new_page()
  mm/page_alloc: Move set_page_refcounted() to callers of
    get_page_from_freelist()
  mm/page_alloc: Move set_page_refcounted() to callers of
    __alloc_pages_cpuset_fallback()
  mm/page_alloc: Move set_page_refcounted() to callers of
    __alloc_pages_may_oom()
  mm/page_alloc: Move set_page_refcounted() to callers of
    __alloc_pages_direct_compact()
  mm/page_alloc: Move set_page_refcounted() to callers of
    __alloc_pages_direct_reclaim()
  mm/page_alloc: Move set_page_refcounted() to callers of
    __alloc_pages_slowpath()
  mm/page_alloc: Move set_page_refcounted() to end of __alloc_pages()
  mm/page_alloc: Add __alloc_frozen_pages()
  mm/mempolicy: Add alloc_frozen_pages()
  slab: Allocate frozen pages

 include/linux/gfp.h  |  8 -------
 mm/compaction.c      |  2 ++
 mm/internal.h        | 21 +++++++++++++++---
 mm/mempolicy.c       | 53 ++++++++++++++++++++++++++++----------------
 mm/page_alloc.c      | 45 ++++++++++++++++++++++++-------------
 mm/page_frag_cache.c |  6 ++---
 mm/slub.c            |  6 ++---
 mm/swap.c            |  2 +-
 8 files changed, 90 insertions(+), 53 deletions(-)

-- 
2.45.2

[PATCH v3 01/15] mm/page_alloc: Cache page_zone() result in free_unref_page()

[Matthew Wilcox (Oracle)]

Save 17 bytes of text by calculating page_zone() once instead of twice.

Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: Muchun Song <songmuchun@bytedance.com>
Acked-by: Mel Gorman <mgorman@techsinguularity.net>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/page_alloc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a8cd54c02f13..c40a0c29a89c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2700,16 +2700,16 @@ void free_unref_page(struct page *page, unsigned int order)
 	 * get those areas back if necessary. Otherwise, we may have to free
 	 * excessively into the page allocator
 	 */
+	zone = page_zone(page);
 	migratetype = get_pfnblock_migratetype(page, pfn);
 	if (unlikely(migratetype >= MIGRATE_PCPTYPES)) {
 		if (unlikely(is_migrate_isolate(migratetype))) {
-			free_one_page(page_zone(page), page, pfn, order, FPI_NONE);
+			free_one_page(zone, page, pfn, order, FPI_NONE);
 			return;
 		}
 		migratetype = MIGRATE_MOVABLE;
 	}
 
-	zone = page_zone(page);
 	pcp_trylock_prepare(UP_flags);
 	pcp = pcp_spin_trylock(zone->per_cpu_pageset);
 	if (pcp) {
-- 
2.45.2

Re: [PATCH v3 01/15] mm/page_alloc: Cache page_zone() result in free_unref_page()

[David Hildenbrand]

On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
> Save 17 bytes of text by calculating page_zone() once instead of twice.
> 
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> Reviewed-by: Muchun Song <songmuchun@bytedance.com>
> Acked-by: Mel Gorman <mgorman@techsinguularity.net>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---

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

-- 
Cheers,

David / dhildenb

Re: [PATCH v3 01/15] mm/page_alloc: Cache page_zone() result in free_unref_page()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> Save 17 bytes of text by calculating page_zone() once instead of twice.
>
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> Reviewed-by: Muchun Song <songmuchun@bytedance.com>
> Acked-by: Mel Gorman <mgorman@techsinguularity.net>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/page_alloc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Zi Yan <ziy@nvidia.com>


--
Best Regards,
Yan, Zi

Re: [PATCH v3 01/15] mm/page_alloc: Cache page_zone() result in free_unref_page()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> Save 17 bytes of text by calculating page_zone() once instead of twice.
> 
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> Reviewed-by: Muchun Song <songmuchun@bytedance.com>
> Acked-by: Mel Gorman <mgorman@techsinguularity.net>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/page_alloc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a8cd54c02f13..c40a0c29a89c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2700,16 +2700,16 @@ void free_unref_page(struct page *page, unsigned int order)
>  	 * get those areas back if necessary. Otherwise, we may have to free
>  	 * excessively into the page allocator
>  	 */
> +	zone = page_zone(page);
>  	migratetype = get_pfnblock_migratetype(page, pfn);
>  	if (unlikely(migratetype >= MIGRATE_PCPTYPES)) {
>  		if (unlikely(is_migrate_isolate(migratetype))) {
> -			free_one_page(page_zone(page), page, pfn, order, FPI_NONE);
> +			free_one_page(zone, page, pfn, order, FPI_NONE);
>  			return;
>  		}
>  		migratetype = MIGRATE_MOVABLE;
>  	}
>  
> -	zone = page_zone(page);
>  	pcp_trylock_prepare(UP_flags);
>  	pcp = pcp_spin_trylock(zone->per_cpu_pageset);
>  	if (pcp) {

Re: [PATCH v3 01/15] mm/page_alloc: Cache page_zone() result in free_unref_page()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> Save 17 bytes of text by calculating page_zone() once instead of twice.
> 
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> Reviewed-by: Muchun Song <songmuchun@bytedance.com>
> Acked-by: Mel Gorman <mgorman@techsinguularity.net>

Should be without the extra u            ^

> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/page_alloc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a8cd54c02f13..c40a0c29a89c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2700,16 +2700,16 @@ void free_unref_page(struct page *page, unsigned int order)
>  	 * get those areas back if necessary. Otherwise, we may have to free
>  	 * excessively into the page allocator
>  	 */
> +	zone = page_zone(page);
>  	migratetype = get_pfnblock_migratetype(page, pfn);
>  	if (unlikely(migratetype >= MIGRATE_PCPTYPES)) {
>  		if (unlikely(is_migrate_isolate(migratetype))) {
> -			free_one_page(page_zone(page), page, pfn, order, FPI_NONE);
> +			free_one_page(zone, page, pfn, order, FPI_NONE);
>  			return;
>  		}
>  		migratetype = MIGRATE_MOVABLE;
>  	}
>  
> -	zone = page_zone(page);
>  	pcp_trylock_prepare(UP_flags);
>  	pcp = pcp_spin_trylock(zone->per_cpu_pageset);
>  	if (pcp) {

Re: [PATCH v3 01/15] mm/page_alloc: Cache page_zone() result in free_unref_page()

[Matthew Wilcox]

On Tue, Dec 03, 2024 at 05:54:59PM +0100, Vlastimil Babka wrote:
> On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> > Save 17 bytes of text by calculating page_zone() once instead of twice.
> > 
> > Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> > Reviewed-by: Muchun Song <songmuchun@bytedance.com>
> > Acked-by: Mel Gorman <mgorman@techsinguularity.net>
> 
> Should be without the extra u            ^

Yeah, Mel typod it here:

https://lore.kernel.org/all/20220810150051.kptkj53ndrep446p@techsingularity.net/

and b4 doesn't have a --dwim flag yet.

Re: [PATCH v3 01/15] mm/page_alloc: Cache page_zone() result in free_unref_page()

[Konstantin Ryabitsev]

On Tue, Dec 03, 2024 at 05:20:55PM +0000, Matthew Wilcox wrote:
> > > Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> > > Reviewed-by: Muchun Song <songmuchun@bytedance.com>
> > > Acked-by: Mel Gorman <mgorman@techsinguularity.net>
> > 
> > Should be without the extra u            ^
> 
> Yeah, Mel typod it here:
> 
> https://lore.kernel.org/all/20220810150051.kptkj53ndrep446p@techsingularity.net/
> 
> and b4 doesn't have a --dwim flag yet.

 __
/  \        _____________
|  |       /             \
@  @       | It looks    |
|| ||      | like you've |
|| ||   <--| made a typo.|
|\_/|      | Tsk, tsk.   |
\___/      \_____________/

-K

[PATCH v3 02/15] mm: Make alloc_pages_mpol() static

[Matthew Wilcox (Oracle)]

All callers outside mempolicy.c now use folio_alloc_mpol() thanks to
Kefeng's cleanups, so we can remove this as a visible symbol.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/gfp.h | 8 --------
 mm/mempolicy.c      | 8 ++++----
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index b0fe9f62d15b..c96d5d7f7b89 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -300,8 +300,6 @@ static inline struct page *alloc_pages_node_noprof(int nid, gfp_t gfp_mask,
 
 #ifdef CONFIG_NUMA
 struct page *alloc_pages_noprof(gfp_t gfp, unsigned int order);
-struct page *alloc_pages_mpol_noprof(gfp_t gfp, unsigned int order,
-		struct mempolicy *mpol, pgoff_t ilx, int nid);
 struct folio *folio_alloc_noprof(gfp_t gfp, unsigned int order);
 struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
 		struct mempolicy *mpol, pgoff_t ilx, int nid);
@@ -312,11 +310,6 @@ static inline struct page *alloc_pages_noprof(gfp_t gfp_mask, unsigned int order
 {
 	return alloc_pages_node_noprof(numa_node_id(), gfp_mask, order);
 }
-static inline struct page *alloc_pages_mpol_noprof(gfp_t gfp, unsigned int order,
-		struct mempolicy *mpol, pgoff_t ilx, int nid)
-{
-	return alloc_pages_noprof(gfp, order);
-}
 static inline struct folio *folio_alloc_noprof(gfp_t gfp, unsigned int order)
 {
 	return __folio_alloc_node_noprof(gfp, order, numa_node_id());
@@ -331,7 +324,6 @@ static inline struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int orde
 #endif
 
 #define alloc_pages(...)			alloc_hooks(alloc_pages_noprof(__VA_ARGS__))
-#define alloc_pages_mpol(...)			alloc_hooks(alloc_pages_mpol_noprof(__VA_ARGS__))
 #define folio_alloc(...)			alloc_hooks(folio_alloc_noprof(__VA_ARGS__))
 #define folio_alloc_mpol(...)			alloc_hooks(folio_alloc_mpol_noprof(__VA_ARGS__))
 #define vma_alloc_folio(...)			alloc_hooks(vma_alloc_folio_noprof(__VA_ARGS__))
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index bb37cd1a51d8..cda5f56085e6 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2218,7 +2218,7 @@ static struct page *alloc_pages_preferred_many(gfp_t gfp, unsigned int order,
  *
  * Return: The page on success or NULL if allocation fails.
  */
-struct page *alloc_pages_mpol_noprof(gfp_t gfp, unsigned int order,
+static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
 		struct mempolicy *pol, pgoff_t ilx, int nid)
 {
 	nodemask_t *nodemask;
@@ -2280,7 +2280,7 @@ struct page *alloc_pages_mpol_noprof(gfp_t gfp, unsigned int order,
 struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
 		struct mempolicy *pol, pgoff_t ilx, int nid)
 {
-	return page_rmappable_folio(alloc_pages_mpol_noprof(gfp | __GFP_COMP,
+	return page_rmappable_folio(alloc_pages_mpol(gfp | __GFP_COMP,
 							order, pol, ilx, nid));
 }
 
@@ -2295,7 +2295,7 @@ struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
  * NUMA policy.  The caller must hold the mmap_lock of the mm_struct of the
  * VMA to prevent it from going away.  Should be used for all allocations
  * for folios that will be mapped into user space, excepting hugetlbfs, and
- * excepting where direct use of alloc_pages_mpol() is more appropriate.
+ * excepting where direct use of folio_alloc_mpol() is more appropriate.
  *
  * Return: The folio on success or NULL if allocation fails.
  */
@@ -2341,7 +2341,7 @@ struct page *alloc_pages_noprof(gfp_t gfp, unsigned int order)
 	if (!in_interrupt() && !(gfp & __GFP_THISNODE))
 		pol = get_task_policy(current);
 
-	return alloc_pages_mpol_noprof(gfp, order, pol, NO_INTERLEAVE_INDEX,
+	return alloc_pages_mpol(gfp, order, pol, NO_INTERLEAVE_INDEX,
 				       numa_node_id());
 }
 EXPORT_SYMBOL(alloc_pages_noprof);
-- 
2.45.2

Re: [PATCH v3 02/15] mm: Make alloc_pages_mpol() static

[David Hildenbrand]

On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
> All callers outside mempolicy.c now use folio_alloc_mpol() thanks to
> Kefeng's cleanups, so we can remove this as a visible symbol.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---

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

-- 
Cheers,

David / dhildenb

Re: [PATCH v3 02/15] mm: Make alloc_pages_mpol() static

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> All callers outside mempolicy.c now use folio_alloc_mpol() thanks to
> Kefeng's cleanups, so we can remove this as a visible symbol.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  include/linux/gfp.h | 8 --------
>  mm/mempolicy.c      | 8 ++++----
>  2 files changed, 4 insertions(+), 12 deletions(-)
>

And also remove the alloc_hooks for alloc_pages_mpol(), since
all users in mempolicy.c are using the nonprof version.

Reviewed-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH v3 02/15] mm: Make alloc_pages_mpol() static

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> All callers outside mempolicy.c now use folio_alloc_mpol() thanks to
> Kefeng's cleanups, so we can remove this as a visible symbol.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

[PATCH v3 03/15] mm/page_alloc: Export free_frozen_pages() instead of free_unref_page()

[Matthew Wilcox (Oracle)]

We already have the concept of "frozen pages" (eg page_ref_freeze()),
so let's not complicate things by also having the concept of "unref
pages".

Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: William Kucharski <william.kucharski@oracle.com>
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: Muchun Song <songmuchun@bytedance.com>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/internal.h        |  2 +-
 mm/page_alloc.c      | 18 +++++++++---------
 mm/page_frag_cache.c |  6 +++---
 mm/swap.c            |  2 +-
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index cb8d8e8e3ffa..ca400c70199c 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -741,7 +741,7 @@ extern bool free_pages_prepare(struct page *page, unsigned int order);
 
 extern int user_min_free_kbytes;
 
-void free_unref_page(struct page *page, unsigned int order);
+void free_frozen_pages(struct page *page, unsigned int order);
 void free_unref_folios(struct folio_batch *fbatch);
 
 extern void zone_pcp_reset(struct zone *zone);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c40a0c29a89c..adac485e3254 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2626,9 +2626,9 @@ static int nr_pcp_high(struct per_cpu_pages *pcp, struct zone *zone,
 	return high;
 }
 
-static void free_unref_page_commit(struct zone *zone, struct per_cpu_pages *pcp,
-				   struct page *page, int migratetype,
-				   unsigned int order)
+static void free_frozen_page_commit(struct zone *zone,
+		struct per_cpu_pages *pcp, struct page *page, int migratetype,
+		unsigned int order)
 {
 	int high, batch;
 	int pindex;
@@ -2677,7 +2677,7 @@ static void free_unref_page_commit(struct zone *zone, struct per_cpu_pages *pcp,
 /*
  * Free a pcp page
  */
-void free_unref_page(struct page *page, unsigned int order)
+void free_frozen_pages(struct page *page, unsigned int order)
 {
 	unsigned long __maybe_unused UP_flags;
 	struct per_cpu_pages *pcp;
@@ -2713,7 +2713,7 @@ void free_unref_page(struct page *page, unsigned int order)
 	pcp_trylock_prepare(UP_flags);
 	pcp = pcp_spin_trylock(zone->per_cpu_pageset);
 	if (pcp) {
-		free_unref_page_commit(zone, pcp, page, migratetype, order);
+		free_frozen_page_commit(zone, pcp, page, migratetype, order);
 		pcp_spin_unlock(pcp);
 	} else {
 		free_one_page(zone, page, pfn, order, FPI_NONE);
@@ -2777,7 +2777,7 @@ void free_unref_folios(struct folio_batch *folios)
 
 			/*
 			 * Free isolated pages directly to the
-			 * allocator, see comment in free_unref_page.
+			 * allocator, see comment in free_frozen_pages.
 			 */
 			if (is_migrate_isolate(migratetype)) {
 				free_one_page(zone, &folio->page, pfn,
@@ -2808,7 +2808,7 @@ void free_unref_folios(struct folio_batch *folios)
 			migratetype = MIGRATE_MOVABLE;
 
 		trace_mm_page_free_batched(&folio->page);
-		free_unref_page_commit(zone, pcp, &folio->page, migratetype,
+		free_frozen_page_commit(zone, pcp, &folio->page, migratetype,
 				order);
 	}
 
@@ -4871,11 +4871,11 @@ void __free_pages(struct page *page, unsigned int order)
 	struct alloc_tag *tag = pgalloc_tag_get(page);
 
 	if (put_page_testzero(page))
-		free_unref_page(page, order);
+		free_frozen_pages(page, order);
 	else if (!head) {
 		pgalloc_tag_sub_pages(tag, (1 << order) - 1);
 		while (order-- > 0)
-			free_unref_page(page + (1 << order), order);
+			free_frozen_pages(page + (1 << order), order);
 	}
 }
 EXPORT_SYMBOL(__free_pages);
diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
index 3f7a203d35c6..d2423f30577e 100644
--- a/mm/page_frag_cache.c
+++ b/mm/page_frag_cache.c
@@ -86,7 +86,7 @@ void __page_frag_cache_drain(struct page *page, unsigned int count)
 	VM_BUG_ON_PAGE(page_ref_count(page) == 0, page);
 
 	if (page_ref_sub_and_test(page, count))
-		free_unref_page(page, compound_order(page));
+		free_frozen_pages(page, compound_order(page));
 }
 EXPORT_SYMBOL(__page_frag_cache_drain);
 
@@ -138,7 +138,7 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
 			goto refill;
 
 		if (unlikely(encoded_page_decode_pfmemalloc(encoded_page))) {
-			free_unref_page(page,
+			free_frozen_pages(page,
 					encoded_page_decode_order(encoded_page));
 			goto refill;
 		}
@@ -166,6 +166,6 @@ void page_frag_free(void *addr)
 	struct page *page = virt_to_head_page(addr);
 
 	if (unlikely(put_page_testzero(page)))
-		free_unref_page(page, compound_order(page));
+		free_frozen_pages(page, compound_order(page));
 }
 EXPORT_SYMBOL(page_frag_free);
diff --git a/mm/swap.c b/mm/swap.c
index 10decd9dffa1..3a01acfd5a89 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -109,7 +109,7 @@ void __folio_put(struct folio *folio)
 	page_cache_release(folio);
 	folio_unqueue_deferred_split(folio);
 	mem_cgroup_uncharge(folio);
-	free_unref_page(&folio->page, folio_order(folio));
+	free_frozen_pages(&folio->page, folio_order(folio));
 }
 EXPORT_SYMBOL(__folio_put);
 
-- 
2.45.2

Re: [PATCH v3 03/15] mm/page_alloc: Export free_frozen_pages() instead of free_unref_page()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> We already have the concept of "frozen pages" (eg page_ref_freeze()),
> so let's not complicate things by also having the concept of "unref
> pages".
>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: William Kucharski <william.kucharski@oracle.com>
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> Reviewed-by: Muchun Song <songmuchun@bytedance.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/internal.h        |  2 +-
>  mm/page_alloc.c      | 18 +++++++++---------
>  mm/page_frag_cache.c |  6 +++---
>  mm/swap.c            |  2 +-
>  4 files changed, 14 insertions(+), 14 deletions(-)

Shouldn't the name be free_frozen_page() instead of free_frozen_pages()?
It is still free a single page, right?

If free_frozen_pages() is what we want, free_frozen_page_commit()
should be free_frozen_pages_commit() to be consistent.

Otherwise, LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH v3 03/15] mm/page_alloc: Export free_frozen_pages() instead of free_unref_page()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> We already have the concept of "frozen pages" (eg page_ref_freeze()),
> so let's not complicate things by also having the concept of "unref
> pages".
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: William Kucharski <william.kucharski@oracle.com>
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> Reviewed-by: Muchun Song <songmuchun@bytedance.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

Yeah, free_frozen_pages_commit() would be more consistent, but not a big deal.

[PATCH v3 04/15] mm/page_alloc: Move set_page_refcounted() to callers of post_alloc_hook()

[Matthew Wilcox (Oracle)]

In preparation for allocating frozen pages, stop initialising
the page refcount in post_alloc_hook().

Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/compaction.c | 2 ++
 mm/internal.h   | 3 +--
 mm/page_alloc.c | 3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 6009f5d1021a..2915a13b34a5 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -83,6 +83,7 @@ static inline bool is_via_compact_memory(int order) { return false; }
 static struct page *mark_allocated_noprof(struct page *page, unsigned int order, gfp_t gfp_flags)
 {
 	post_alloc_hook(page, order, __GFP_MOVABLE);
+	set_page_refcounted(page);
 	return page;
 }
 #define mark_allocated(...)	alloc_hooks(mark_allocated_noprof(__VA_ARGS__))
@@ -1868,6 +1869,7 @@ static struct folio *compaction_alloc_noprof(struct folio *src, unsigned long da
 	dst = (struct folio *)freepage;
 
 	post_alloc_hook(&dst->page, order, __GFP_MOVABLE);
+	set_page_refcounted(&dst->page);
 	if (order)
 		prep_compound_page(&dst->page, order);
 	cc->nr_freepages -= 1 << order;
diff --git a/mm/internal.h b/mm/internal.h
index ca400c70199c..9cc5fdc614cf 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -735,8 +735,7 @@ static inline void prep_compound_tail(struct page *head, int tail_idx)
 
 extern void prep_compound_page(struct page *page, unsigned int order);
 
-extern void post_alloc_hook(struct page *page, unsigned int order,
-					gfp_t gfp_flags);
+void post_alloc_hook(struct page *page, unsigned int order, gfp_t gfp_flags);
 extern bool free_pages_prepare(struct page *page, unsigned int order);
 
 extern int user_min_free_kbytes;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index adac485e3254..e3a4aaf437f9 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1506,7 +1506,6 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
 	int i;
 
 	set_page_private(page, 0);
-	set_page_refcounted(page);
 
 	arch_alloc_page(page, order);
 	debug_pagealloc_map_pages(page, 1 << order);
@@ -1562,6 +1561,7 @@ static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags
 							unsigned int alloc_flags)
 {
 	post_alloc_hook(page, order, gfp_flags);
+	set_page_refcounted(page);
 
 	if (order && (gfp_flags & __GFP_COMP))
 		prep_compound_page(page, order);
@@ -6394,6 +6394,7 @@ static void split_free_pages(struct list_head *list)
 			int i;
 
 			post_alloc_hook(page, order, __GFP_MOVABLE);
+			set_page_refcounted(page);
 			if (!order)
 				continue;
 
-- 
2.45.2

Re: [PATCH v3 04/15] mm/page_alloc: Move set_page_refcounted() to callers of post_alloc_hook()

[David Hildenbrand]

On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
> In preparation for allocating frozen pages, stop initialising
> the page refcount in post_alloc_hook().
> 
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---

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

-- 
Cheers,

David / dhildenb

Re: [PATCH v3 04/15] mm/page_alloc: Move set_page_refcounted() to callers of post_alloc_hook()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> In preparation for allocating frozen pages, stop initialising
> the page refcount in post_alloc_hook().
>
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/compaction.c | 2 ++
>  mm/internal.h   | 3 +--
>  mm/page_alloc.c | 3 ++-
>  3 files changed, 5 insertions(+), 3 deletions(-)
>

Reviewed-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH v3 04/15] mm/page_alloc: Move set_page_refcounted() to callers of post_alloc_hook()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> In preparation for allocating frozen pages, stop initialising
> the page refcount in post_alloc_hook().
> 
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

[PATCH v3 05/15] mm/page_alloc: Move set_page_refcounted() to callers of prep_new_page()

[Matthew Wilcox (Oracle)]

In preparation for allocating frozen pages, stop initialising the page
refcount in prep_new_page().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/page_alloc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e3a4aaf437f9..c2b46cdc7ffd 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1561,7 +1561,6 @@ static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags
 							unsigned int alloc_flags)
 {
 	post_alloc_hook(page, order, gfp_flags);
-	set_page_refcounted(page);
 
 	if (order && (gfp_flags & __GFP_COMP))
 		prep_compound_page(page, order);
@@ -3508,6 +3507,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
 				gfp_mask, alloc_flags, ac->migratetype);
 		if (page) {
 			prep_new_page(page, order, gfp_mask, alloc_flags);
+			set_page_refcounted(page);
 
 			/*
 			 * If this is a high-order atomic allocation then check
@@ -3732,8 +3732,10 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
 	count_vm_event(COMPACTSTALL);
 
 	/* Prep a captured page if available */
-	if (page)
+	if (page) {
 		prep_new_page(page, order, gfp_mask, alloc_flags);
+		set_page_refcounted(page);
+	}
 
 	/* Try get a page from the freelist if available */
 	if (!page)
@@ -4712,6 +4714,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
 		nr_account++;
 
 		prep_new_page(page, 0, gfp, 0);
+		set_page_refcounted(page);
 		if (page_list)
 			list_add(&page->lru, page_list);
 		else
@@ -6534,6 +6537,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 
 		check_new_pages(head, order);
 		prep_new_page(head, order, gfp_mask, 0);
+		set_page_refcounted(head);
 	} else {
 		ret = -EINVAL;
 		WARN(true, "PFN range: requested [%lu, %lu), allocated [%lu, %lu)\n",
-- 
2.45.2

Re: [PATCH v3 05/15] mm/page_alloc: Move set_page_refcounted() to callers of prep_new_page()

[David Hildenbrand]

On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
> In preparation for allocating frozen pages, stop initialising the page
 > refcount in prep_new_page().>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---

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

-- 
Cheers,

David / dhildenb

Re: [PATCH v3 05/15] mm/page_alloc: Move set_page_refcounted() to callers of prep_new_page()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> In preparation for allocating frozen pages, stop initialising the page
> refcount in prep_new_page().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/page_alloc.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>

LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH v3 05/15] mm/page_alloc: Move set_page_refcounted() to callers of prep_new_page()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> In preparation for allocating frozen pages, stop initialising the page
> refcount in prep_new_page().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Can't hurt to only set refcount on a compound page after it's fully initialized.
But we do it without any memory barriers so it's probably the same, and
hopefully no speculative sanner is tricked by that before nor after.

Anyway,

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

[PATCH v3 06/15] mm/page_alloc: Move set_page_refcounted() to callers of get_page_from_freelist()

[Matthew Wilcox (Oracle)]

In preparation for allocating frozen pages, stop initialising the page
refcount in get_page_from_freelist().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/page_alloc.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c2b46cdc7ffd..14fa6bf7578a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3507,7 +3507,6 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
 				gfp_mask, alloc_flags, ac->migratetype);
 		if (page) {
 			prep_new_page(page, order, gfp_mask, alloc_flags);
-			set_page_refcounted(page);
 
 			/*
 			 * If this is a high-order atomic allocation then check
@@ -3602,6 +3601,8 @@ __alloc_pages_cpuset_fallback(gfp_t gfp_mask, unsigned int order,
 		page = get_page_from_freelist(gfp_mask, order,
 				alloc_flags, ac);
 
+	if (page)
+		set_page_refcounted(page);
 	return page;
 }
 
@@ -3640,8 +3641,10 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
 	page = get_page_from_freelist((gfp_mask | __GFP_HARDWALL) &
 				      ~__GFP_DIRECT_RECLAIM, order,
 				      ALLOC_WMARK_HIGH|ALLOC_CPUSET, ac);
-	if (page)
+	if (page) {
+		set_page_refcounted(page);
 		goto out;
+	}
 
 	/* Coredumps can quickly deplete all memory reserves */
 	if (current->flags & PF_DUMPCORE)
@@ -3732,10 +3735,8 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
 	count_vm_event(COMPACTSTALL);
 
 	/* Prep a captured page if available */
-	if (page) {
+	if (page)
 		prep_new_page(page, order, gfp_mask, alloc_flags);
-		set_page_refcounted(page);
-	}
 
 	/* Try get a page from the freelist if available */
 	if (!page)
@@ -3744,6 +3745,7 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
 	if (page) {
 		struct zone *zone = page_zone(page);
 
+		set_page_refcounted(page);
 		zone->compact_blockskip_flush = false;
 		compaction_defer_reset(zone, order, true);
 		count_vm_event(COMPACTSUCCESS);
@@ -4002,6 +4004,7 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask, unsigned int order,
 		drained = true;
 		goto retry;
 	}
+	set_page_refcounted(page);
 out:
 	psi_memstall_leave(&pflags);
 
@@ -4322,8 +4325,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 	 * that first
 	 */
 	page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac);
-	if (page)
+	if (page) {
+		set_page_refcounted(page);
 		goto got_pg;
+	}
 
 	/*
 	 * For costly allocations, try direct compaction first, as it's likely
@@ -4403,8 +4408,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 
 	/* Attempt with potentially adjusted zonelist and alloc_flags */
 	page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac);
-	if (page)
+	if (page) {
+		set_page_refcounted(page);
 		goto got_pg;
+	}
 
 	/* Caller is not willing to reclaim, we can't balance anything */
 	if (!can_direct_reclaim)
@@ -4788,8 +4795,10 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
 
 	/* First allocation attempt */
 	page = get_page_from_freelist(alloc_gfp, order, alloc_flags, &ac);
-	if (likely(page))
+	if (likely(page)) {
+		set_page_refcounted(page);
 		goto out;
+	}
 
 	alloc_gfp = gfp;
 	ac.spread_dirty_pages = false;
-- 
2.45.2

Re: [PATCH v3 06/15] mm/page_alloc: Move set_page_refcounted() to callers of get_page_from_freelist()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> In preparation for allocating frozen pages, stop initialising the page
> refcount in get_page_from_freelist().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/page_alloc.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
>
Reviewed-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH v3 06/15] mm/page_alloc: Move set_page_refcounted() to callers of get_page_from_freelist()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> In preparation for allocating frozen pages, stop initialising the page
> refcount in get_page_from_freelist().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

but there's a bug introduced, probably lasting only until a later patch

> @@ -4002,6 +4004,7 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask, unsigned int order,
>  		drained = true;
>  		goto retry;
>  	}
> +	set_page_refcounted(page);

this needs to be "if (page)", the "if" above isn't enough to rule NULL out.

>  out:
>  	psi_memstall_leave(&pflags);
>  

[PATCH v3 07/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_cpuset_fallback()

[Matthew Wilcox (Oracle)]

In preparation for allocating frozen pages, stop initialising the page
refcount in __alloc_pages_cpuset_fallback().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/page_alloc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 14fa6bf7578a..c9e5c69f0cb9 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3600,9 +3600,6 @@ __alloc_pages_cpuset_fallback(gfp_t gfp_mask, unsigned int order,
 	if (!page)
 		page = get_page_from_freelist(gfp_mask, order,
 				alloc_flags, ac);
-
-	if (page)
-		set_page_refcounted(page);
 	return page;
 }
 
@@ -3689,6 +3686,8 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
 		if (gfp_mask & __GFP_NOFAIL)
 			page = __alloc_pages_cpuset_fallback(gfp_mask, order,
 					ALLOC_NO_WATERMARKS, ac);
+		if (page)
+			set_page_refcounted(page);
 	}
 out:
 	mutex_unlock(&oom_lock);
@@ -4517,8 +4516,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 		 * the situation worse.
 		 */
 		page = __alloc_pages_cpuset_fallback(gfp_mask, order, ALLOC_MIN_RESERVE, ac);
-		if (page)
+		if (page) {
+			set_page_refcounted(page);
 			goto got_pg;
+		}
 
 		cond_resched();
 		goto retry;
-- 
2.45.2

Re: [PATCH v3 07/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_cpuset_fallback()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> In preparation for allocating frozen pages, stop initialising the page
> refcount in __alloc_pages_cpuset_fallback().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/page_alloc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>

Reviewed-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH v3 07/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_cpuset_fallback()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> In preparation for allocating frozen pages, stop initialising the page
> refcount in __alloc_pages_cpuset_fallback().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/page_alloc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 14fa6bf7578a..c9e5c69f0cb9 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3600,9 +3600,6 @@ __alloc_pages_cpuset_fallback(gfp_t gfp_mask, unsigned int order,
>  	if (!page)
>  		page = get_page_from_freelist(gfp_mask, order,
>  				alloc_flags, ac);
> -
> -	if (page)
> -		set_page_refcounted(page);
>  	return page;
>  }
>  
> @@ -3689,6 +3686,8 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
>  		if (gfp_mask & __GFP_NOFAIL)
>  			page = __alloc_pages_cpuset_fallback(gfp_mask, order,
>  					ALLOC_NO_WATERMARKS, ac);
> +		if (page)
> +			set_page_refcounted(page);
>  	}
>  out:
>  	mutex_unlock(&oom_lock);
> @@ -4517,8 +4516,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  		 * the situation worse.
>  		 */
>  		page = __alloc_pages_cpuset_fallback(gfp_mask, order, ALLOC_MIN_RESERVE, ac);
> -		if (page)
> +		if (page) {
> +			set_page_refcounted(page);
>  			goto got_pg;
> +		}
>  
>  		cond_resched();
>  		goto retry;

[PATCH v3 08/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_may_oom()

[Matthew Wilcox (Oracle)]

In preparation for allocating frozen pages, stop initialising the page
refcount in __alloc_pages_may_oom().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/page_alloc.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c9e5c69f0cb9..514994cd67b8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3638,10 +3638,8 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
 	page = get_page_from_freelist((gfp_mask | __GFP_HARDWALL) &
 				      ~__GFP_DIRECT_RECLAIM, order,
 				      ALLOC_WMARK_HIGH|ALLOC_CPUSET, ac);
-	if (page) {
-		set_page_refcounted(page);
+	if (page)
 		goto out;
-	}
 
 	/* Coredumps can quickly deplete all memory reserves */
 	if (current->flags & PF_DUMPCORE)
@@ -3686,8 +3684,6 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
 		if (gfp_mask & __GFP_NOFAIL)
 			page = __alloc_pages_cpuset_fallback(gfp_mask, order,
 					ALLOC_NO_WATERMARKS, ac);
-		if (page)
-			set_page_refcounted(page);
 	}
 out:
 	mutex_unlock(&oom_lock);
@@ -4471,8 +4467,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 
 	/* Reclaim has failed us, start killing things */
 	page = __alloc_pages_may_oom(gfp_mask, order, ac, &did_some_progress);
-	if (page)
+	if (page) {
+		set_page_refcounted(page);
 		goto got_pg;
+	}
 
 	/* Avoid allocations with no watermarks from looping endlessly */
 	if (tsk_is_oom_victim(current) &&
-- 
2.45.2

Re: [PATCH v3 08/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_may_oom()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> In preparation for allocating frozen pages, stop initialising the page
> refcount in __alloc_pages_may_oom().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/page_alloc.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>

Reviewed-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH v3 08/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_may_oom()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> In preparation for allocating frozen pages, stop initialising the page
> refcount in __alloc_pages_may_oom().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/page_alloc.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index c9e5c69f0cb9..514994cd67b8 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3638,10 +3638,8 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
>  	page = get_page_from_freelist((gfp_mask | __GFP_HARDWALL) &
>  				      ~__GFP_DIRECT_RECLAIM, order,
>  				      ALLOC_WMARK_HIGH|ALLOC_CPUSET, ac);
> -	if (page) {
> -		set_page_refcounted(page);
> +	if (page)
>  		goto out;
> -	}
>  
>  	/* Coredumps can quickly deplete all memory reserves */
>  	if (current->flags & PF_DUMPCORE)
> @@ -3686,8 +3684,6 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
>  		if (gfp_mask & __GFP_NOFAIL)
>  			page = __alloc_pages_cpuset_fallback(gfp_mask, order,
>  					ALLOC_NO_WATERMARKS, ac);
> -		if (page)
> -			set_page_refcounted(page);
>  	}
>  out:
>  	mutex_unlock(&oom_lock);
> @@ -4471,8 +4467,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  
>  	/* Reclaim has failed us, start killing things */
>  	page = __alloc_pages_may_oom(gfp_mask, order, ac, &did_some_progress);
> -	if (page)
> +	if (page) {
> +		set_page_refcounted(page);
>  		goto got_pg;
> +	}
>  
>  	/* Avoid allocations with no watermarks from looping endlessly */
>  	if (tsk_is_oom_victim(current) &&

[PATCH v3 09/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_direct_compact()

[Matthew Wilcox (Oracle)]

In preparation for allocating frozen pages, stop initialising the page
refcount in __alloc_pages_direct_compact().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/page_alloc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 514994cd67b8..0f02cb253bf5 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3740,7 +3740,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
 	if (page) {
 		struct zone *zone = page_zone(page);
 
-		set_page_refcounted(page);
 		zone->compact_blockskip_flush = false;
 		compaction_defer_reset(zone, order, true);
 		count_vm_event(COMPACTSUCCESS);
@@ -4342,8 +4341,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 						alloc_flags, ac,
 						INIT_COMPACT_PRIORITY,
 						&compact_result);
-		if (page)
+		if (page) {
+			set_page_refcounted(page);
 			goto got_pg;
+		}
 
 		/*
 		 * Checks for costly allocations with __GFP_NORETRY, which
@@ -4425,8 +4426,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 	/* Try direct compaction and then allocating */
 	page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags, ac,
 					compact_priority, &compact_result);
-	if (page)
+	if (page) {
+		set_page_refcounted(page);
 		goto got_pg;
+	}
 
 	/* Do not loop if specifically requested */
 	if (gfp_mask & __GFP_NORETRY)
-- 
2.45.2

Re: [PATCH v3 09/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_direct_compact()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> In preparation for allocating frozen pages, stop initialising the page
> refcount in __alloc_pages_direct_compact().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/page_alloc.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>

Reviewed-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH v3 09/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_direct_compact()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> In preparation for allocating frozen pages, stop initialising the page
> refcount in __alloc_pages_direct_compact().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/page_alloc.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 514994cd67b8..0f02cb253bf5 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3740,7 +3740,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
>  	if (page) {
>  		struct zone *zone = page_zone(page);
>  
> -		set_page_refcounted(page);
>  		zone->compact_blockskip_flush = false;
>  		compaction_defer_reset(zone, order, true);
>  		count_vm_event(COMPACTSUCCESS);
> @@ -4342,8 +4341,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  						alloc_flags, ac,
>  						INIT_COMPACT_PRIORITY,
>  						&compact_result);
> -		if (page)
> +		if (page) {
> +			set_page_refcounted(page);
>  			goto got_pg;
> +		}
>  
>  		/*
>  		 * Checks for costly allocations with __GFP_NORETRY, which
> @@ -4425,8 +4426,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  	/* Try direct compaction and then allocating */
>  	page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags, ac,
>  					compact_priority, &compact_result);
> -	if (page)
> +	if (page) {
> +		set_page_refcounted(page);
>  		goto got_pg;
> +	}
>  
>  	/* Do not loop if specifically requested */
>  	if (gfp_mask & __GFP_NORETRY)

[PATCH v3 10/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_direct_reclaim()

[Matthew Wilcox (Oracle)]

In preparation for allocating frozen pages, stop initialising the page
refcount in __alloc_pages_direct_reclaim().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/page_alloc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0f02cb253bf5..7acc32902fc9 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3998,7 +3998,6 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask, unsigned int order,
 		drained = true;
 		goto retry;
 	}
-	set_page_refcounted(page);
 out:
 	psi_memstall_leave(&pflags);
 
@@ -4420,8 +4419,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 	/* Try direct reclaim and then allocating */
 	page = __alloc_pages_direct_reclaim(gfp_mask, order, alloc_flags, ac,
 							&did_some_progress);
-	if (page)
+	if (page) {
+		set_page_refcounted(page);
 		goto got_pg;
+	}
 
 	/* Try direct compaction and then allocating */
 	page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags, ac,
-- 
2.45.2

Re: [PATCH v3 10/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_direct_reclaim()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> In preparation for allocating frozen pages, stop initialising the page
> refcount in __alloc_pages_direct_reclaim().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/page_alloc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Zi Yan <ziy@nvidia.com>


--
Best Regards,
Yan, Zi

Re: [PATCH v3 10/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_direct_reclaim()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> In preparation for allocating frozen pages, stop initialising the page
> refcount in __alloc_pages_direct_reclaim().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/page_alloc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0f02cb253bf5..7acc32902fc9 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3998,7 +3998,6 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask, unsigned int order,
>  		drained = true;
>  		goto retry;
>  	}
> -	set_page_refcounted(page);
>  out:
>  	psi_memstall_leave(&pflags);
>  
> @@ -4420,8 +4419,10 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  	/* Try direct reclaim and then allocating */
>  	page = __alloc_pages_direct_reclaim(gfp_mask, order, alloc_flags, ac,
>  							&did_some_progress);
> -	if (page)
> +	if (page) {
> +		set_page_refcounted(page);
>  		goto got_pg;
> +	}
>  
>  	/* Try direct compaction and then allocating */
>  	page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags, ac,

[PATCH v3 11/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_slowpath()

[Matthew Wilcox (Oracle)]

In preparation for allocating frozen pages, stop initialising the page
refcount in __alloc_pages_slowpath().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/page_alloc.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 7acc32902fc9..c219d2471408 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4318,10 +4318,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 	 * that first
 	 */
 	page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac);
-	if (page) {
-		set_page_refcounted(page);
+	if (page)
 		goto got_pg;
-	}
 
 	/*
 	 * For costly allocations, try direct compaction first, as it's likely
@@ -4340,10 +4338,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 						alloc_flags, ac,
 						INIT_COMPACT_PRIORITY,
 						&compact_result);
-		if (page) {
-			set_page_refcounted(page);
+		if (page)
 			goto got_pg;
-		}
 
 		/*
 		 * Checks for costly allocations with __GFP_NORETRY, which
@@ -4403,10 +4399,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 
 	/* Attempt with potentially adjusted zonelist and alloc_flags */
 	page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac);
-	if (page) {
-		set_page_refcounted(page);
+	if (page)
 		goto got_pg;
-	}
 
 	/* Caller is not willing to reclaim, we can't balance anything */
 	if (!can_direct_reclaim)
@@ -4419,18 +4413,14 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 	/* Try direct reclaim and then allocating */
 	page = __alloc_pages_direct_reclaim(gfp_mask, order, alloc_flags, ac,
 							&did_some_progress);
-	if (page) {
-		set_page_refcounted(page);
+	if (page)
 		goto got_pg;
-	}
 
 	/* Try direct compaction and then allocating */
 	page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags, ac,
 					compact_priority, &compact_result);
-	if (page) {
-		set_page_refcounted(page);
+	if (page)
 		goto got_pg;
-	}
 
 	/* Do not loop if specifically requested */
 	if (gfp_mask & __GFP_NORETRY)
@@ -4471,10 +4461,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 
 	/* Reclaim has failed us, start killing things */
 	page = __alloc_pages_may_oom(gfp_mask, order, ac, &did_some_progress);
-	if (page) {
-		set_page_refcounted(page);
+	if (page)
 		goto got_pg;
-	}
 
 	/* Avoid allocations with no watermarks from looping endlessly */
 	if (tsk_is_oom_victim(current) &&
@@ -4518,10 +4506,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 		 * the situation worse.
 		 */
 		page = __alloc_pages_cpuset_fallback(gfp_mask, order, ALLOC_MIN_RESERVE, ac);
-		if (page) {
-			set_page_refcounted(page);
+		if (page)
 			goto got_pg;
-		}
 
 		cond_resched();
 		goto retry;
@@ -4813,6 +4799,8 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
 	ac.nodemask = nodemask;
 
 	page = __alloc_pages_slowpath(alloc_gfp, order, &ac);
+	if (page)
+		set_page_refcounted(page);
 
 out:
 	if (memcg_kmem_online() && (gfp & __GFP_ACCOUNT) && page &&
-- 
2.45.2

Re: [PATCH v3 11/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_slowpath()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> In preparation for allocating frozen pages, stop initialising the page
> refcount in __alloc_pages_slowpath().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/page_alloc.c | 30 +++++++++---------------------
>  1 file changed, 9 insertions(+), 21 deletions(-)
>

Reviewed-by: Zi Yan <ziy@nvidia.com>
--
Best Regards,
Yan, Zi

Re: [PATCH v3 11/15] mm/page_alloc: Move set_page_refcounted() to callers of __alloc_pages_slowpath()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> In preparation for allocating frozen pages, stop initialising the page
> refcount in __alloc_pages_slowpath().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

[PATCH v3 12/15] mm/page_alloc: Move set_page_refcounted() to end of __alloc_pages()

[Matthew Wilcox (Oracle)]

Remove some code duplication by calling set_page_refcounted() at the
end of __alloc_pages() instead of after each call that can allocate
a page.  That means that we free a frozen page if we've exceeded the
allowed memcg memory.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/page_alloc.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c219d2471408..35fb45b8b369 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4784,10 +4784,8 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
 
 	/* First allocation attempt */
 	page = get_page_from_freelist(alloc_gfp, order, alloc_flags, &ac);
-	if (likely(page)) {
-		set_page_refcounted(page);
+	if (likely(page))
 		goto out;
-	}
 
 	alloc_gfp = gfp;
 	ac.spread_dirty_pages = false;
@@ -4799,15 +4797,15 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
 	ac.nodemask = nodemask;
 
 	page = __alloc_pages_slowpath(alloc_gfp, order, &ac);
-	if (page)
-		set_page_refcounted(page);
 
 out:
 	if (memcg_kmem_online() && (gfp & __GFP_ACCOUNT) && page &&
 	    unlikely(__memcg_kmem_charge_page(page, gfp, order) != 0)) {
-		__free_pages(page, order);
+		free_frozen_pages(page, order);
 		page = NULL;
 	}
+	if (page)
+		set_page_refcounted(page);
 
 	trace_mm_page_alloc(page, order, alloc_gfp, ac.migratetype);
 	kmsan_alloc_page(page, order, alloc_gfp);
-- 
2.45.2

Re: [PATCH v3 12/15] mm/page_alloc: Move set_page_refcounted() to end of __alloc_pages()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> Remove some code duplication by calling set_page_refcounted() at the
> end of __alloc_pages() instead of after each call that can allocate
> a page.  That means that we free a frozen page if we've exceeded the
> allowed memcg memory.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/page_alloc.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
Reviewed-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH v3 12/15] mm/page_alloc: Move set_page_refcounted() to end of __alloc_pages()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> Remove some code duplication by calling set_page_refcounted() at the
> end of __alloc_pages() instead of after each call that can allocate
> a page.  That means that we free a frozen page if we've exceeded the
> allowed memcg memory.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/page_alloc.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index c219d2471408..35fb45b8b369 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4784,10 +4784,8 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
>  
>  	/* First allocation attempt */
>  	page = get_page_from_freelist(alloc_gfp, order, alloc_flags, &ac);
> -	if (likely(page)) {
> -		set_page_refcounted(page);
> +	if (likely(page))
>  		goto out;
> -	}
>  
>  	alloc_gfp = gfp;
>  	ac.spread_dirty_pages = false;
> @@ -4799,15 +4797,15 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
>  	ac.nodemask = nodemask;
>  
>  	page = __alloc_pages_slowpath(alloc_gfp, order, &ac);
> -	if (page)
> -		set_page_refcounted(page);
>  
>  out:
>  	if (memcg_kmem_online() && (gfp & __GFP_ACCOUNT) && page &&
>  	    unlikely(__memcg_kmem_charge_page(page, gfp, order) != 0)) {
> -		__free_pages(page, order);
> +		free_frozen_pages(page, order);
>  		page = NULL;
>  	}
> +	if (page)
> +		set_page_refcounted(page);
>  
>  	trace_mm_page_alloc(page, order, alloc_gfp, ac.migratetype);
>  	kmsan_alloc_page(page, order, alloc_gfp);

[PATCH v3 13/15] mm/page_alloc: Add __alloc_frozen_pages()

[Matthew Wilcox (Oracle)]

Defer the initialisation of the page refcount to the new __alloc_pages()
wrapper and turn the old __alloc_pages() into __alloc_frozen_pages().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/internal.h   |  4 ++++
 mm/page_alloc.c | 18 ++++++++++++++----
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index 9cc5fdc614cf..55e03f8f41d9 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -740,6 +740,10 @@ extern bool free_pages_prepare(struct page *page, unsigned int order);
 
 extern int user_min_free_kbytes;
 
+struct page *__alloc_frozen_pages_noprof(gfp_t, unsigned int order, int nid,
+		nodemask_t *);
+#define __alloc_frozen_pages(...) \
+	alloc_hooks(__alloc_frozen_pages_noprof(__VA_ARGS__))
 void free_frozen_pages(struct page *page, unsigned int order);
 void free_unref_folios(struct folio_batch *fbatch);
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 35fb45b8b369..67b1cb757def 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4747,8 +4747,8 @@ EXPORT_SYMBOL_GPL(alloc_pages_bulk_noprof);
 /*
  * This is the 'heart' of the zoned buddy allocator.
  */
-struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
-				      int preferred_nid, nodemask_t *nodemask)
+struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
+		int preferred_nid, nodemask_t *nodemask)
 {
 	struct page *page;
 	unsigned int alloc_flags = ALLOC_WMARK_LOW;
@@ -4804,14 +4804,24 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
 		free_frozen_pages(page, order);
 		page = NULL;
 	}
-	if (page)
-		set_page_refcounted(page);
 
 	trace_mm_page_alloc(page, order, alloc_gfp, ac.migratetype);
 	kmsan_alloc_page(page, order, alloc_gfp);
 
 	return page;
 }
+EXPORT_SYMBOL(__alloc_frozen_pages_noprof);
+
+struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
+		int preferred_nid, nodemask_t *nodemask)
+{
+	struct page *page;
+
+	page = __alloc_frozen_pages_noprof(gfp, order, preferred_nid, nodemask);
+	if (page)
+		set_page_refcounted(page);
+	return page;
+}
 EXPORT_SYMBOL(__alloc_pages_noprof);
 
 struct folio *__folio_alloc_noprof(gfp_t gfp, unsigned int order, int preferred_nid,
-- 
2.45.2

Re: [PATCH v3 13/15] mm/page_alloc: Add __alloc_frozen_pages()

[David Hildenbrand]

On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
> Defer the initialisation of the page refcount to the new __alloc_pages()
> wrapper and turn the old __alloc_pages() into __alloc_frozen_pages().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   mm/internal.h   |  4 ++++
>   mm/page_alloc.c | 18 ++++++++++++++----
>   2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 9cc5fdc614cf..55e03f8f41d9 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -740,6 +740,10 @@ extern bool free_pages_prepare(struct page *page, unsigned int order);
>   
>   extern int user_min_free_kbytes;
>   
> +struct page *__alloc_frozen_pages_noprof(gfp_t, unsigned int order, int nid,
> +		nodemask_t *);
> +#define __alloc_frozen_pages(...) \
> +	alloc_hooks(__alloc_frozen_pages_noprof(__VA_ARGS__))
>   void free_frozen_pages(struct page *page, unsigned int order);
>   void free_unref_folios(struct folio_batch *fbatch);
>   
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 35fb45b8b369..67b1cb757def 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4747,8 +4747,8 @@ EXPORT_SYMBOL_GPL(alloc_pages_bulk_noprof);
>   /*
>    * This is the 'heart' of the zoned buddy allocator.
>    */
> -struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
> -				      int preferred_nid, nodemask_t *nodemask)
> +struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
> +		int preferred_nid, nodemask_t *nodemask)
>   {
>   	struct page *page;
>   	unsigned int alloc_flags = ALLOC_WMARK_LOW;
> @@ -4804,14 +4804,24 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
>   		free_frozen_pages(page, order);
>   		page = NULL;
>   	}
> -	if (page)
> -		set_page_refcounted(page);
>   
>   	trace_mm_page_alloc(page, order, alloc_gfp, ac.migratetype);
>   	kmsan_alloc_page(page, order, alloc_gfp);
>   
>   	return page;
>   }
> +EXPORT_SYMBOL(__alloc_frozen_pages_noprof);
> +
> +struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
> +		int preferred_nid, nodemask_t *nodemask)
> +{
> +	struct page *page;
> +
> +	page = __alloc_frozen_pages_noprof(gfp, order, preferred_nid, nodemask);
> +	if (page)
> +		set_page_refcounted(page);
> +	return page;
> +}
>   EXPORT_SYMBOL(__alloc_pages_noprof);
>   
>   struct folio *__folio_alloc_noprof(gfp_t gfp, unsigned int order, int preferred_nid,


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

-- 
Cheers,

David / dhildenb

Re: [PATCH v3 13/15] mm/page_alloc: Add __alloc_frozen_pages()

[Zi Yan]

On 25 Nov 2024, at 16:01, Matthew Wilcox (Oracle) wrote:

> Defer the initialisation of the page refcount to the new __alloc_pages()
> wrapper and turn the old __alloc_pages() into __alloc_frozen_pages().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/internal.h   |  4 ++++
>  mm/page_alloc.c | 18 ++++++++++++++----
>  2 files changed, 18 insertions(+), 4 deletions(-)
>
Reviewed-by: Zi Yan <ziy@nvidia.com>
--
Best Regards,
Yan, Zi

Re: [PATCH v3 13/15] mm/page_alloc: Add __alloc_frozen_pages()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> Defer the initialisation of the page refcount to the new __alloc_pages()
> wrapper and turn the old __alloc_pages() into __alloc_frozen_pages().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

But:

> @@ -4804,14 +4804,24 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
>  		free_frozen_pages(page, order);
>  		page = NULL;
>  	}
> -	if (page)
> -		set_page_refcounted(page);
>  
>  	trace_mm_page_alloc(page, order, alloc_gfp, ac.migratetype);
>  	kmsan_alloc_page(page, order, alloc_gfp);
>  
>  	return page;
>  }
> +EXPORT_SYMBOL(__alloc_frozen_pages_noprof);

Since there's no user, we should't export. If a user appears (I can imagine
e.g. KVM), it should be EXPORT_SYMBOL_GPL(). As should have been the rest of
symbols here, but oh well.

> +
> +struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
> +		int preferred_nid, nodemask_t *nodemask)
> +{
> +	struct page *page;
> +
> +	page = __alloc_frozen_pages_noprof(gfp, order, preferred_nid, nodemask);
> +	if (page)
> +		set_page_refcounted(page);
> +	return page;
> +}
>  EXPORT_SYMBOL(__alloc_pages_noprof);
>  
>  struct folio *__folio_alloc_noprof(gfp_t gfp, unsigned int order, int preferred_nid,

Re: [PATCH v3 13/15] mm/page_alloc: Add __alloc_frozen_pages()

[Andrew Morton]

On Wed, 4 Dec 2024 12:10:12 +0100 Vlastimil Babka <vbabka@suse.cz> wrote:

> > @@ -4804,14 +4804,24 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order,
> >  		free_frozen_pages(page, order);
> >  		page = NULL;
> >  	}
> > -	if (page)
> > -		set_page_refcounted(page);
> >  
> >  	trace_mm_page_alloc(page, order, alloc_gfp, ac.migratetype);
> >  	kmsan_alloc_page(page, order, alloc_gfp);
> >  
> >  	return page;
> >  }
> > +EXPORT_SYMBOL(__alloc_frozen_pages_noprof);
> 
> Since there's no user, we should't export. If a user appears (I can imagine
> e.g. KVM), it should be EXPORT_SYMBOL_GPL(). As should have been the rest of
> symbols here, but oh well.

I'll move this into mm-stable, but could we please address this at some
point?

[PATCH v3 14/15] mm/mempolicy: Add alloc_frozen_pages()

[Matthew Wilcox (Oracle)]

Provide an interface to allocate pages from the page allocator without
incrementing their refcount.  This saves an atomic operation on free,
which may be beneficial to some users (eg slab).

Reviewed-by: William Kucharski <william.kucharski@oracle.com>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/internal.h  | 12 ++++++++++++
 mm/mempolicy.c | 49 ++++++++++++++++++++++++++++++++-----------------
 2 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index 55e03f8f41d9..74713b44bedb 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -747,6 +747,18 @@ struct page *__alloc_frozen_pages_noprof(gfp_t, unsigned int order, int nid,
 void free_frozen_pages(struct page *page, unsigned int order);
 void free_unref_folios(struct folio_batch *fbatch);
 
+#ifdef CONFIG_NUMA
+struct page *alloc_frozen_pages_noprof(gfp_t, unsigned int order);
+#else
+static inline struct page *alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order)
+{
+	return __alloc_frozen_pages_noprof(gfp, order, numa_node_id(), NULL);
+}
+#endif
+
+#define alloc_frozen_pages(...) \
+	alloc_hooks(alloc_frozen_pages_noprof(__VA_ARGS__))
+
 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/mempolicy.c b/mm/mempolicy.c
index cda5f56085e6..3682184993dd 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2201,9 +2201,9 @@ static struct page *alloc_pages_preferred_many(gfp_t gfp, unsigned int order,
 	 */
 	preferred_gfp = gfp | __GFP_NOWARN;
 	preferred_gfp &= ~(__GFP_DIRECT_RECLAIM | __GFP_NOFAIL);
-	page = __alloc_pages_noprof(preferred_gfp, order, nid, nodemask);
+	page = __alloc_frozen_pages_noprof(preferred_gfp, order, nid, nodemask);
 	if (!page)
-		page = __alloc_pages_noprof(gfp, order, nid, NULL);
+		page = __alloc_frozen_pages_noprof(gfp, order, nid, NULL);
 
 	return page;
 }
@@ -2249,8 +2249,9 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
 			 * First, try to allocate THP only on local node, but
 			 * don't reclaim unnecessarily, just compact.
 			 */
-			page = __alloc_pages_node_noprof(nid,
-				gfp | __GFP_THISNODE | __GFP_NORETRY, order);
+			page = __alloc_frozen_pages_noprof(
+				gfp | __GFP_THISNODE | __GFP_NORETRY, order,
+				nid, NULL);
 			if (page || !(gfp & __GFP_DIRECT_RECLAIM))
 				return page;
 			/*
@@ -2262,7 +2263,7 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
 		}
 	}
 
-	page = __alloc_pages_noprof(gfp, order, nid, nodemask);
+	page = __alloc_frozen_pages_noprof(gfp, order, nid, nodemask);
 
 	if (unlikely(pol->mode == MPOL_INTERLEAVE) && page) {
 		/* skip NUMA_INTERLEAVE_HIT update if numa stats is disabled */
@@ -2280,8 +2281,13 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
 struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
 		struct mempolicy *pol, pgoff_t ilx, int nid)
 {
-	return page_rmappable_folio(alloc_pages_mpol(gfp | __GFP_COMP,
-							order, pol, ilx, nid));
+	struct page *page = alloc_pages_mpol(gfp | __GFP_COMP, order, pol,
+			ilx, nid);
+	if (!page)
+		return NULL;
+
+	set_page_refcounted(page);
+	return page_rmappable_folio(page);
 }
 
 /**
@@ -2316,6 +2322,21 @@ struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order, struct vm_area_struct
 }
 EXPORT_SYMBOL(vma_alloc_folio_noprof);
 
+struct page *alloc_frozen_pages_noprof(gfp_t gfp, unsigned order)
+{
+	struct mempolicy *pol = &default_policy;
+
+	/*
+	 * No reference counting needed for current->mempolicy
+	 * nor system default_policy
+	 */
+	if (!in_interrupt() && !(gfp & __GFP_THISNODE))
+		pol = get_task_policy(current);
+
+	return alloc_pages_mpol(gfp, order, pol, NO_INTERLEAVE_INDEX,
+				       numa_node_id());
+}
+
 /**
  * alloc_pages - Allocate pages.
  * @gfp: GFP flags.
@@ -2332,17 +2353,11 @@ EXPORT_SYMBOL(vma_alloc_folio_noprof);
  */
 struct page *alloc_pages_noprof(gfp_t gfp, unsigned int order)
 {
-	struct mempolicy *pol = &default_policy;
-
-	/*
-	 * No reference counting needed for current->mempolicy
-	 * nor system default_policy
-	 */
-	if (!in_interrupt() && !(gfp & __GFP_THISNODE))
-		pol = get_task_policy(current);
+	struct page *page = alloc_frozen_pages_noprof(gfp, order);
 
-	return alloc_pages_mpol(gfp, order, pol, NO_INTERLEAVE_INDEX,
-				       numa_node_id());
+	if (page)
+		set_page_refcounted(page);
+	return page;
 }
 EXPORT_SYMBOL(alloc_pages_noprof);
 
-- 
2.45.2

Re: [PATCH v3 14/15] mm/mempolicy: Add alloc_frozen_pages()

[David Hildenbrand]

On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
> Provide an interface to allocate pages from the page allocator without
> incrementing their refcount.  This saves an atomic operation on free,
> which may be beneficial to some users (eg slab).
> 
> Reviewed-by: William Kucharski <william.kucharski@oracle.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   mm/internal.h  | 12 ++++++++++++
>   mm/mempolicy.c | 49 ++++++++++++++++++++++++++++++++-----------------
>   2 files changed, 44 insertions(+), 17 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 55e03f8f41d9..74713b44bedb 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -747,6 +747,18 @@ struct page *__alloc_frozen_pages_noprof(gfp_t, unsigned int order, int nid,
>   void free_frozen_pages(struct page *page, unsigned int order);
>   void free_unref_folios(struct folio_batch *fbatch);
>   
> +#ifdef CONFIG_NUMA
> +struct page *alloc_frozen_pages_noprof(gfp_t, unsigned int order);
> +#else
> +static inline struct page *alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order)
> +{
> +	return __alloc_frozen_pages_noprof(gfp, order, numa_node_id(), NULL);
> +}
> +#endif
> +
> +#define alloc_frozen_pages(...) \
> +	alloc_hooks(alloc_frozen_pages_noprof(__VA_ARGS__))
> +
>   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/mempolicy.c b/mm/mempolicy.c
> index cda5f56085e6..3682184993dd 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2201,9 +2201,9 @@ static struct page *alloc_pages_preferred_many(gfp_t gfp, unsigned int order,
>   	 */
>   	preferred_gfp = gfp | __GFP_NOWARN;
>   	preferred_gfp &= ~(__GFP_DIRECT_RECLAIM | __GFP_NOFAIL);
> -	page = __alloc_pages_noprof(preferred_gfp, order, nid, nodemask);
> +	page = __alloc_frozen_pages_noprof(preferred_gfp, order, nid, nodemask);
>   	if (!page)
> -		page = __alloc_pages_noprof(gfp, order, nid, NULL);
> +		page = __alloc_frozen_pages_noprof(gfp, order, nid, NULL);
>   
>   	return page;
>   }
> @@ -2249,8 +2249,9 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>   			 * First, try to allocate THP only on local node, but
>   			 * don't reclaim unnecessarily, just compact.
>   			 */
> -			page = __alloc_pages_node_noprof(nid,
> -				gfp | __GFP_THISNODE | __GFP_NORETRY, order);
> +			page = __alloc_frozen_pages_noprof(
> +				gfp | __GFP_THISNODE | __GFP_NORETRY, order,
> +				nid, NULL);
>   			if (page || !(gfp & __GFP_DIRECT_RECLAIM))
>   				return page;
>   			/*
> @@ -2262,7 +2263,7 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>   		}
>   	}
>   
> -	page = __alloc_pages_noprof(gfp, order, nid, nodemask);
> +	page = __alloc_frozen_pages_noprof(gfp, order, nid, nodemask);
>   
>   	if (unlikely(pol->mode == MPOL_INTERLEAVE) && page) {
>   		/* skip NUMA_INTERLEAVE_HIT update if numa stats is disabled */
> @@ -2280,8 +2281,13 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>   struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
>   		struct mempolicy *pol, pgoff_t ilx, int nid)
>   {
> -	return page_rmappable_folio(alloc_pages_mpol(gfp | __GFP_COMP,
> -							order, pol, ilx, nid));
> +	struct page *page = alloc_pages_mpol(gfp | __GFP_COMP, order, pol,
> +			ilx, nid);
> +	if (!page)
> +		return NULL;
> +
> +	set_page_refcounted(page);
> +	return page_rmappable_folio(page);

What I don't quite like is that we now have a bit of an inconsistency, 
that makes it harder to understand what gives you frozen and what 
doesn't give you frozen pages.

alloc_pages(): non-frozen/refcounted
alloc_frozen_pages(): frozen
folio_alloc_mpol_noprof(): non-frozen/refcounted
alloc_pages_mpol(): ... frozen pages?


Ideally, it would be "alloc_pages": non-frozen, "alloc_frozen_pages": 
frozen.

The same concern applies to other internal functions, like 
"__alloc_pages_cpuset_fallback".


I'll note that that's one of the reasons I thought of having GFP_FROZEN 
instead (function name doesn't really matter). The only ugly thing with 
GFP_FROZEN is that we still need separate free*() functions. Well, or 
detect during free_*() that the refcount is 0 and assume that it was an 
frozen allocation ...

Anyhow, if we could make this more consistent, that would help.

-- 
Cheers,

David / dhildenb

Re: [PATCH v3 14/15] mm/mempolicy: Add alloc_frozen_pages()

[Zi Yan]

On 29 Nov 2024, at 9:44, David Hildenbrand wrote:

> On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
>> Provide an interface to allocate pages from the page allocator without
>> incrementing their refcount.  This saves an atomic operation on free,
>> which may be beneficial to some users (eg slab).
>>
>> Reviewed-by: William Kucharski <william.kucharski@oracle.com>
>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>> ---
>>   mm/internal.h  | 12 ++++++++++++
>>   mm/mempolicy.c | 49 ++++++++++++++++++++++++++++++++-----------------
>>   2 files changed, 44 insertions(+), 17 deletions(-)
>>
>> diff --git a/mm/internal.h b/mm/internal.h
>> index 55e03f8f41d9..74713b44bedb 100644
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -747,6 +747,18 @@ struct page *__alloc_frozen_pages_noprof(gfp_t, unsigned int order, int nid,
>>   void free_frozen_pages(struct page *page, unsigned int order);
>>   void free_unref_folios(struct folio_batch *fbatch);
>>  +#ifdef CONFIG_NUMA
>> +struct page *alloc_frozen_pages_noprof(gfp_t, unsigned int order);
>> +#else
>> +static inline struct page *alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order)
>> +{
>> +	return __alloc_frozen_pages_noprof(gfp, order, numa_node_id(), NULL);
>> +}
>> +#endif
>> +
>> +#define alloc_frozen_pages(...) \
>> +	alloc_hooks(alloc_frozen_pages_noprof(__VA_ARGS__))
>> +
>>   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/mempolicy.c b/mm/mempolicy.c
>> index cda5f56085e6..3682184993dd 100644
>> --- a/mm/mempolicy.c
>> +++ b/mm/mempolicy.c
>> @@ -2201,9 +2201,9 @@ static struct page *alloc_pages_preferred_many(gfp_t gfp, unsigned int order,
>>   	 */
>>   	preferred_gfp = gfp | __GFP_NOWARN;
>>   	preferred_gfp &= ~(__GFP_DIRECT_RECLAIM | __GFP_NOFAIL);
>> -	page = __alloc_pages_noprof(preferred_gfp, order, nid, nodemask);
>> +	page = __alloc_frozen_pages_noprof(preferred_gfp, order, nid, nodemask);
>>   	if (!page)
>> -		page = __alloc_pages_noprof(gfp, order, nid, NULL);
>> +		page = __alloc_frozen_pages_noprof(gfp, order, nid, NULL);
>>    	return page;
>>   }
>> @@ -2249,8 +2249,9 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>>   			 * First, try to allocate THP only on local node, but
>>   			 * don't reclaim unnecessarily, just compact.
>>   			 */
>> -			page = __alloc_pages_node_noprof(nid,
>> -				gfp | __GFP_THISNODE | __GFP_NORETRY, order);
>> +			page = __alloc_frozen_pages_noprof(
>> +				gfp | __GFP_THISNODE | __GFP_NORETRY, order,
>> +				nid, NULL);
>>   			if (page || !(gfp & __GFP_DIRECT_RECLAIM))
>>   				return page;
>>   			/*
>> @@ -2262,7 +2263,7 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>>   		}
>>   	}
>>  -	page = __alloc_pages_noprof(gfp, order, nid, nodemask);
>> +	page = __alloc_frozen_pages_noprof(gfp, order, nid, nodemask);
>>    	if (unlikely(pol->mode == MPOL_INTERLEAVE) && page) {
>>   		/* skip NUMA_INTERLEAVE_HIT update if numa stats is disabled */
>> @@ -2280,8 +2281,13 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>>   struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
>>   		struct mempolicy *pol, pgoff_t ilx, int nid)
>>   {
>> -	return page_rmappable_folio(alloc_pages_mpol(gfp | __GFP_COMP,
>> -							order, pol, ilx, nid));
>> +	struct page *page = alloc_pages_mpol(gfp | __GFP_COMP, order, pol,
>> +			ilx, nid);
>> +	if (!page)
>> +		return NULL;
>> +
>> +	set_page_refcounted(page);
>> +	return page_rmappable_folio(page);
>
> What I don't quite like is that we now have a bit of an inconsistency, that makes it harder to understand what gives you frozen and what doesn't give you frozen pages.
>
> alloc_pages(): non-frozen/refcounted
> alloc_frozen_pages(): frozen
> folio_alloc_mpol_noprof(): non-frozen/refcounted
> alloc_pages_mpol(): ... frozen pages?
>
>
> Ideally, it would be "alloc_pages": non-frozen, "alloc_frozen_pages": frozen.
>
> The same concern applies to other internal functions, like "__alloc_pages_cpuset_fallback".

Or all internal functions (with "__" prefix) give frozen pages and external
ones give non-frozen ones, unless with frozen in its name, i.e., alloc_frozen_pages()?


--
Best Regards,
Yan, Zi

Re: [PATCH v3 14/15] mm/mempolicy: Add alloc_frozen_pages()

[David Hildenbrand]

On 29.11.24 17:29, Zi Yan wrote:
> On 29 Nov 2024, at 9:44, David Hildenbrand wrote:
> 
>> On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
>>> Provide an interface to allocate pages from the page allocator without
>>> incrementing their refcount.  This saves an atomic operation on free,
>>> which may be beneficial to some users (eg slab).
>>>
>>> Reviewed-by: William Kucharski <william.kucharski@oracle.com>
>>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>>> ---
>>>    mm/internal.h  | 12 ++++++++++++
>>>    mm/mempolicy.c | 49 ++++++++++++++++++++++++++++++++-----------------
>>>    2 files changed, 44 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/mm/internal.h b/mm/internal.h
>>> index 55e03f8f41d9..74713b44bedb 100644
>>> --- a/mm/internal.h
>>> +++ b/mm/internal.h
>>> @@ -747,6 +747,18 @@ struct page *__alloc_frozen_pages_noprof(gfp_t, unsigned int order, int nid,
>>>    void free_frozen_pages(struct page *page, unsigned int order);
>>>    void free_unref_folios(struct folio_batch *fbatch);
>>>   +#ifdef CONFIG_NUMA
>>> +struct page *alloc_frozen_pages_noprof(gfp_t, unsigned int order);
>>> +#else
>>> +static inline struct page *alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order)
>>> +{
>>> +	return __alloc_frozen_pages_noprof(gfp, order, numa_node_id(), NULL);
>>> +}
>>> +#endif
>>> +
>>> +#define alloc_frozen_pages(...) \
>>> +	alloc_hooks(alloc_frozen_pages_noprof(__VA_ARGS__))
>>> +
>>>    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/mempolicy.c b/mm/mempolicy.c
>>> index cda5f56085e6..3682184993dd 100644
>>> --- a/mm/mempolicy.c
>>> +++ b/mm/mempolicy.c
>>> @@ -2201,9 +2201,9 @@ static struct page *alloc_pages_preferred_many(gfp_t gfp, unsigned int order,
>>>    	 */
>>>    	preferred_gfp = gfp | __GFP_NOWARN;
>>>    	preferred_gfp &= ~(__GFP_DIRECT_RECLAIM | __GFP_NOFAIL);
>>> -	page = __alloc_pages_noprof(preferred_gfp, order, nid, nodemask);
>>> +	page = __alloc_frozen_pages_noprof(preferred_gfp, order, nid, nodemask);
>>>    	if (!page)
>>> -		page = __alloc_pages_noprof(gfp, order, nid, NULL);
>>> +		page = __alloc_frozen_pages_noprof(gfp, order, nid, NULL);
>>>     	return page;
>>>    }
>>> @@ -2249,8 +2249,9 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>>>    			 * First, try to allocate THP only on local node, but
>>>    			 * don't reclaim unnecessarily, just compact.
>>>    			 */
>>> -			page = __alloc_pages_node_noprof(nid,
>>> -				gfp | __GFP_THISNODE | __GFP_NORETRY, order);
>>> +			page = __alloc_frozen_pages_noprof(
>>> +				gfp | __GFP_THISNODE | __GFP_NORETRY, order,
>>> +				nid, NULL);
>>>    			if (page || !(gfp & __GFP_DIRECT_RECLAIM))
>>>    				return page;
>>>    			/*
>>> @@ -2262,7 +2263,7 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>>>    		}
>>>    	}
>>>   -	page = __alloc_pages_noprof(gfp, order, nid, nodemask);
>>> +	page = __alloc_frozen_pages_noprof(gfp, order, nid, nodemask);
>>>     	if (unlikely(pol->mode == MPOL_INTERLEAVE) && page) {
>>>    		/* skip NUMA_INTERLEAVE_HIT update if numa stats is disabled */
>>> @@ -2280,8 +2281,13 @@ static struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
>>>    struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
>>>    		struct mempolicy *pol, pgoff_t ilx, int nid)
>>>    {
>>> -	return page_rmappable_folio(alloc_pages_mpol(gfp | __GFP_COMP,
>>> -							order, pol, ilx, nid));
>>> +	struct page *page = alloc_pages_mpol(gfp | __GFP_COMP, order, pol,
>>> +			ilx, nid);
>>> +	if (!page)
>>> +		return NULL;
>>> +
>>> +	set_page_refcounted(page);
>>> +	return page_rmappable_folio(page);
>>
>> What I don't quite like is that we now have a bit of an inconsistency, that makes it harder to understand what gives you frozen and what doesn't give you frozen pages.
>>
>> alloc_pages(): non-frozen/refcounted
>> alloc_frozen_pages(): frozen
>> folio_alloc_mpol_noprof(): non-frozen/refcounted
>> alloc_pages_mpol(): ... frozen pages?
>>
>>
>> Ideally, it would be "alloc_pages": non-frozen, "alloc_frozen_pages": frozen.
>>
>> The same concern applies to other internal functions, like "__alloc_pages_cpuset_fallback".
> 
> Or all internal functions (with "__" prefix) give frozen pages and external
> ones give non-frozen ones, unless with frozen in its name, i.e., alloc_frozen_pages()?

If we go that path, we should make it consistent for __free_pages() as 
well I am afraid.

-- 
Cheers,

David / dhildenb

Re: [PATCH v3 14/15] mm/mempolicy: Add alloc_frozen_pages()

[Vlastimil Babka]

On 11/29/24 18:18, David Hildenbrand wrote:
> On 29.11.24 17:29, Zi Yan wrote:
>> On 29 Nov 2024, at 9:44, David Hildenbrand wrote:
>> 
>>> On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
>>>
>>> What I don't quite like is that we now have a bit of an inconsistency, that makes it harder to understand what gives you frozen and what doesn't give you frozen pages.
>>>
>>> alloc_pages(): non-frozen/refcounted
>>> alloc_frozen_pages(): frozen
>>> folio_alloc_mpol_noprof(): non-frozen/refcounted
>>> alloc_pages_mpol(): ... frozen pages?
>>>
>>>
>>> Ideally, it would be "alloc_pages": non-frozen, "alloc_frozen_pages": frozen.

I don't mind it that much as long as it concerns only static functions. We
didn't add _frozen to all those in mm/page_alloc.c either.

The exported ones seem consistent, AFAICS?

>>> The same concern applies to other internal functions, like "__alloc_pages_cpuset_fallback".
>> 
>> Or all internal functions (with "__" prefix) give frozen pages and external
>> ones give non-frozen ones, unless with frozen in its name, i.e., alloc_frozen_pages()?
> 
> If we go that path, we should make it consistent for __free_pages() as 
> well I am afraid.

That one seems to have many callers so not worth the churn.

Re: [PATCH v3 14/15] mm/mempolicy: Add alloc_frozen_pages()

[David Hildenbrand]

On 04.12.24 12:34, Vlastimil Babka wrote:
> On 11/29/24 18:18, David Hildenbrand wrote:
>> On 29.11.24 17:29, Zi Yan wrote:
>>> On 29 Nov 2024, at 9:44, David Hildenbrand wrote:
>>>
>>>> On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
>>>>
>>>> What I don't quite like is that we now have a bit of an inconsistency, that makes it harder to understand what gives you frozen and what doesn't give you frozen pages.
>>>>
>>>> alloc_pages(): non-frozen/refcounted
>>>> alloc_frozen_pages(): frozen
>>>> folio_alloc_mpol_noprof(): non-frozen/refcounted
>>>> alloc_pages_mpol(): ... frozen pages?
>>>>
>>>>
>>>> Ideally, it would be "alloc_pages": non-frozen, "alloc_frozen_pages": frozen.
> 
> I don't mind it that much as long as it concerns only static functions. We
> didn't add _frozen to all those in mm/page_alloc.c either.
 > > The exported ones seem consistent, AFAICS?

alloc_frozen / free_frozen even only exists in mm/internal.h

So at least to the outside world it is consistent.

> 
>>>> The same concern applies to other internal functions, like "__alloc_pages_cpuset_fallback".
>>>
>>> Or all internal functions (with "__" prefix) give frozen pages and external
>>> ones give non-frozen ones, unless with frozen in its name, i.e., alloc_frozen_pages()?
>>
>> If we go that path, we should make it consistent for __free_pages() as
>> well I am afraid.
> 
> That one seems to have many callers so not worth the churn.

Agreed.

-- 
Cheers,

David / dhildenb

Re: [PATCH v3 14/15] mm/mempolicy: Add alloc_frozen_pages()

[Vlastimil Babka]

On 11/25/24 22:01, Matthew Wilcox (Oracle) wrote:
> Provide an interface to allocate pages from the page allocator without
> incrementing their refcount.  This saves an atomic operation on free,
> which may be beneficial to some users (eg slab).
> 
> Reviewed-by: William Kucharski <william.kucharski@oracle.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Naming aside,

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

[PATCH v3 15/15] slab: Allocate frozen pages

[Matthew Wilcox (Oracle)]

Since slab does not use the page refcount, it can allocate and
free frozen pages, saving one atomic operation per free.

Reviewed-by: William Kucharski <william.kucharski@oracle.com>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/slub.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 19980419b176..3d0c1cd6bdf5 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2405,9 +2405,9 @@ static inline struct slab *alloc_slab_page(gfp_t flags, int node,
 	unsigned int order = oo_order(oo);
 
 	if (node == NUMA_NO_NODE)
-		folio = (struct folio *)alloc_pages(flags, order);
+		folio = (struct folio *)alloc_frozen_pages(flags, order);
 	else
-		folio = (struct folio *)__alloc_pages_node(node, flags, order);
+		folio = (struct folio *)__alloc_frozen_pages(flags, order, node, NULL);
 
 	if (!folio)
 		return NULL;
@@ -2641,7 +2641,7 @@ static void __free_slab(struct kmem_cache *s, struct slab *slab)
 	__folio_clear_slab(folio);
 	mm_account_reclaimed_pages(pages);
 	unaccount_slab(slab, order, s);
-	__free_pages(&folio->page, order);
+	free_frozen_pages(&folio->page, order);
 }
 
 static void rcu_free_slab(struct rcu_head *h)
-- 
2.45.2

Re: [PATCH v3 15/15] slab: Allocate frozen pages

[David Hildenbrand]

On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
> Since slab does not use the page refcount, it can allocate and
> free frozen pages, saving one atomic operation per free.
> 
> Reviewed-by: William Kucharski <william.kucharski@oracle.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   mm/slub.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 19980419b176..3d0c1cd6bdf5 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2405,9 +2405,9 @@ static inline struct slab *alloc_slab_page(gfp_t flags, int node,
>   	unsigned int order = oo_order(oo);
>   
>   	if (node == NUMA_NO_NODE)
> -		folio = (struct folio *)alloc_pages(flags, order);
> +		folio = (struct folio *)alloc_frozen_pages(flags, order);
>   	else
> -		folio = (struct folio *)__alloc_pages_node(node, flags, order);
> +		folio = (struct folio *)__alloc_frozen_pages(flags, order, node, NULL);
>   
>   	if (!folio)
>   		return NULL;
> @@ -2641,7 +2641,7 @@ static void __free_slab(struct kmem_cache *s, struct slab *slab)
>   	__folio_clear_slab(folio);
>   	mm_account_reclaimed_pages(pages);
>   	unaccount_slab(slab, order, s);
> -	__free_pages(&folio->page, order);
> +	free_frozen_pages(&folio->page, order);
>   }
>   
>   static void rcu_free_slab(struct rcu_head *h)


I think we discussed in v1 or v2 that page isolation should be taught about that.

Likely we want something like:

diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 7e04047977cfe..7db2f79b39f0d 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -101,6 +101,8 @@ static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned long e
                  * because their page->_refcount is zero at all time.
                  */
                 if (!page_ref_count(page)) {
+                       if (PageSlab(page))
+                               return page;
                         if (PageBuddy(page))
                                 pfn += (1 << buddy_order(page)) - 1;
                         continue;

-- 
Cheers,

David / dhildenb

Re: [PATCH v3 15/15] slab: Allocate frozen pages

[Matthew Wilcox]

On Wed, Nov 27, 2024 at 04:07:01PM +0100, David Hildenbrand wrote:
> I think we discussed in v1 or v2 that page isolation should be taught about that.
> 
> Likely we want something like:
> 
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 7e04047977cfe..7db2f79b39f0d 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -101,6 +101,8 @@ static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned long e
>                  * because their page->_refcount is zero at all time.
>                  */
>                 if (!page_ref_count(page)) {
> +                       if (PageSlab(page))
> +                               return page;
>                         if (PageBuddy(page))
>                                 pfn += (1 << buddy_order(page)) - 1;

Ah, for order 0 slabs!  I got caught up thinking that slabs would be
caught by the earlier PageTransCompound() check.  But that's also a
bit messy since we know that __folio_test_movable() can sometimes appear
true on slab pages.  So I'm tempted to hoist this way up to between the
check for ZONE_MOVABLE and PageHuge.

Re: [PATCH v3 15/15] slab: Allocate frozen pages

[Vlastimil Babka]

On 11/27/24 16:52, Matthew Wilcox wrote:
> On Wed, Nov 27, 2024 at 04:07:01PM +0100, David Hildenbrand wrote:
>> I think we discussed in v1 or v2 that page isolation should be taught about that.
>> 
>> Likely we want something like:
>> 
>> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
>> index 7e04047977cfe..7db2f79b39f0d 100644
>> --- a/mm/page_isolation.c
>> +++ b/mm/page_isolation.c
>> @@ -101,6 +101,8 @@ static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned long e
>>                  * because their page->_refcount is zero at all time.
>>                  */
>>                 if (!page_ref_count(page)) {
>> +                       if (PageSlab(page))
>> +                               return page;
>>                         if (PageBuddy(page))
>>                                 pfn += (1 << buddy_order(page)) - 1;
> 
> Ah, for order 0 slabs!  I got caught up thinking that slabs would be
> caught by the earlier PageTransCompound() check.  But that's also a
> bit messy since we know that __folio_test_movable() can sometimes appear
> true on slab pages.  So I'm tempted to hoist this way up to between the
> check for ZONE_MOVABLE and PageHuge.

That should work. Are you going to try advancing by folio_nr_pages() in that
case as well?

Re: [PATCH v3 15/15] slab: Allocate frozen pages

[Vlastimil Babka]

On 12/4/24 15:43, Vlastimil Babka wrote:
> On 11/27/24 16:52, Matthew Wilcox wrote:
>> On Wed, Nov 27, 2024 at 04:07:01PM +0100, David Hildenbrand wrote:
>>> I think we discussed in v1 or v2 that page isolation should be taught about that.
>>> 
>>> Likely we want something like:
>>> 
>>> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
>>> index 7e04047977cfe..7db2f79b39f0d 100644
>>> --- a/mm/page_isolation.c
>>> +++ b/mm/page_isolation.c
>>> @@ -101,6 +101,8 @@ static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned long e
>>>                  * because their page->_refcount is zero at all time.
>>>                  */
>>>                 if (!page_ref_count(page)) {
>>> +                       if (PageSlab(page))
>>> +                               return page;
>>>                         if (PageBuddy(page))
>>>                                 pfn += (1 << buddy_order(page)) - 1;
>> 
>> Ah, for order 0 slabs!  I got caught up thinking that slabs would be
>> caught by the earlier PageTransCompound() check.  But that's also a
>> bit messy since we know that __folio_test_movable() can sometimes appear
>> true on slab pages.  So I'm tempted to hoist this way up to between the
>> check for ZONE_MOVABLE and PageHuge.
> 
> That should work. Are you going to try advancing by folio_nr_pages() in that
> case as well?

This seems also unresolved?

Re: [PATCH v3 00/15] Allocate and free frozen pages

[Hyeonggon Yoo]

On Tue, Nov 26, 2024 at 7:38 AM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
>
> Slab does not need to use the page refcount at all, and it can avoid
> an atomic operation on page free.  Hugetlb wants to delay setting the
> refcount until it has assembled a complete gigantic page. We already
> have the ability to freeze a page (safely reduce its reference count to
> 0), so this patchset adds APIs to allocate and free pages which are in
> a frozen state.

[+Cc Vlastimil]

I think with this patchset, we can finally remove the memory barriers used
in isolate_movable_page(), alloc_slab_page(), and __free_slab(),
because folio_get_nontail_page() should fail for frozen pages?

for reference, the patch that introduced the memory barrier:
https://lore.kernel.org/linux-mm/Y20BRJmRzRVMzoJw@hyeyoo

Best,
Hyeonggon

Re: [PATCH v3 00/15] Allocate and free frozen pages

[Vlastimil Babka]

On 11/26/24 06:04, Hyeonggon Yoo wrote:
> On Tue, Nov 26, 2024 at 7:38 AM Matthew Wilcox (Oracle)
> <willy@infradead.org> wrote:
>>
>> Slab does not need to use the page refcount at all, and it can avoid
>> an atomic operation on page free.  Hugetlb wants to delay setting the
>> refcount until it has assembled a complete gigantic page. We already
>> have the ability to freeze a page (safely reduce its reference count to
>> 0), so this patchset adds APIs to allocate and free pages which are in
>> a frozen state.
> 
> [+Cc Vlastimil]
> 
> I think with this patchset, we can finally remove the memory barriers used
> in isolate_movable_page(), alloc_slab_page(), and __free_slab(),
> because folio_get_nontail_page() should fail for frozen pages?

Thanks for the heads-up!

> for reference, the patch that introduced the memory barrier:
> https://lore.kernel.org/linux-mm/Y20BRJmRzRVMzoJw@hyeyoo

Are you going to sent the necessary patch on top of this series then? :) We
could route it via -mm to avoid waiting a cycle or needing extra
synchronization. It's not a pure slab change anyway...

Thanks,
Vlastimil

> Best,
> Hyeonggon

Re: [PATCH v3 00/15] Allocate and free frozen pages

[Hyeonggon Yoo]

On Thu, Dec 5, 2024 at 1:07 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 11/26/24 06:04, Hyeonggon Yoo wrote:
> > On Tue, Nov 26, 2024 at 7:38 AM Matthew Wilcox (Oracle)
> > <willy@infradead.org> wrote:
> >>
> >> Slab does not need to use the page refcount at all, and it can avoid
> >> an atomic operation on page free.  Hugetlb wants to delay setting the
> >> refcount until it has assembled a complete gigantic page. We already
> >> have the ability to freeze a page (safely reduce its reference count to
> >> 0), so this patchset adds APIs to allocate and free pages which are in
> >> a frozen state.
> >
> > [+Cc Vlastimil]
> >
> > I think with this patchset, we can finally remove the memory barriers used
> > in isolate_movable_page(), alloc_slab_page(), and __free_slab(),
> > because folio_get_nontail_page() should fail for frozen pages?
>
> Thanks for the heads-up!
>
> > for reference, the patch that introduced the memory barrier:
> > https://lore.kernel.org/linux-mm/Y20BRJmRzRVMzoJw@hyeyoo
>
> Are you going to sent the necessary patch on top of this series then? :)

I will send it after some testing.

> We could route it via -mm to avoid waiting a cycle or needing extra
> synchronization. It's not a pure slab change anyway...

Agreed. Will route it via mm.

Best,
Hyeonggon

>
> Thanks,
> Vlastimil
>
> > Best,
> > Hyeonggon
>