From: Huacai Chen <chenhuacai@kernel.org>
To: paulmck@kernel.org
Cc: Joel Fernandes <joel@joelfernandes.org>,
Thomas Gleixner <tglx@linutronix.de>,
Z qiang <qiang.zhang1211@gmail.com>,
Huacai Chen <chenhuacai@loongson.cn>,
Frederic Weisbecker <frederic@kernel.org>,
Neeraj Upadhyay <quic_neeraju@quicinc.com>,
Josh Triplett <josh@joshtriplett.org>,
Boqun Feng <boqun.feng@gmail.com>, Ingo Molnar <mingo@kernel.org>,
John Stultz <jstultz@google.com>, Stephen Boyd <sboyd@kernel.org>,
Steven Rostedt <rostedt@goodmis.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Lai Jiangshan <jiangshanlai@gmail.com>,
Sergey Senozhatsky <senozhatsky@chromium.org>,
rcu@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, Binbin Zhou <zhoubinbin@loongson.cn>
Subject: Re: [PATCH V4 2/2] rcu: Update jiffies in rcu_cpu_stall_reset()
Date: Fri, 25 Aug 2023 00:32:54 +0800 [thread overview]
Message-ID: <CAAhV-H4tYm++MHP4vSUbMTDXQrg5rc=iPNoen4OcRuwc5HTVQA@mail.gmail.com> (raw)
In-Reply-To: <CAAhV-H7PSR-3Xi84uz0cs4yojVxODPJnFo_G3BTQK+cHDxx-ww@mail.gmail.com>
On Fri, Aug 25, 2023 at 12:03 AM Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi, Paul,
>
> On Thu, Aug 24, 2023 at 9:28 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Thu, Aug 24, 2023 at 01:09:42PM +0000, Joel Fernandes wrote:
> > > On Thu, Aug 24, 2023 at 04:40:42AM -0700, Paul E. McKenney wrote:
> > > > On Thu, Aug 24, 2023 at 10:50:41AM +0800, Huacai Chen wrote:
> > > > > Hi, Paul,
> > > > >
> > > > > On Thu, Aug 24, 2023 at 6:41 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > > >
> > > > > > On Thu, Aug 24, 2023 at 12:03:25AM +0200, Thomas Gleixner wrote:
> > > > > > > On Thu, Aug 17 2023 at 16:06, Huacai Chen wrote:
> > > > > > > > On Thu, Aug 17, 2023 at 3:27 AM Joel Fernandes <joel@joelfernandes.org> wrote:
> > > > > > > >> > If do_update_jiffies_64() cannot be used in NMI context,
> > > > > > > >>
> > > > > > > >> Can you not make the jiffies update conditional on whether it is
> > > > > > > >> called within NMI context?
> > > > > > >
> > > > > > > Which solves what? If KGDB has a breakpoint in the jiffies lock held
> > > > > > > region then you still dead lock.
> > > > > > >
> > > > > > > >> I dislike that..
> > > > > > > > Is this acceptable?
> > > > > > > >
> > > > > > > > void rcu_cpu_stall_reset(void)
> > > > > > > > {
> > > > > > > > unsigned long delta;
> > > > > > > >
> > > > > > > > delta = nsecs_to_jiffies(ktime_get_ns() - ktime_get_coarse_ns());
> > > > > > > >
> > > > > > > > WRITE_ONCE(rcu_state.jiffies_stall,
> > > > > > > > jiffies + delta + rcu_jiffies_till_stall_check());
> > > > > > > > }
> > > > > > > >
> > > > > > > > This can update jiffies_stall without updating jiffies (but has the
> > > > > > > > same effect).
> > > > > > >
> > > > > > > Now you traded the potential dead lock on jiffies lock for a potential
> > > > > > > live lock vs. tk_core.seq. Not really an improvement, right?
> > > > > > >
> > > > > > > The only way you can do the above is something like the incomplete and
> > > > > > > uncompiled below. NMI safe and therefore livelock proof time interfaces
> > > > > > > exist for a reason.
> > > > > >
> > > > > > Just for completeness, another approach, with its own advantages
> > > > > > and disadvantage, is to add something like ULONG_MAX/4 to
> > > > > > rcu_state.jiffies_stall, but also set a counter indicating that this
> > > > > > has been done. Then RCU's force-quiescent processing could decrement
> > > > > > that counter (if non-zero) and reset rcu_state.jiffies_stall when it
> > > > > > does reach zero.
> > > > > >
> > > > > > Setting the counter to three should cover most cases, but "live by the
> > > > > > heuristic, die by the heuristic". ;-)
> > > > > >
> > > > > > It would be good to have some indication when gdb exited, but things
> > > > > > like the gdb "next" command can make that "interesting" when applied to
> > > > > > a long-running function.
> > > > >
> > > > > The original code is adding ULONG_MAX/2, so adding ULONG_MAX/4 may
> > > > > make no much difference? The simplest way is adding 300*HZ, but Joel
> > > > > dislikes that.
> > > >
> > > > I am not seeing the ULONG_MAX/2, so could you please point me to that
> > > > original code?
> > > >
> > > > The advantage of ULONG_MAX/4 over ULONG_MAX/2 is that the time_after()
> > > > and time_before() macros have ULONG_MAX/4 slop in either direction
> > > > before giving you the wrong answer. You can get nearly the same result
> > > > using ULONG_MAX/2, but it requires a bit more care. And even on 32-bit
> > > > HZ=1000 systems, ULONG_MAX/4 gets you more than 12 days of gdb session
> > > > or jiffies-update delay before you start getting false positives.
> > > >
> > > > Then things can be reset after (say) 3 calls to rcu_gp_fqs() and
> > > > also the current reset at the beginning of a grace period, which
> > > > is in record_gp_stall_check_time().
> > >
> > > I like Paul's suggestion a lot except that if someone sets a breakpoint right
> > > when the jiffies is being reset, so then we have to come back to doing
> > > Thomas's suggestion.
> >
> > Please note that ULONG_MAX / 4 allows for jiffies not having been reset
> > for more than 10 days on 32-bit systems and for many millions of years
> > on 64-bit systems. ;-)
> >
> > > So maybe a combination of Paul's and Thomas's suggestions (of using
> > > last_jiffies_update with the NMI-safe timestamp read) may work.
> >
> > I am absolutely not a fan of reworking all of the RCU CPU stall-warning
> > code to use some other timebase, at least not without a very important
> > reason to do so. Nothing mentioned in this thread even comes close to
> > that level of importance.
> >
> > > > It would be better if RCU could get notified at both ends of the debug
> > > > session, but given gdb commands such as "next", along with Thomas's
> > > > point about gdb breakpoints being pretty much anywhere, this might or
> > > > might not be so helpful in real life. But worth looking into.
> > >
> > > True, I was curious if rcu_cpu_stall_reset() can be called on a tickless
> > > kernel as well before jiffies gets a chance to update, in which case I think
> > > your suggestion of biasing the stall time and later resetting it would help a
> > > lot for such situations.
> >
> > What code path can possibly invoke rcu_cpu_stall_reset() after an
> > extended full-system nohz_full time period without first doing at least
> > one context switch on the CPU that invokes rcu_cpu_stall_reset()?
> In my commit message, the "KGDB initial breakpoint" means the
> automatic call to kgdb_initial_breakpoint() at system boot. In my
> test:
> 1, the "stall timeout" is 21s;
> 2, when I use "continue" to exit kgdb, the "total jiffies delayed
> time" is ~40s (of course it will cause stall warning);
> 3, the "irq disabled time" (nearly the same as execution time of
> kgdb_cpu_enter()) is ~12s;
> 4, this means the "jiffies delayed time" due to the tickless mechanism is ~28s.
>
> So, at least in this case, the tickless mechanism contributes much for
> the jiffies delay.
I'm sorry here is a typo. The "irq disabled time" is ~28s and the
"tickless caused jiffies delayed time" is ~12s in the above test.
>
> Huacai
>
> >
> > Thanx, Paul
> >
> > > thanks,
> > >
> > > - Joel
> > >
> > >
> > > > Thanx, Paul
> > > >
> > > > > Huacai
> > > > >
> > > > > >
> > > > > > Thanx, Paul
> > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > tglx
> > > > > > > ---
> > > > > > > --- a/kernel/time/tick-sched.c
> > > > > > > +++ b/kernel/time/tick-sched.c
> > > > > > > @@ -51,6 +51,13 @@ struct tick_sched *tick_get_tick_sched(i
> > > > > > > */
> > > > > > > static ktime_t last_jiffies_update;
> > > > > > >
> > > > > > > +unsigned long tick_estimate_stale_jiffies(void)
> > > > > > > +{
> > > > > > > + ktime_t delta = ktime_get_mono_fast_ns() - READ_ONCE(last_jiffies_update);
> > > > > > > +
> > > > > > > + return delta < 0 ? 0 : div_s64(delta, TICK_NSEC);
> > > > > > > +}
> > > > > > > +
> > > > > > > /*
> > > > > > > * Must be called with interrupts disabled !
> > > > > > > */
> > > > > > >
> > > > > > >
next prev parent reply other threads:[~2023-08-24 16:33 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-14 2:00 [PATCH V4 1/2] tick: Rename tick_do_update_jiffies64() and allow external usage Huacai Chen
2023-08-14 2:00 ` [PATCH V4 2/2] rcu: Update jiffies in rcu_cpu_stall_reset() Huacai Chen
2023-08-14 16:15 ` Paul E. McKenney
2023-08-15 6:05 ` Huacai Chen
2023-08-16 3:16 ` Z qiang
2023-08-16 4:53 ` Huacai Chen
2023-08-16 5:09 ` Z qiang
2023-08-16 9:33 ` Huacai Chen
2023-08-16 10:06 ` Z qiang
2023-08-16 12:28 ` Huacai Chen
2023-08-16 15:56 ` Alan Huang
2023-08-16 16:13 ` Huacai Chen
2023-08-16 16:52 ` Alan Huang
2023-08-17 4:04 ` Huacai Chen
2023-08-23 21:41 ` Thomas Gleixner
2023-08-16 19:27 ` Joel Fernandes
2023-08-17 8:06 ` Huacai Chen
2023-08-23 22:03 ` Thomas Gleixner
2023-08-23 22:41 ` Paul E. McKenney
2023-08-24 2:50 ` Huacai Chen
2023-08-24 11:40 ` Paul E. McKenney
2023-08-24 12:40 ` Huacai Chen
2023-08-24 13:24 ` Paul E. McKenney
2023-08-24 15:43 ` Huacai Chen
2023-08-24 18:28 ` Paul E. McKenney
2023-08-25 11:15 ` Huacai Chen
2023-08-25 23:28 ` Joel Fernandes
2023-08-27 3:27 ` Joel Fernandes
2023-08-27 5:50 ` Huacai Chen
2023-08-27 22:11 ` Joel Fernandes
2023-08-28 10:47 ` Paul E. McKenney
2023-08-28 11:30 ` Huacai Chen
2023-08-28 11:54 ` Paul E. McKenney
2023-08-28 13:33 ` Joel Fernandes
2023-08-28 14:02 ` Paul E. McKenney
2023-08-28 14:37 ` Huacai Chen
2023-08-28 14:50 ` Joel Fernandes
2023-08-28 15:12 ` Huacai Chen
2023-08-28 20:47 ` Joel Fernandes
2023-08-29 4:07 ` Huacai Chen
2023-08-29 14:46 ` Joel Fernandes
2023-08-30 4:25 ` Huacai Chen
2023-08-30 10:18 ` Joel Fernandes
2023-08-30 1:04 ` Joel Fernandes
2023-08-26 1:45 ` Paul E. McKenney
2023-08-24 13:09 ` Joel Fernandes
2023-08-24 13:28 ` Paul E. McKenney
2023-08-24 16:03 ` Huacai Chen
2023-08-24 16:32 ` Huacai Chen [this message]
2023-08-24 16:34 ` Paul E. McKenney
2023-08-24 2:47 ` Huacai Chen
2023-08-24 9:39 ` Thomas Gleixner
2023-08-24 13:21 ` Joel Fernandes
2023-08-24 13:29 ` Paul E. McKenney
2023-08-24 16:15 ` Huacai Chen
2023-08-23 21:36 ` Thomas Gleixner
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='CAAhV-H4tYm++MHP4vSUbMTDXQrg5rc=iPNoen4OcRuwc5HTVQA@mail.gmail.com' \
--to=chenhuacai@kernel.org \
--cc=boqun.feng@gmail.com \
--cc=chenhuacai@loongson.cn \
--cc=frederic@kernel.org \
--cc=jiangshanlai@gmail.com \
--cc=joel@joelfernandes.org \
--cc=josh@joshtriplett.org \
--cc=jstultz@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@kernel.org \
--cc=paulmck@kernel.org \
--cc=qiang.zhang1211@gmail.com \
--cc=quic_neeraju@quicinc.com \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=sboyd@kernel.org \
--cc=senozhatsky@chromium.org \
--cc=stable@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=zhoubinbin@loongson.cn \
/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).