Re: [RFC PATCH v2 2/5] mm/show_mem: Avoid blocking on zone->lock during Oops or panic

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/show_mem: false lock contention warning in show_free_areas()
--

commit e1c6e11316c0b99b2f700b8251c5ed2b76aad84a
Author: Aaron Tomlin <atomlin@atomlin.com>

mm/show_mem: Avoid blocking on zone->lock during Oops or panic

This commit updates show_free_areas() in mm/show_mem.c to avoid unconditionally calling spin_lock_irqsave() on zone->lock during Oops, panic, or NMI/atomic contexts, skipping zones when acquiring the lock is unsafe or contended.

> diff --git a/mm/show_mem.c b/mm/show_mem.c
> index b938cbcd774a1..77098077523a9 100644
> --- a/mm/show_mem.c
> +++ b/mm/show_mem.c
> @@ -367,10 +367,17 @@ static void show_free_areas(unsigned int filter, const nodemask_t *nodemask,
>  			continue;
>  		if (show_mem_node_skip(filter, zone_to_nid(zone), nodemask))
>  			continue;
> -		show_node(zone);
> -		printk(KERN_CONT "%s: ", zone->name);
>  
> -		spin_lock_irqsave(&zone->lock, flags);
> +		if (!can_spin_trylock() || in_nmi() ||
> +		    oops_in_progress || panic_in_progress()) {
> +			if (!can_spin_trylock() ||
> +			    !spin_trylock_irqsave(&zone->lock, flags)) {
> +				pr_warn("zone %s lock contended\n", zone->name);
> +				continue;
> +			}
> +		} else {
> +			spin_lock_irqsave(&zone->lock, flags);
> +		}

[Severity: Medium]
In show_free_areas(), when can_spin_trylock() returns false (such as in NMI or
hardirq context under CONFIG_PREEMPT_RT, or in NMI context under !CONFIG_SMP):

mm/internal.h:can_spin_trylock() {
	if (IS_ENABLED(CONFIG_PREEMPT_RT) && (in_nmi() || in_hardirq()))
		return false;

	/* On UP, spin_trylock() always succeeds even when it is locked */
	if (!IS_ENABLED(CONFIG_SMP) && in_nmi())
		return false;

	return true;
}

the inner !can_spin_trylock() check short-circuits before
spin_trylock_irqsave() is ever called, yet still prints "zone %s lock
contended" for every populated zone even when zone->lock is completely
uncontended.

The commit message also states:

  Check can_spin_trylock() and use spin_trylock_irqsave() when
  !can_spin_trylock(), in_nmi(), oops_in_progress, or panic_in_progress()
  is active

Additionally, because show_node(zone) is moved after spin_unlock_irqrestore()
below, skipping a zone here only logs zone->name (for example, Normal or
DMA32) without the NUMA node ID on CONFIG_NUMA systems.

Should the !can_spin_trylock() case avoid claiming that zone->lock is
contended (and match the commit message description), and should the skipped
zone message include the node ID?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103024453.922373-1-atomlin@atomlin.com?part=2