linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@kernel.org>
To: Waiman Long <llong@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Mike Rapoport <rppt@kernel.org>,
	Clark Williams <clrkwllms@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	linux-rt-devel@lists.linux.dev,
	Wei Yang <richard.weiyang@gmail.com>,
	David Hildenbrand <david@kernel.org>
Subject: Re: [PATCH] mm/mm_init: Don't call cond_resched() in deferred_init_memmap_chunk() if rcu_preempt_depth() set
Date: Wed, 21 Jan 2026 13:27:32 -0800	[thread overview]
Message-ID: <13d0b8b5-1ba7-4a3e-a686-13a7b993d471@paulmck-laptop> (raw)
In-Reply-To: <0e385146-67a3-4fdd-b119-059caba8c5f0@redhat.com>

On Wed, Jan 21, 2026 at 03:07:46PM -0500, Waiman Long wrote:
> On 1/21/26 2:43 PM, Andrew Morton wrote:
> > On Wed, 21 Jan 2026 14:10:36 -0500 Waiman Long<longman@redhat.com> wrote:
> > 
> > > Commit 3acb913c9d5b ("mm/mm_init: use deferred_init_memmap_chunk()
> > > in deferred_grow_zone()") made deferred_grow_zone() call
> > > deferred_init_memmap_chunk() within a pgdat_resize_lock() critical
> > > section with irqs disabled. It did check for irqs_disabled() in
> > > deferred_init_memmap_chunk() to avoid calling cond_resched(). For a
> > > PREEMPT_RT kernel build, however, spin_lock_irqsave() does not disable
> > > interrupt but rcu_read_lock() is called. This leads to the following
> > > bug report.
> > > 
> > >    BUG: sleeping function called from invalid context at mm/mm_init.c:2091
> > >    in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 1, name: swapper/0
> > >    preempt_count: 0, expected: 0
> > >    RCU nest depth: 1, expected: 0
> > >    3 locks held by swapper/0/1:
> > >     #0: ffff80008471b7a0 (sched_domains_mutex){+.+.}-{4:4}, at: sched_domains_mutex_lock+0x28/0x40
> > >     #1: ffff003bdfffef48 (&pgdat->node_size_lock){+.+.}-{3:3}, at: deferred_grow_zone+0x140/0x278
> > >     #2: ffff800084acf600 (rcu_read_lock){....}-{1:3}, at: rt_spin_lock+0x1b4/0x408
> > >    CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W           6.19.0-rc6-test #1 PREEMPT_{RT,(full)
> > > }
> > >    Tainted: [W]=WARN
> > >    Call trace:
> > >     show_stack+0x20/0x38 (C)
> > >     dump_stack_lvl+0xdc/0xf8
> > >     dump_stack+0x1c/0x28
> > >     __might_resched+0x384/0x530
> > >     deferred_init_memmap_chunk+0x560/0x688
> > >     deferred_grow_zone+0x190/0x278
> > >     _deferred_grow_zone+0x18/0x30
> > >     get_page_from_freelist+0x780/0xf78
> > >     __alloc_frozen_pages_noprof+0x1dc/0x348
> > >     alloc_slab_page+0x30/0x110
> > >     allocate_slab+0x98/0x2a0
> > >     new_slab+0x4c/0x80
> > >     ___slab_alloc+0x5a4/0x770
> > >     __slab_alloc.constprop.0+0x88/0x1e0
> > >     __kmalloc_node_noprof+0x2c0/0x598
> > >     __sdt_alloc+0x3b8/0x728
> > >     build_sched_domains+0xe0/0x1260
> > >     sched_init_domains+0x14c/0x1c8
> > >     sched_init_smp+0x9c/0x1d0
> > >     kernel_init_freeable+0x218/0x358
> > >     kernel_init+0x28/0x208
> > >     ret_from_fork+0x10/0x20
> > > 
> > > Fix it by checking rcu_preempt_depth() as well to prevent calling
> > > cond_resched(). Note that CONFIG_PREEMPT_RCU should always be enabled
> > > in a PREEMPT_RT kernel.
> > > 
> > > ...
> > > 
> > > --- a/mm/mm_init.c
> > > +++ b/mm/mm_init.c
> > > @@ -2085,7 +2085,12 @@ deferred_init_memmap_chunk(unsigned long start_pfn, unsigned long end_pfn,
> > >   			spfn = chunk_end;
> > > -			if (irqs_disabled())
> > > +			/*
> > > +			 * pgdat_resize_lock() only disables irqs in non-RT
> > > +			 * kernels but calls rcu_read_lock() in a PREEMPT_RT
> > > +			 * kernel.
> > > +			 */
> > > +			if (irqs_disabled() || rcu_preempt_depth())
> > >   				touch_nmi_watchdog();
> > rcu_preempt_depth() seems a fairly internal low-level thing - it's
> > rarely used.
> That is true. Beside the scheduler, workqueue also use rcu_preempt_depth().
> This API is included in "include/linux/rcupdate.h" which is included
> directly or indirectly by many kernel files. So even though it is rarely
> used, but it is still a public API.

It is a bit tricky, for example, given a kernel built with both
CONFIG_PREEMPT_NONE=y and CONFIG_PREEMPT_DYNAMIC=y, it will never
invoke touch_nmi_watchdog(), even if it really is in an RCU read-side
critical section.  This is because it was intended for lockdep-like use,
where (for example) you don't want to complain about sleeping in an RCU
read-side critical section unless you are 100% sure that you are in fact
in an RCU read-side critical section.

Maybe something like this?

	if (irqs_disabled() || !IS_ENABLED(CONFIG_PREEMPT_RCU) || rcu_preempt_depth())
		touch_nmi_watchdog();

This would *always* invoke touch_nmi_watchdog() for such kernels, which
might or might not be OK.

I freely confesss that I am not sure which of these is appropriate in
this setting.

							Thanx, Paul

> > Is there a more official way of detecting this condition?  Maybe even
> > #ifdef CONFIG_PREEMPT_RCU?
> > 
> I am not aware of a more official way of detecting this. Maybe Sebastian has
> some ideas. rcu_preempt_count() is defined whether CONFIG_PREEMPT_RCU is
> defined or not. So we don't need a "#ifdef CONFIG_PREEMPT_RCU". Maybe I
> should explicitly include "include/linux/rcupdate.h" in mm/mm_init.c just to
> be sure.
> 
> CONFIG_PREEMPT_RCU defaults to on if PREMPT_RT is set. With
> !CONFIG_PREEMPT_RCU, rcu_preempt_depth() is hard-coded to 0 and will be
> optimized out.
> 
> Cheers,
> Longman


  reply	other threads:[~2026-01-21 21:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-21 19:10 Waiman Long
2026-01-21 19:43 ` Andrew Morton
2026-01-21 20:07   ` Waiman Long
2026-01-21 21:27     ` Paul E. McKenney [this message]
2026-01-22  7:57       ` Sebastian Andrzej Siewior
2026-01-22  9:47         ` Mike Rapoport
2026-01-22 17:17         ` Paul E. McKenney
2026-01-22 17:59         ` Waiman Long

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=13d0b8b5-1ba7-4a3e-a686-13a7b993d471@paulmck-laptop \
    --to=paulmck@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bigeasy@linutronix.de \
    --cc=clrkwllms@kernel.org \
    --cc=david@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=llong@redhat.com \
    --cc=richard.weiyang@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=rppt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox