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

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 <[email protected]>

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/[email protected]/

> 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/[email protected]?part=2