[PATCH 0/2] mm/page_alloc: couple of followups for recent cleanups

[PATCH 0/2] mm/page_alloc: couple of followups for recent cleanups

[Brendan Jackman]

These patches are technically orthogonal to each other I'm just sending
them as a series to avoid someone needing to deal with the (trivial)
conflicts.

Based on mm-new, depends on
https://lore.kernel.org/all/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com/

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
Brendan Jackman (2):
      mm/page_alloc: don't spin_trylock() in NMI on UP
      mm/page_alloc: rename FPI_TRYLOCK -> FPI_NOLOCK

 mm/page_alloc.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)
---
base-commit: 9d6a99520ac667ab6c1dbed364169b68d38f1a5c
change-id: 20260710-spin-trylock-followup-332c636e0d99

Best regards,
--  
Brendan Jackman <jackmanb@google.com>

[PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Brendan Jackman]

As noted in can_spin_trylock(), using this is unsafe in this context.
commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
but missed the free side.

Reported-by: sashiko-bot@kernel.org
Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com
Cc: stable@vger.kernel.org
Fixes: d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 mm/page_alloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 9c97a86da2b9f..5fe1c11f919d7 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2979,8 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
 		migratetype = MIGRATE_MOVABLE;
 	}
 
-	if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT)
-		     && (in_nmi() || in_hardirq()))) {
+	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
 		add_page_to_zone_llist(zone, page, order);
 		return;
 	}

-- 
2.54.0

Re: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Andrew Morton]

On Fri, 10 Jul 2026 10:42:20 +0000 Brendan Jackman <jackmanb@google.com> wrote:

> As noted in can_spin_trylock(), using this is unsafe in this context.
> commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
> alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
> but missed the free side.
> 
> Reported-by: sashiko-bot@kernel.org
> Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com
> Cc: stable@vger.kernel.org
> Fixes: d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")

Is this correct?  I'm not seeing anything in that commit which could
have caused this?

> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2979,8 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
>  		migratetype = MIGRATE_MOVABLE;
>  	}
>  
> -	if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT)
> -		     && (in_nmi() || in_hardirq()))) {
> +	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
>  		add_page_to_zone_llist(zone, page, order);
>  		return;
>  	}

It would be nice to include a description of the userspace impact.  I'm
suspecting that's "none known", but some speculation on what might
happen to someone is appropriate.

Also, please let's not combine a cc:stable bugfix with a minor macro
renaming.  They're very different things and will take quite different
paths into mainline and -stable kernels.

Also, Sashiko might have found yet more pre-existing issues:
	https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00@google.com

[PATCH 2/2] mm/page_alloc: rename FPI_TRYLOCK -> FPI_NOLOCK

[Brendan Jackman]

As discussed in the linked patch, the there is some inconsistency between
"trylock" and "nolock" nomenclature, let's align it. Since "nolock" is
used in the public API it seems to have more mindshare so do that.

The linked patch did this for the ALLOC_ flag but forgot about FPI_.

Link: https://lore.kernel.org/all/20260703-alloc-trylock-v5-1-c87b714e19d3@google.com/
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 mm/page_alloc.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5fe1c11f919d7..ba8d882072de5 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -90,7 +90,7 @@ typedef int __bitwise fpi_t;
 #define FPI_TO_TAIL		((__force fpi_t)BIT(1))
 
 /* Free the page without taking locks. Rely on trylock only. */
-#define FPI_TRYLOCK		((__force fpi_t)BIT(2))
+#define FPI_NOLOCK		((__force fpi_t)BIT(2))
 
 /* free_pages_prepare() has already been called for page(s) being freed. */
 #define FPI_PREPARED		((__force fpi_t)BIT(3))
