Re: [PATCH 9/9] mm/page_alloc: check pageblock pfns under span seqlock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: NMI deadlock and invalid wait context in zone_spans_pfn_stable()
--

commit 46b6bec2b8c6bc868d5bf37583c51e88e5fe1429
Author: Josef Bacik <josef@toxicpanda.com>

mm/page_alloc: check pageblock pfns under span seqlock

This patch adds zone_spans_pfn_stable() to read zone->zone_start_pfn and
zone->spanned_pages under zone->span_seqlock when checking pageblock PFNs in
get_pfnblock_bitmap_bitidx() under CONFIG_DEBUG_VM.

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 8f8885165045b..f7bb0d59efc81 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -357,6 +357,21 @@ static __always_inline bool is_standalone_pb_bit(enum pageblock_bits pb_bit)
>  	return pb_bit >= PB_compact_skip && pb_bit < __NR_PAGEBLOCK_BITS;
>  }
>  
> +/* Memory hotplug can resize the zone while we look at its span */
> +static bool __maybe_unused zone_spans_pfn_stable(struct zone *zone,
> +						 unsigned long pfn)
> +{
> +	unsigned int seq;
> +	bool ret;
> +
> +	do {
> +		seq = zone_span_seqbegin(zone);
> +		ret = zone_spans_pfn(zone, pfn);
> +	} while (zone_span_seqretry(zone, seq));
> +
> +	return ret;
> +}
> +
>  static __always_inline void
>  get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
>  			   unsigned long **bitmap_word, unsigned long *bitidx)
> @@ -370,7 +385,7 @@ get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
>  	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
>  #endif
>  	BUILD_BUG_ON(__MIGRATE_TYPE_END > PAGEBLOCK_MIGRATETYPE_MASK);
> -	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
> +	VM_BUG_ON_PAGE(!zone_spans_pfn_stable(page_zone(page), pfn), page);

[Severity: High]
Can calling zone_spans_pfn_stable() here cause an NMI deadlock or an invalid
wait context when freeing or allocating pages through the nolock paths with
CONFIG_DEBUG_VM and CONFIG_MEMORY_HOTPLUG enabled?

free_pages_nolock(), free_frozen_pages_nolock(), and alloc_pages_nolock() can
be called from NMI, hardirq, or while holding a raw_spinlock_t. On the free
path, __free_frozen_pages() calls get_pfnblock_migratetype() before checking
can_spin_trylock():

free_pages_nolock()
  ___free_pages()
    __free_frozen_pages()
      get_pfnblock_migratetype()
        __get_pfnblock_flags_mask()
          get_pfnblock_bitmap_bitidx()
            zone_spans_pfn_stable()
              zone_span_seqbegin()

In mm/page_alloc.c:__free_frozen_pages():
	zone = page_zone(page);
	migratetype = get_pfnblock_migratetype(page, pfn);
	if (unlikely(migratetype >= MIGRATE_PCPTYPES)) {
		if (unlikely(is_migrate_isolate(migratetype))) {
			free_one_page(zone, page, pfn, order, fpi_flags);
			return;
		}
		migratetype = MIGRATE_MOVABLE;
	}

	if (unlikely((fpi_flags & FPI_NOLOCK) && !can_spin_trylock())) {
		add_page_to_zone_llist(zone, page, order);
		return;
	}

On the write side, mm/memory_hotplug.c:zone_set_span() updates the span using
write_seqlock_irqsave(&zone->span_seqlock, flags), which disables regular
interrupts but cannot mask NMIs:

	write_seqlock_irqsave(&zone->span_seqlock, flags);
	zone->zone_start_pfn = start_pfn;
	zone->spanned_pages = spanned_pages;
	write_sequnlock_irqrestore(&zone->span_seqlock, flags);

This seems to introduce three regressions on the nolock paths:

1. If an NMI interrupts zone_set_span() while the sequence count is odd and
the NMI handler calls free_pages_nolock(), free_frozen_pages_nolock(), or
alloc_pages_nolock() on a page in that zone, read_seqbegin() in
zone_span_seqbegin() will spin forever waiting for the interrupted writer on
the same CPU.

2. With CONFIG_PROVE_LOCKING enabled, read_seqbegin() calls
seqcount_lockdep_reader_access(), which records a non-trylock lock_acquire()
on span_seqlock and triggers a lockdep inconsistent NMI usage splat once
zone_set_span() has used the lock.

3. On CONFIG_PREEMPT_RT, zone->span_seqlock is a seqlock_t backed by a
sleeping spinlock_t. If a concurrent writer is active (seq & 1),
read_seqbegin() takes spin_lock(&zone->span_seqlock.lock) via
__seqprop_spinlock_sequence(), attempting to acquire a sleeping lock from
NMI, hardirq, or raw_spinlock_t context.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-pgalloc-ai-hotfixes-v1-0-9c7d1aca801e@kernel.org?part=9