[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

[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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] FPI_NOLOCK frees can trigger unsafe spin_trylock in NMI on PREEMPT_RT via high-order or MIGRATE_ISOLATE bypass paths.
--

--- Patch [2]: [PATCH 2/2] mm/page_alloc: rename FPI_TRYLOCK -> FPI_NOLOCK ---
commit 57a8fbd6534fe3dfce75d671c889c4e1ae65159c
Author: Brendan Jackman <jackmanb@google.com>

mm/page_alloc: rename FPI_TRYLOCK -> FPI_NOLOCK
    
As discussed in the linked patch, there is some inconsistency between
trylock and nolock nomenclature, so this aligns it. Since nolock is
used in the public API, the FPI_ prefix is updated to match.
    
Link: https://lore.kernel.org/all/20260703-alloc-trylock-v5-1-c87b714e19d3@google.com/

> 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

[ ... ]

> @@ -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())) {

[Severity: High]
This is a pre-existing issue, but does this bypass create a risk of executing
spin_trylock_irqsave() in NMI context on PREEMPT_RT?

Looking at __free_frozen_pages() in mm/page_alloc.c, if a page's block has
concurrently been marked MIGRATE_ISOLATE (e.g., by CMA or compaction), it
takes this path just before the can_spin_trylock() check:

		if (unlikely(is_migrate_isolate(migratetype))) {
			free_one_page(zone, page, pfn, order, fpi_flags);
			return;
		}

Similarly, if the allocation order is greater than pcp_allowed_order(), it
immediately delegates to __free_pages_ok().

In both bypass paths, they skip the !can_spin_trylock() check entirely and
unconditionally call spin_trylock_irqsave(&zone->lock, flags) via
free_one_page().

Could this lead to a deadlock or kernel panic if a BPF program or other
NMI-context code calls free_pages_nolock() and hits one of these conditions?

>  		add_page_to_zone_llist(zone, page, order);
>  		return;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00@google.com?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `!can_spin_trylock()` safety check in `__free_frozen_pages()` is bypassed for allocations with order > `pcp_allowed_order()` and for isolated pages, leaving them vulnerable to unsafe lock acquisition and potential memory corruption on UP systems or deadlocks on PREEMPT_RT.
--

--- Patch [1]: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP ---
commit af22d8399ea9f942d20dfb24bc4e88e16f5fe2e1
Author: Brendan Jackman <jackmanb@google.com>

mm/page_alloc: don't spin_trylock() in NMI on UP
    
This commit updates the NMI and hardirq safety check in __free_frozen_pages()
to use can_spin_trylock(). It addresses an issue on UP systems where
spin_trylock() is unsafe in NMI context, which was missed on the free path.

Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com

> 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())) {

[Severity: High]
This isn't a bug introduced by this patch, but since the goal is to prevent
unsafe lock acquisition, does this check need to be moved earlier in
__free_frozen_pages() in mm/page_alloc.c?

There are early returns earlier in __free_frozen_pages() that bypass this
safety check completely. For example, for large orders:

    if (!pcp_allowed_order(order)) {
        __free_pages_ok(page, order, fpi_flags);
        return;
    }

And for isolated pages:

        if (unlikely(is_migrate_isolate(migratetype))) {
            free_one_page(zone, page, pfn, order, fpi_flags);
            return;
        }

Both of these paths eventually reach free_one_page() (around line 1561), which
unconditionally calls spin_trylock_irqsave() if FPI_TRYLOCK is set:

    if (unlikely(fpi_flags & FPI_TRYLOCK)) {
        if (!spin_trylock_irqsave(&zone->lock, flags)) {
            add_page_to_zone_llist(zone, page, order);
            return;
        }
    }

Could this lead to memory corruption on UP systems or deadlocks on PREEMPT_RT
if a high-order or isolated page is freed in NMI context via
free_frozen_pages_nolock() with FPI_TRYLOCK?

>  		add_page_to_zone_llist(zone, page, order);
>  		return;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00@google.com?part=1

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

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

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

[Brendan Jackman]

On Fri Jul 10, 2026 at 2:14 PM UTC, Zi Yan wrote:
> 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.

Yeah I do actually think "nolock" is a bad name here, it takes a lock.
But I never cared very much about the _bad_ ame. On the other hand
_inconsistent_ naming is a concrete problem IMO.

> 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/

Yeah I also dunno what's best here. I guess this is a decision for Vlastimil?

>>> ---
>>>  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"

This is accurate though. It's talking about the implementation which
calls spin_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

IMO can_spin_trylock() is matched with spin_trlock() while FPI_NOLOCK is
matched with ALLOC_NOLOCK which is matched with alloc_pages_nolock().

If you like, we could just drop the ALLOC_ and FPI_ renames and just
rename alloc_pages_nolock(). I steered away from that because "the
latter is public API", but... it's not like it would be a huge treewide
patch, it only has one user.

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

[Vlastimil Babka (SUSE)]

On 7/13/26 15:30, Brendan Jackman wrote:
> On Fri Jul 10, 2026 at 2:14 PM UTC, Zi Yan wrote:
>> 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.
> 
> Yeah I do actually think "nolock" is a bad name here, it takes a lock.
> But I never cared very much about the _bad_ ame. On the other hand
> _inconsistent_ naming is a concrete problem IMO.
> 
>> 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/
> 
> Yeah I also dunno what's best here. I guess this is a decision for Vlastimil?

Let's keep the FPI_NOLOCK rename then, but separately from the fix.

> 
> IMO can_spin_trylock() is matched with spin_trlock() while FPI_NOLOCK is
> matched with ALLOC_NOLOCK which is matched with alloc_pages_nolock().
> 
> If you like, we could just drop the ALLOC_ and FPI_ renames and just

ALLOC_NOLOCK is better to me than ALLOC_TRYLOCK.

> rename alloc_pages_nolock(). I steered away from that because "the
> latter is public API", but... it's not like it would be a huge treewide
> patch, it only has one user.

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

[Brendan Jackman]

On Sat Jul 11, 2026 at 12:03 AM UTC, Andrew Morton wrote:
> 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?

Oh yeah I guess it should be:

Fixes: 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")

This is confusing coz we have:

A: commit d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")
B: commit 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
...
X: commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from alloc_frozen_pages_nolock() in NMI on UP") 

X is marked as Fixing A, but it was an incomplete fix. I just copy
pasted the Fixes tag. But actually I'm now changing the free path that
was only introduced in B. 

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

Ack. I think if you trigger this bug by accident it will probably crash
your machine in extremely confusing ways. If you can trigger it
deliberately from unpriv (depends on the rest of the host setup, e.g.
what tracing is being used) you can probably use it to get root/ring0.
Can mention this in the commit message.

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

Ack. I assume by "combine" you mean put them in the same series - let
me know if I misunderstood that. Will separate them for v2.

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

There are 2 cases here:

1: !pcp_allowed_order() -> This is forbidden by alloc_order_allowed(),
   quite a bad miss from Sashiko IMO. Usually I expect AI to do better
   at spotting this kind of thing, makes me wonder if I'm the dumb one
   here.

2. pageblock is isolated -> Yeah this looks broken to me. 

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

[Harry Yoo]

[-- Attachment #1.1: Type: text/plain, Size: 3090 bytes --]



On 7/13/26 11:31 PM, Brendan Jackman wrote:
> On Sat Jul 11, 2026 at 12:03 AM UTC, Andrew Morton wrote:
>> 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.

Ouch, do we allow alloc_pages() -> free_pages_nolock()?
Didn't notice.

>>> 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?
> 
> Oh yeah I guess it should be:
> 
> Fixes: 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
> 
> This is confusing coz we have:
> 
> A: commit d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")
> B: commit 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
> ...
> X: commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from alloc_frozen_pages_nolock() in NMI on UP") 
> 
> X is marked as Fixing A, but it was an incomplete fix. I just copy
> pasted the Fixes tag. But actually I'm now changing the free path that
> was only introduced in B. 
> 
>>> --- 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.
> 
> Ack. I think if you trigger this bug by accident it will probably crash
> your machine in extremely confusing ways. If you can trigger it
> deliberately from unpriv (depends on the rest of the host setup, e.g.
> what tracing is being used) you can probably use it to get root/ring0.
> Can mention this in the commit message.

Just noting, it is quite niche because it requires UP, and tracing
something that can be called in NMI on UP is even nicher.

>> Also, Sashiko might have found yet more pre-existing issues:
>> 	https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00@google.com
> 
> There are 2 cases here:
> 
> 1: !pcp_allowed_order() -> This is forbidden by alloc_order_allowed(),
>    quite a bad miss from Sashiko IMO.

Hmm but alloc_order_allowed() would return true for !pcp_allowed_order()
when spinning is allowed, then pages can be freed via
free_pages_nolock()?

-- 
Cheers,
Harry / Hyeonggon

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Harry Yoo]

[-- Attachment #1.1: Type: text/plain, Size: 1412 bytes --]



On 7/10/26 11:14 PM, Zi Yan wrote:
> 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/

Agreed, "nospin" is a better name!

At some point I realized "NOLOCK? well, we're using locks, but without
spinning. should be NOSPIN?", then thought it's bit late to change the
name without churn :)

-- 
Cheers,
Harry / Hyeonggon


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]