From: Phil Auld <pauld@redhat.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Frederic Weisbecker <frederic@kernel.org>,
Nicholas Piggin <npiggin@gmail.com>,
Leonardo Bras <leobras@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>, Tejun Heo <tj@kernel.org>,
Lai Jiangshan <jiangshanlai@gmail.com>,
linux-kernel@vger.kernel.org, Junyao Zhao <junzhao@redhat.com>,
Chris von Recklinghausen <crecklin@redhat.com>
Subject: Re: [PATCH] sched/isolation: fix boot crash when the boot CPU is nohz_full
Date: Wed, 24 Apr 2024 10:42:13 -0400 [thread overview]
Message-ID: <20240424144213.GA5682@lorien.usersys.redhat.com> (raw)
In-Reply-To: <20240422184934.GC32422@redhat.com>
HI Oleg, Ingo,
On Mon, Apr 22, 2024 at 08:50:21PM +0200 Oleg Nesterov wrote:
> Phil, Frederic,
>
> Thanks for your review! Who do you think can take these patches?
> At least the 1st one.
>
> To remind, there are more problems with boot CPU in nohz mask, but
> can we at least fix the kernel crash?
>
I think that would be good. I don't know if Peter is at full strength.
Ingo could you take look at this, please?
Cheers,
Phil
> Oleg.
>
> On 04/18, Phil Auld wrote:
> >
> > On Thu, Apr 11, 2024 at 04:39:05PM +0200 Oleg Nesterov wrote:
> > > Documentation/timers/no_hz.rst states that the "nohz_full=" mask must not
> > > include the boot CPU, this is no longer true after the commit 08ae95f4fd3b
> > > ("nohz_full: Allow the boot CPU to be nohz_full").
> > >
> > > However after another commit aae17ebb53cd ("workqueue: Avoid using isolated
> > > cpus' timers on queue_delayed_work") the kernel will crash at boot time in
> > > this case; housekeeping_any_cpu() returns an invalid cpu nr until smp_init()
> > > paths bring the 1st housekeeping CPU up.
> > >
> > > Change housekeeping_any_cpu() to check the result of cpumask_any_and() and
> > > return smp_processor_id() in this case. Yes, this is just the simple and
> > > backportable workaround which fixes the symptom, but smp_processor_id() at
> > > boot time should be safe at least for type == HK_TYPE_TIMER, this more or
> > > less matches the tick_do_timer_boot_cpu logic.
> > >
> > > We should not worry about cpu_down(); tick_nohz_cpu_down() will not allow
> > > to offline tick_do_timer_cpu (the 1st online housekeeping CPU).
> > >
> > > Fixes: aae17ebb53cd ("workqueue: Avoid using isolated cpus' timers on queue_delayed_work")
> > > Reported-by: Chris von Recklinghausen <crecklin@redhat.com>
> > > Closes: https://lore.kernel.org/all/20240402105847.GA24832@redhat.com/
> > > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> >
> > Checking the returned value instead of assuming seems safer in any case.
> >
> > Reviewed-by: Phil Auld <pauld@redhat.com>
> >
> > > ---
> > > Documentation/timers/no_hz.rst | 7 ++-----
> > > kernel/sched/isolation.c | 11 ++++++++++-
> > > 2 files changed, 12 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/Documentation/timers/no_hz.rst b/Documentation/timers/no_hz.rst
> > > index f8786be15183..7fe8ef9718d8 100644
> > > --- a/Documentation/timers/no_hz.rst
> > > +++ b/Documentation/timers/no_hz.rst
> > > @@ -129,11 +129,8 @@ adaptive-tick CPUs: At least one non-adaptive-tick CPU must remain
> > > online to handle timekeeping tasks in order to ensure that system
> > > calls like gettimeofday() returns accurate values on adaptive-tick CPUs.
> > > (This is not an issue for CONFIG_NO_HZ_IDLE=y because there are no running
> > > -user processes to observe slight drifts in clock rate.) Therefore, the
> > > -boot CPU is prohibited from entering adaptive-ticks mode. Specifying a
> > > -"nohz_full=" mask that includes the boot CPU will result in a boot-time
> > > -error message, and the boot CPU will be removed from the mask. Note that
> > > -this means that your system must have at least two CPUs in order for
> > > +user processes to observe slight drifts in clock rate.) Note that this
> > > +means that your system must have at least two CPUs in order for
> > > CONFIG_NO_HZ_FULL=y to do anything for you.
> > >
> > > Finally, adaptive-ticks CPUs must have their RCU callbacks offloaded.
> > > diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> > > index 373d42c707bc..2a262d3ecb3d 100644
> > > --- a/kernel/sched/isolation.c
> > > +++ b/kernel/sched/isolation.c
> > > @@ -46,7 +46,16 @@ int housekeeping_any_cpu(enum hk_type type)
> > > if (cpu < nr_cpu_ids)
> > > return cpu;
> > >
> > > - return cpumask_any_and(housekeeping.cpumasks[type], cpu_online_mask);
> > > + cpu = cpumask_any_and(housekeeping.cpumasks[type], cpu_online_mask);
> > > + if (likely(cpu < nr_cpu_ids))
> > > + return cpu;
> > > + /*
> > > + * Unless we have another problem this can only happen
> > > + * at boot time before start_secondary() brings the 1st
> > > + * housekeeping CPU up.
> > > + */
> > > + WARN_ON_ONCE(system_state == SYSTEM_RUNNING ||
> > > + type != HK_TYPE_TIMER);
> > > }
> > > }
> > > return smp_processor_id();
> > > --
> > > 2.25.1.362.g51ebf55
> > >
> > >
> > >
> >
> > --
> >
>
--
next prev parent reply other threads:[~2024-04-24 14:42 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-30 1:00 [PATCH v2 1/1] wq: Avoid using isolated cpus' timers on queue_delayed_work Leonardo Bras
2024-01-30 1:22 ` Tejun Heo
2024-01-30 2:58 ` Leonardo Bras
2024-04-02 10:58 ` Oleg Nesterov
2024-04-03 19:12 ` Tejun Heo
2024-04-03 20:38 ` Oleg Nesterov
2024-04-05 14:04 ` Oleg Nesterov
2024-04-05 15:38 ` Tejun Heo
2024-04-05 22:03 ` Frederic Weisbecker
2024-04-05 21:52 ` Nohz_full on boot CPU is broken (was: Re: [PATCH v2 1/1] wq: Avoid using isolated cpus' timers on queue_delayed_work) Frederic Weisbecker
2024-04-07 13:09 ` Oleg Nesterov
2024-04-07 13:52 ` Oleg Nesterov
2024-04-09 12:05 ` Frederic Weisbecker
2024-04-09 12:04 ` Frederic Weisbecker
2024-04-09 13:07 ` Oleg Nesterov
2024-04-09 13:59 ` Frederic Weisbecker
2024-04-10 4:26 ` Nicholas Piggin
2024-04-10 13:55 ` Oleg Nesterov
2024-04-11 13:41 ` Oleg Nesterov
2024-04-11 14:39 ` [PATCH] sched/isolation: fix boot crash when the boot CPU is nohz_full Oleg Nesterov
2024-04-11 16:59 ` Oleg Nesterov
2024-04-13 14:17 ` [PATCH] sched/isolation: fix boot crash when maxcpus < first-housekeeping-cpu Oleg Nesterov
2024-04-18 14:54 ` Phil Auld
2024-04-18 15:40 ` Frederic Weisbecker
2024-04-24 20:05 ` [tip: sched/urgent] sched/isolation: Fix boot crash when maxcpus < first housekeeping CPU tip-bot2 for Oleg Nesterov
2024-04-28 8:13 ` Ingo Molnar
2024-04-28 13:16 ` Oleg Nesterov
2024-04-28 8:24 ` tip-bot2 for Oleg Nesterov
2024-04-15 21:37 ` [PATCH] sched/isolation: fix boot crash when the boot CPU is nohz_full Frederic Weisbecker
2024-04-18 14:50 ` Phil Auld
2024-04-22 18:50 ` Oleg Nesterov
2024-04-24 14:42 ` Phil Auld [this message]
2024-04-24 20:05 ` [tip: sched/urgent] sched/isolation: {revent " tip-bot2 for Oleg Nesterov
2024-04-24 20:41 ` Phil Auld
2024-04-28 8:14 ` Ingo Molnar
2024-04-29 11:50 ` Phil Auld
2024-04-28 8:24 ` [tip: sched/urgent] sched/isolation: Prevent " tip-bot2 for Oleg Nesterov
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=20240424144213.GA5682@lorien.usersys.redhat.com \
--to=pauld@redhat.com \
--cc=crecklin@redhat.com \
--cc=frederic@kernel.org \
--cc=jiangshanlai@gmail.com \
--cc=junzhao@redhat.com \
--cc=leobras@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=npiggin@gmail.com \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=tj@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;
as well as URLs for NNTP newsgroup(s).