[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aXHyM1R18GI7-gSD@kernel.org>
Date: Thu, 22 Jan 2026 11:47:31 +0200
From: Mike Rapoport <rppt@...nel.org>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: "Paul E. McKenney" <paulmck@...nel.org>, Waiman Long <llong@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Clark Williams <clrkwllms@...nel.org>,
Steven Rostedt <rostedt@...dmis.org>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
Wei Yang <richard.weiyang@...il.com>,
David Hildenbrand <david@...nel.org>
Subject: Re: [PATCH] mm/mm_init: Don't call cond_resched() in
deferred_init_memmap_chunk() if rcu_preempt_depth() set
On Thu, Jan 22, 2026 at 08:57:47AM +0100, Sebastian Andrzej Siewior wrote:
> On 2026-01-21 13:27:32 [-0800], Paul E. McKenney wrote:
> >
> > 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();
>
> I don't understand the PREEMPT_NONE+DYNAMIC reasoning. irqs_disabled()
> should not be affected by this and rcu_preempt_depth() will be 0 for
> !CONFIG_PREEMPT_RCU so I don't think this is required.
>
> > 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.
>
> What about a more straight forward and obvious approach?
>
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index fc2a6f1e518f1..0b283fd48b282 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -2059,7 +2059,7 @@ static unsigned long __init deferred_init_pages(struct zone *zone,
> */
> static unsigned long __init
> deferred_init_memmap_chunk(unsigned long start_pfn, unsigned long end_pfn,
> - struct zone *zone)
> + struct zone *zone, bool may_schedule)
> {
> int nid = zone_to_nid(zone);
> unsigned long nr_pages = 0;
> @@ -2085,10 +2085,10 @@ deferred_init_memmap_chunk(unsigned long start_pfn, unsigned long end_pfn,
>
> spfn = chunk_end;
>
> - if (irqs_disabled())
> - touch_nmi_watchdog();
> - else
> + if (may_schedule)
> cond_resched();
> + else
> + touch_nmi_watchdog();
> }
> }
>
> @@ -2101,7 +2101,7 @@ deferred_init_memmap_job(unsigned long start_pfn, unsigned long end_pfn,
> {
> struct zone *zone = arg;
>
> - deferred_init_memmap_chunk(start_pfn, end_pfn, zone);
> + deferred_init_memmap_chunk(start_pfn, end_pfn, zone, true);
> }
>
> static unsigned int __init
> @@ -2216,7 +2216,7 @@ bool __init deferred_grow_zone(struct zone *zone, unsigned int order)
> for (spfn = first_deferred_pfn, epfn = SECTION_ALIGN_UP(spfn + 1);
> nr_pages < nr_pages_needed && spfn < zone_end_pfn(zone);
> spfn = epfn, epfn += PAGES_PER_SECTION) {
> - nr_pages += deferred_init_memmap_chunk(spfn, epfn, zone);
> + nr_pages += deferred_init_memmap_chunk(spfn, epfn, zone, false);
> }
>
> /*
>
> Wouldn't this work?
Yes, it will. And I think this is less fragile and clearer.
> Sebastian
>
--
Sincerely yours,
Mike.
Powered by blists - more mailing lists