From: Alan Huang <mmpgouride@gmail.com>
To: Huacai Chen <chenhuacai@kernel.org>
Cc: Z qiang <qiang.zhang1211@gmail.com>,
"Paul E . McKenney" <paulmck@kernel.org>,
Huacai Chen <chenhuacai@loongson.cn>,
Frederic Weisbecker <frederic@kernel.org>,
Neeraj Upadhyay <quic_neeraju@quicinc.com>,
Joel Fernandes <joel@joelfernandes.org>,
Josh Triplett <josh@joshtriplett.org>,
Boqun Feng <boqun.feng@gmail.com>,
Thomas Gleixner <tglx@linutronix.de>,
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: Wed, 16 Aug 2023 23:56:32 +0800 [thread overview]
Message-ID: <D3AFC0E7-9E6C-43F2-B9F5-3AC498B14F0F@gmail.com> (raw)
In-Reply-To: <CAAhV-H6ejw=8afS0jmmQvKUrCw=qZm_P6SA0A+tuvvb8bsq4-Q@mail.gmail.com>
>>>>>>>>>
>>>>>>>>> Currently rcu_cpu_stall_reset() set rcu_state.jiffies_stall to one check
>>>>>>>>> period later, i.e. jiffies + rcu_jiffies_till_stall_check(). But jiffies
>>>>>>>>> is only updated in the timer interrupt, so when kgdb_cpu_enter() begins
>>>>>>>>> to run there may already be nearly one rcu check period after jiffies.
>>>>>>>>> Since all interrupts are disabled during kgdb_cpu_enter(), jiffies will
>>>>>>>>> not be updated. When kgdb_cpu_enter() returns, rcu_state.jiffies_stall
>>>>>>>>> maybe already gets timeout.
>>>>>>>>>
>>>>>>>>> We can set rcu_state.jiffies_stall to two rcu check periods later, e.g.
>>>>>>>>> jiffies + (rcu_jiffies_till_stall_check() * 2) in rcu_cpu_stall_reset()
>>>>>>>>> to avoid this problem. But this isn't a complete solution because kgdb
>>>>>>>>> may take a very long time in irq disabled context.
>>>>>>>>>
>>>>>>>>> Instead, update jiffies at the beginning of rcu_cpu_stall_reset() can
>>>>>>>>> solve all kinds of problems.
>>>>>>>>
>>>>>>>> Would it make sense for there to be a kgdb_cpu_exit()? In that case,
>>>>>>>> the stalls could simply be suppressed at the beginning of the debug
>>>>>>>> session and re-enabled upon exit, as is currently done for sysrq output
>>>>>>>> via rcu_sysrq_start() and rcu_sysrq_end().
>>>>>>> Thank you for your advice, but that doesn't help. Because
>>>>>>> rcu_sysrq_start() and rcu_sysrq_end() try to suppress the warnings
>>>>>>> during sysrq, but kgdb already has no warnings during kgdb_cpu_enter()
>>>>>>> since it is executed in irq disabled context. Instead, this patch
>>>>>>> wants to suppress the warnings *after* kgdb_cpu_enter() due to a very
>>>>>>> old jiffies value.
>>>>>>>
>>>>>>
>>>>>> Hello, Huacai
>>>>>>
>>>>>> Is it possible to set the rcu_cpu_stall_suppress is true in
>>>>>> dbg_touch_watchdogs()
>>>>>> and reset the rcu_cpu_stall_suppress at the beginning and end of the
>>>>>> RCU grace period?
>>>>> This is possible but not the best: 1, kgdb is not the only caller of
>>>>> rcu_cpu_stall_reset(); 2, it is difficult to find the "end" to reset
>>>>> rcu_cpu_stall_suppress.
>>>>>
>>>>
>>>> You can replace rcu_state.jiffies_stall update by setting rcu_cpu_stall_suppress
>>>> in rcu_cpu_stall_reset(), and reset rcu_cpu_stall_suppress in rcu_gp_init() and
>>>> rcu_gp_cleanup().
>>> What's the advantage compared with updating jiffies? Updating jiffies
>>> seems more straight forward.
>>>
>>
>> In do_update_jiffies_64(), need to acquire jiffies_lock raw spinlock,
>> like you said, kgdb is not the only caller of rcu_cpu_stall_reset(),
>> the rcu_cpu_stall_reset() maybe invoke in NMI (arch/x86/platform/uv/uv_nmi.c)
> Reset rcu_cpu_stall_suppress in rcu_gp_init()/rcu_gp_cleanup() is
> still not so good to me, because it does a useless operation in most
> cases. Moreover, the rcu core is refactored again and again, something
> may be changed in future.
>
> If do_update_jiffies_64() cannot be used in NMI context, can we
What about updating jiffies in dbg_touch_watchdogs or adding a wrapper which updates
both jiffies and jiffies_stall?
> consider my old method [1]?
> https://lore.kernel.org/rcu/CAAhV-H7j9Y=VvRLm8thLw-EX1PGqBA9YfT4G1AN7ucYS=iP+DQ@mail.gmail.com/T/#t
>
> Of course we should set rcu_state.jiffies_stall large enough, so we
> can do like this:
>
> void rcu_cpu_stall_reset(void)
> {
> WRITE_ONCE(rcu_state.jiffies_stall,
> - jiffies + rcu_jiffies_till_stall_check());
> + jiffies + 300 * HZ);
> }
>
> 300s is the largest timeout value, and I think 300s is enough here in practice.
>
> Huacai
>
>>
>> Thanks
>> Zqiang
>>
>>
>>> Huacai
>>>
>>>>
>>>> Thanks
>>>> Zqiang
>>>>
>>>>>
>>>>>> or set rcupdate.rcu_cpu_stall_suppress_at_boot=1 in bootargs can
>>>>>> suppress RCU stall
>>>>>> in booting.
>>>>> This is also possible, but it suppresses all kinds of stall warnings,
>>>>> which is not what we want.
>>>>>
>>>>> Huacai
>>>>>>
>>>>>>
>>>>>> Thanks
>>>>>> Zqiang
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>>
>>>>>>>> Thanx, Paul
>>>>>>>>
>>>>>>>>> Cc: stable@vger.kernel.org
>>>>>>>>> Fixes: a80be428fbc1f1f3bc9ed924 ("rcu: Do not disable GP stall detection in rcu_cpu_stall_reset()")
>>>>>>>>> Reported-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
>>>>>>>>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
>>>>>>>>> ---
>>>>>>>>> kernel/rcu/tree_stall.h | 1 +
>>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>>
>>>>>>>>> diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
>>>>>>>>> index b10b8349bb2a..1c7b540985bf 100644
>>>>>>>>> --- a/kernel/rcu/tree_stall.h
>>>>>>>>> +++ b/kernel/rcu/tree_stall.h
>>>>>>>>> @@ -153,6 +153,7 @@ static void panic_on_rcu_stall(void)
>>>>>>>>> */
>>>>>>>>> void rcu_cpu_stall_reset(void)
>>>>>>>>> {
>>>>>>>>> + do_update_jiffies_64(ktime_get());
>>>>>>>>> WRITE_ONCE(rcu_state.jiffies_stall,
>>>>>>>>> jiffies + rcu_jiffies_till_stall_check());
>>>>>>>>> }
>>>>>>>>> --
>>>>>>>>> 2.39.3
next prev parent reply other threads:[~2023-08-16 15:57 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 [this message]
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
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=D3AFC0E7-9E6C-43F2-B9F5-3AC498B14F0F@gmail.com \
--to=mmpgouride@gmail.com \
--cc=boqun.feng@gmail.com \
--cc=chenhuacai@kernel.org \
--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).