@@ -1419,7 +1419,7 @@ static __always_inline bool __free_pages_prepare(struct page *page,
 	page_table_check_free(page, order);
 	pgalloc_tag_sub(page, 1 << order);
 
-	if (!PageHighMem(page) && !(fpi_flags & FPI_TRYLOCK)) {
+	if (!PageHighMem(page) && !(fpi_flags & FPI_NOLOCK)) {
 		debug_check_no_locks_freed(page_address(page),
 					   PAGE_SIZE << order);
 		debug_check_no_obj_freed(page_address(page),
@@ -1558,7 +1558,7 @@ static void free_one_page(struct zone *zone, struct page *page,
 	struct llist_head *llhead;
 	unsigned long flags;
 
-	if (unlikely(fpi_flags & FPI_TRYLOCK)) {
+	if (unlikely(fpi_flags & FPI_NOLOCK)) {
 		if (!spin_trylock_irqsave(&zone->lock, flags)) {
 			add_page_to_zone_llist(zone, page, order);
 			return;
@@ -1569,7 +1569,7 @@ static void free_one_page(struct zone *zone, struct page *page,
 
 	/* The lock succeeded. Process deferred pages. */
 	llhead = &zone->trylock_free_pages;
-	if (unlikely(!llist_empty(llhead) && !(fpi_flags & FPI_TRYLOCK))) {
+	if (unlikely(!llist_empty(llhead) && !(fpi_flags & FPI_NOLOCK))) {
 		struct llist_node *llnode;
 		struct page *p, *tmp;
 
@@ -2882,7 +2882,7 @@ static bool free_frozen_page_commit(struct zone *zone,
 	if (pcp->free_count < (batch << CONFIG_PCP_BATCH_SCALE_MAX))
 		pcp->free_count += (1 << order);
 
-	if (unlikely(fpi_flags & FPI_TRYLOCK)) {
+	if (unlikely(fpi_flags & FPI_NOLOCK)) {
 		/*
 		 * Do not attempt to take a zone lock. Let pcp->count get
 		 * over high mark temporarily.
@@ -2979,7 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
 		migratetype = MIGRATE_MOVABLE;
 	}
 
-	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
+	if (unlikely((fpi_flags & FPI_NOLOCK) && !can_spin_trylock())) {
 		add_page_to_zone_llist(zone, page, order);
 		return;
 	}
@@ -3001,7 +3001,7 @@ void free_frozen_pages(struct page *page, unsigned int order)
 
 void free_frozen_pages_nolock(struct page *page, unsigned int order)
 {
-	__free_frozen_pages(page, order, FPI_TRYLOCK);
+	__free_frozen_pages(page, order, FPI_NOLOCK);
 }
 
 /*
@@ -5409,7 +5409,7 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
 	if (memcg_kmem_online() && (gfp & __GFP_ACCOUNT) && page &&
 	    unlikely(__memcg_kmem_charge_page(page, gfp, order) != 0)) {
 		__free_frozen_pages(page, order,
-				    alloc_flags & ALLOC_NOLOCK ? FPI_TRYLOCK : 0);
+				    alloc_flags & ALLOC_NOLOCK ? FPI_NOLOCK : 0);
 		page = NULL;
 	}
 
@@ -5532,7 +5532,7 @@ EXPORT_SYMBOL(__free_pages);
  */
 void free_pages_nolock(struct page *page, unsigned int order)
 {
-	___free_pages(page, order, FPI_TRYLOCK);
+	___free_pages(page, order, FPI_NOLOCK);
 }
 
 /**

-- 
2.54.0

Re: [PATCH 2/2] mm/page_alloc: rename FPI_TRYLOCK -> FPI_NOLOCK

[Vlastimil Babka (SUSE)]

On 7/10/26 12:42, Brendan Jackman wrote:
> As discussed in the linked patch, the there is some inconsistency between
> "trylock" and "nolock" nomenclature, let's align it. Since "nolock" is
> used in the public API it seems to have more mindshare so do that.
> 
> The linked patch did this for the ALLOC_ flag but forgot about FPI_.
> 
> Link: https://lore.kernel.org/all/20260703-alloc-trylock-v5-1-c87b714e19d3@google.com/
> Signed-off-by: Brendan Jackman <jackmanb@google.com>

Naming things is hard. Maybe it should have all been called "nospin". I
don't know anymore :)
_nolock() functions and ALLOC_NOLOCK are part of API, FPI_ is internal so
it's not that urgent. Furthermore:

> ---
>  mm/page_alloc.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5fe1c11f919d7..ba8d882072de5 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -90,7 +90,7 @@ typedef int __bitwise fpi_t;
>  #define FPI_TO_TAIL		((__force fpi_t)BIT(1))
>  
>  /* Free the page without taking locks. Rely on trylock only. */

here's a "trylock"

> -#define FPI_TRYLOCK		((__force fpi_t)BIT(2))
> +#define FPI_NOLOCK		((__force fpi_t)BIT(2))

And here's not anymore.

>  
>  /* free_pages_prepare() has already been called for page(s) being freed. */
>  #define FPI_PREPARED		((__force fpi_t)BIT(3))
> @@ -1419,7 +1419,7 @@ static __always_inline bool __free_pages_prepare(struct page *page,
>  	page_table_check_free(page, order);
>  	pgalloc_tag_sub(page, 1 << order);
>  
> -	if (!PageHighMem(page) && !(fpi_flags & FPI_TRYLOCK)) {
> +	if (!PageHighMem(page) && !(fpi_flags & FPI_NOLOCK)) {
>  		debug_check_no_locks_freed(page_address(page),
>  					   PAGE_SIZE << order);
>  		debug_check_no_obj_freed(page_address(page),
> @@ -1558,7 +1558,7 @@ static void free_one_page(struct zone *zone, struct page *page,
>  	struct llist_head *llhead;
>  	unsigned long flags;
>  
> -	if (unlikely(fpi_flags & FPI_TRYLOCK)) {
> +	if (unlikely(fpi_flags & FPI_NOLOCK)) {
>  		if (!spin_trylock_irqsave(&zone->lock, flags)) {
>  			add_page_to_zone_llist(zone, page, order);
>  			return;
> @@ -1569,7 +1569,7 @@ static void free_one_page(struct zone *zone, struct page *page,
>  
>  	/* The lock succeeded. Process deferred pages. */
>  	llhead = &zone->trylock_free_pages;
> -	if (unlikely(!llist_empty(llhead) && !(fpi_flags & FPI_TRYLOCK))) {
> +	if (unlikely(!llist_empty(llhead) && !(fpi_flags & FPI_NOLOCK))) {
>  		struct llist_node *llnode;
>  		struct page *p, *tmp;
>  
> @@ -2882,7 +2882,7 @@ static bool free_frozen_page_commit(struct zone *zone,
>  	if (pcp->free_count < (batch << CONFIG_PCP_BATCH_SCALE_MAX))
>  		pcp->free_count += (1 << order);
>  
> -	if (unlikely(fpi_flags & FPI_TRYLOCK)) {
> +	if (unlikely(fpi_flags & FPI_NOLOCK)) {
>  		/*
>  		 * Do not attempt to take a zone lock. Let pcp->count get
>  		 * over high mark temporarily.
> @@ -2979,7 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
>  		migratetype = MIGRATE_MOVABLE;
>  	}
>  
> -	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
> +	if (unlikely((fpi_flags & FPI_NOLOCK) && !can_spin_trylock())) {

can_spin_trylock() was matched with FPI_TRYLOCK, now not anymore

>  		add_page_to_zone_llist(zone, page, order);
>  		return;
>  	}
> @@ -3001,7 +3001,7 @@ void free_frozen_pages(struct page *page, unsigned int order)
>  
>  void free_frozen_pages_nolock(struct page *page, unsigned int order)
>  {
> -	__free_frozen_pages(page, order, FPI_TRYLOCK);
> +	__free_frozen_pages(page, order, FPI_NOLOCK);
>  }
>  
>  /*
> @@ -5409,7 +5409,7 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
>  	if (memcg_kmem_online() && (gfp & __GFP_ACCOUNT) && page &&
>  	    unlikely(__memcg_kmem_charge_page(page, gfp, order) != 0)) {
>  		__free_frozen_pages(page, order,
> -				    alloc_flags & ALLOC_NOLOCK ? FPI_TRYLOCK : 0);
> +				    alloc_flags & ALLOC_NOLOCK ? FPI_NOLOCK : 0);

Although here it does improve things. Sigh.

>  		page = NULL;
>  	}
>  
> @@ -5532,7 +5532,7 @@ EXPORT_SYMBOL(__free_pages);
>   */
>  void free_pages_nolock(struct page *page, unsigned int order)
>  {
> -	___free_pages(page, order, FPI_TRYLOCK);
> +	___free_pages(page, order, FPI_NOLOCK);
>  }
>  
>  /**
> 

Re: [PATCH 2/2] mm/page_alloc: rename FPI_TRYLOCK -> FPI_NOLOCK

[Zi Yan]

On Fri Jul 10, 2026 at 8:40 AM EDT, Vlastimil Babka (SUSE) wrote:
> On 7/10/26 12:42, Brendan Jackman wrote:
>> As discussed in the linked patch, the there is some inconsistency between
>> "trylock" and "nolock" nomenclature, let's align it. Since "nolock" is
>> used in the public API it seems to have more mindshare so do that.
>> 
>> The linked patch did this for the ALLOC_ flag but forgot about FPI_.
>> 
>> Link: https://lore.kernel.org/all/20260703-alloc-trylock-v5-1-c87b714e19d3@google.com/
>> Signed-off-by: Brendan Jackman <jackmanb@google.com>
>
> Naming things is hard. Maybe it should have all been called "nospin". I
> don't know anymore :)
> _nolock() functions and ALLOC_NOLOCK are part of API, FPI_ is internal so
> it's not that urgent. Furthermore:

I had a similar concern when reading ALLOC_TRYLOCK -> ALLOC_NOLOCK[1],
since the name is _NOLOCK, but the comment says spin_trylock.

I agree that "nospin" is better and less confusing. But whether we want
to churn it again, TBD. :)

[1] https://lore.kernel.org/all/DJOZN5H048UX.1ZVSFD48QZN05@nvidia.com/
>
>> ---
>>  mm/page_alloc.c | 18 +++++++++---------
>>  1 file changed, 9 insertions(+), 9 deletions(-)
>> 
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 5fe1c11f919d7..ba8d882072de5 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -90,7 +90,7 @@ typedef int __bitwise fpi_t;
>>  #define FPI_TO_TAIL		((__force fpi_t)BIT(1))
>>  
>>  /* Free the page without taking locks. Rely on trylock only. */
>
> here's a "trylock"
>
>> -#define FPI_TRYLOCK		((__force fpi_t)BIT(2))
>> +#define FPI_NOLOCK		((__force fpi_t)BIT(2))
>
> And here's not anymore.
>
>>  
>>  /* free_pages_prepare() has already been called for page(s) being freed. */
>>  #define FPI_PREPARED		((__force fpi_t)BIT(3))
>> @@ -1419,7 +1419,7 @@ static __always_inline bool __free_pages_prepare(struct page *page,
>>  	page_table_check_free(page, order);
>>  	pgalloc_tag_sub(page, 1 << order);
>>  
>> -	if (!PageHighMem(page) && !(fpi_flags & FPI_TRYLOCK)) {
>> +	if (!PageHighMem(page) && !(fpi_flags & FPI_NOLOCK)) {
>>  		debug_check_no_locks_freed(page_address(page),
>>  					   PAGE_SIZE << order);
>>  		debug_check_no_obj_freed(page_address(page),
>> @@ -1558,7 +1558,7 @@ static void free_one_page(struct zone *zone, struct page *page,
>>  	struct llist_head *llhead;
>>  	unsigned long flags;
>>  
>> -	if (unlikely(fpi_flags & FPI_TRYLOCK)) {
>> +	if (unlikely(fpi_flags & FPI_NOLOCK)) {
>>  		if (!spin_trylock_irqsave(&zone->lock, flags)) {
>>  			add_page_to_zone_llist(zone, page, order);
>>  			return;
>> @@ -1569,7 +1569,7 @@ static void free_one_page(struct zone *zone, struct page *page,
>>  
>>  	/* The lock succeeded. Process deferred pages. */
>>  	llhead = &zone->trylock_free_pages;
>> -	if (unlikely(!llist_empty(llhead) && !(fpi_flags & FPI_TRYLOCK))) {
>> +	if (unlikely(!llist_empty(llhead) && !(fpi_flags & FPI_NOLOCK))) {
>>  		struct llist_node *llnode;
>>  		struct page *p, *tmp;
>>  
>> @@ -2882,7 +2882,7 @@ static bool free_frozen_page_commit(struct zone *zone,
>>  	if (pcp->free_count < (batch << CONFIG_PCP_BATCH_SCALE_MAX))
>>  		pcp->free_count += (1 << order);
>>  
>> -	if (unlikely(fpi_flags & FPI_TRYLOCK)) {
>> +	if (unlikely(fpi_flags & FPI_NOLOCK)) {
>>  		/*
>>  		 * Do not attempt to take a zone lock. Let pcp->count get
>>  		 * over high mark temporarily.
>> @@ -2979,7 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
>>  		migratetype = MIGRATE_MOVABLE;
>>  	}
>>  
>> -	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
>> +	if (unlikely((fpi_flags & FPI_NOLOCK) && !can_spin_trylock())) {
>
> can_spin_trylock() was matched with FPI_TRYLOCK, now not anymore
>
>>  		add_page_to_zone_llist(zone, page, order);
>>  		return;
>>  	}
>> @@ -3001,7 +3001,7 @@ void free_frozen_pages(struct page *page, unsigned int order)
>>  
>>  void free_frozen_pages_nolock(struct page *page, unsigned int order)
>>  {
>> -	__free_frozen_pages(page, order, FPI_TRYLOCK);
>> +	__free_frozen_pages(page, order, FPI_NOLOCK);
>>  }
>>  
>>  /*
>> @@ -5409,7 +5409,7 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
>>  	if (memcg_kmem_online() && (gfp & __GFP_ACCOUNT) && page &&
>>  	    unlikely(__memcg_kmem_charge_page(page, gfp, order) != 0)) {
>>  		__free_frozen_pages(page, order,
>> -				    alloc_flags & ALLOC_NOLOCK ? FPI_TRYLOCK : 0);
>> +				    alloc_flags & ALLOC_NOLOCK ? FPI_NOLOCK : 0);
>
> Although here it does improve things. Sigh.
>
>>  		page = NULL;
>>  	}
>>  
>> @@ -5532,7 +5532,7 @@ EXPORT_SYMBOL(__free_pages);
>>   */
>>  void free_pages_nolock(struct page *page, unsigned int order)
>>  {
>> -	___free_pages(page, order, FPI_TRYLOCK);
>> +	___free_pages(page, order, FPI_NOLOCK);
>>  }
>>  
>>  /**
>> 




-- 
Best Regards,
Yan, Zi