From: Peter Zijlstra <peterz@infradead.org> To: "Paul E. McKenney" <paulmck@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com>, Steven Rostedt <rostedt@goodmis.org>, mingo@kernel.org, will@kernel.org, boqun.feng@gmail.com, tglx@linutronix.de, bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, seanjc@google.com, pbonzini@redhat.com, jgross@suse.com, srivatsa@csail.mit.edu, amakhalov@vmware.com, pv-drivers@vmware.com, mhiramat@kernel.org, wanpengli@tencent.com, vkuznets@redhat.com, boris.ostrovsky@oracle.com, rafael@kernel.org, daniel.lezcano@linaro.org, juri.lelli@redhat.com, vincent.guittot@linaro.org, dietmar.eggemann@arm.com, bsegall@google.com, mgorman@suse.de, bristot@redhat.com, vschneid@redhat.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-trace-kernel@vger.kernel.org, linux-pm@vger.kernel.org, Frederic Weisbecker <fweisbec@gmail.com> Subject: Re: [PATCH 3/6] ftrace/x86: Warn and ignore graph tracing when RCU is disabled Date: Thu, 26 Jan 2023 10:28:51 +0100 [thread overview] Message-ID: <Y9JH0/Z06254ZJ2g@hirez.programming.kicks-ass.net> (raw) In-Reply-To: <20230125184658.GL2948950@paulmck-ThinkPad-P17-Gen-1> On Wed, Jan 25, 2023 at 10:46:58AM -0800, Paul E. McKenney wrote: > > Ofc. Paul might have an opinion on this glorious bodge ;-) > > For some definition of the word "glorious", to be sure. ;-) > > Am I correct that you have two things happening here? (1) Preventing > trace recursion and (2) forcing RCU to pay attention when needed. Mostly just (1), we're in an error situation, I'm not too worried about (2). > I cannot resist pointing out that you have re-invented RCU_NONIDLE(), > though avoiding much of the overhead when not needed. ;-) Yeah, this was the absolute minimal bodge I could come up with that shuts up the rcu_derefence warning thing. > I would have objections if this ever leaks out onto a non-error code path. Agreed. > There are things that need doing when RCU starts and stops watching, > and this approach omits those things. Which again is OK in this case, > where this code is only ever executed when something is already broken, > but definitely *not* OK when things are not already broken. And agreed. Current version of the bodge looks like so (will repost the whole series a little later today). I managed to tickle the recursion so that it was a test-case for the stack guard... With this on, it prints just the one WARN and lives. --- Subject: bug: Disable rcu_is_watching() during WARN/BUG From: Peter Zijlstra <peterz@infradead.org> Date: Wed Jan 25 13:57:49 CET 2023 In order to avoid WARN/BUG from generating nested or even recursive warnings, force rcu_is_watching() true during WARN/lockdep_rcu_suspicious(). Notably things like unwinding the stack can trigger rcu_dereference() warnings, which then triggers more unwinding which then triggers more warnings etc.. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- include/linux/context_tracking.h | 27 +++++++++++++++++++++++++++ kernel/locking/lockdep.c | 3 +++ kernel/panic.c | 5 +++++ lib/bug.c | 15 ++++++++++++++- 4 files changed, 49 insertions(+), 1 deletion(-) --- a/include/linux/context_tracking.h +++ b/include/linux/context_tracking.h @@ -130,9 +130,36 @@ static __always_inline unsigned long ct_ return arch_atomic_add_return(incby, this_cpu_ptr(&context_tracking.state)); } +static __always_inline bool warn_rcu_enter(void) +{ + bool ret = false; + + /* + * Horrible hack to shut up recursive RCU isn't watching fail since + * lots of the actual reporting also relies on RCU. + */ + preempt_disable_notrace(); + if (rcu_dynticks_curr_cpu_in_eqs()) { + ret = true; + ct_state_inc(RCU_DYNTICKS_IDX); + } + + return ret; +} + +static __always_inline void warn_rcu_exit(bool rcu) +{ + if (rcu) + ct_state_inc(RCU_DYNTICKS_IDX); + preempt_enable_notrace(); +} + #else static inline void ct_idle_enter(void) { } static inline void ct_idle_exit(void) { } + +static __always_inline bool warn_rcu_enter(void) { return false; } +static __always_inline void warn_rcu_exit(bool rcu) { } #endif /* !CONFIG_CONTEXT_TRACKING_IDLE */ #endif --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -55,6 +55,7 @@ #include <linux/rcupdate.h> #include <linux/kprobes.h> #include <linux/lockdep.h> +#include <linux/context_tracking.h> #include <asm/sections.h> @@ -6555,6 +6556,7 @@ void lockdep_rcu_suspicious(const char * { struct task_struct *curr = current; int dl = READ_ONCE(debug_locks); + bool rcu = warn_rcu_enter(); /* Note: the following can be executed concurrently, so be careful. */ pr_warn("\n"); @@ -6595,5 +6597,6 @@ void lockdep_rcu_suspicious(const char * lockdep_print_held_locks(curr); pr_warn("\nstack backtrace:\n"); dump_stack(); + warn_rcu_exit(rcu); } EXPORT_SYMBOL_GPL(lockdep_rcu_suspicious); --- a/kernel/panic.c +++ b/kernel/panic.c @@ -34,6 +34,7 @@ #include <linux/ratelimit.h> #include <linux/debugfs.h> #include <linux/sysfs.h> +#include <linux/context_tracking.h> #include <trace/events/error_report.h> #include <asm/sections.h> @@ -679,6 +680,7 @@ void __warn(const char *file, int line, void warn_slowpath_fmt(const char *file, int line, unsigned taint, const char *fmt, ...) { + bool rcu = warn_rcu_enter(); struct warn_args args; pr_warn(CUT_HERE); @@ -693,11 +695,13 @@ void warn_slowpath_fmt(const char *file, va_start(args.args, fmt); __warn(file, line, __builtin_return_address(0), taint, NULL, &args); va_end(args.args); + warn_rcu_exit(rcu); } EXPORT_SYMBOL(warn_slowpath_fmt); #else void __warn_printk(const char *fmt, ...) { + bool rcu = warn_rcu_enter(); va_list args; pr_warn(CUT_HERE); @@ -705,6 +709,7 @@ void __warn_printk(const char *fmt, ...) va_start(args, fmt); vprintk(fmt, args); va_end(args); + warn_rcu_exit(rcu); } EXPORT_SYMBOL(__warn_printk); #endif --- a/lib/bug.c +++ b/lib/bug.c @@ -47,6 +47,7 @@ #include <linux/sched.h> #include <linux/rculist.h> #include <linux/ftrace.h> +#include <linux/context_tracking.h> extern struct bug_entry __start___bug_table[], __stop___bug_table[]; @@ -153,7 +154,7 @@ struct bug_entry *find_bug(unsigned long return module_find_bug(bugaddr); } -enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs) +static enum bug_trap_type __report_bug(unsigned long bugaddr, struct pt_regs *regs) { struct bug_entry *bug; const char *file; @@ -209,6 +210,18 @@ enum bug_trap_type report_bug(unsigned l return BUG_TRAP_TYPE_BUG; } +enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs) +{ + enum bug_trap_type ret; + bool rcu = false; + + rcu = warn_rcu_enter(); + ret = __report_bug(bugaddr, regs); + warn_rcu_exit(rcu); + + return ret; +} + static void clear_once_table(struct bug_entry *start, struct bug_entry *end) { struct bug_entry *bug;
WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org> To: "Paul E. McKenney" <paulmck@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com>, juri.lelli@redhat.com, daniel.lezcano@linaro.org, wanpengli@tencent.com, kvm@vger.kernel.org, rafael@kernel.org, pv-drivers@vmware.com, Frederic Weisbecker <fweisbec@gmail.com>, dave.hansen@linux.intel.com, virtualization@lists.linux-foundation.org, bsegall@google.com, amakhalov@vmware.com, will@kernel.org, vschneid@redhat.com, hpa@zytor.com, x86@kernel.org, mingo@kernel.org, mgorman@suse.de, linux-trace-kernel@vger.kernel.org, linux-pm@vger.kernel.org, boqun.feng@gmail.com, Steven Rostedt <rostedt@goodmis.org>, bp@alien8.de, vincent.guittot@linaro.org, boris.ostrovsky@oracle.com, dietmar.eggemann@arm.com, jgross@suse.com, seanjc@google.com, linux-kernel@vger.kernel.org, tglx@linutronix.de, mhiramat@kernel.org, pbonzini@redhat.com, bristot@redhat.com Subject: Re: [PATCH 3/6] ftrace/x86: Warn and ignore graph tracing when RCU is disabled Date: Thu, 26 Jan 2023 10:28:51 +0100 [thread overview] Message-ID: <Y9JH0/Z06254ZJ2g@hirez.programming.kicks-ass.net> (raw) In-Reply-To: <20230125184658.GL2948950@paulmck-ThinkPad-P17-Gen-1> On Wed, Jan 25, 2023 at 10:46:58AM -0800, Paul E. McKenney wrote: > > Ofc. Paul might have an opinion on this glorious bodge ;-) > > For some definition of the word "glorious", to be sure. ;-) > > Am I correct that you have two things happening here? (1) Preventing > trace recursion and (2) forcing RCU to pay attention when needed. Mostly just (1), we're in an error situation, I'm not too worried about (2). > I cannot resist pointing out that you have re-invented RCU_NONIDLE(), > though avoiding much of the overhead when not needed. ;-) Yeah, this was the absolute minimal bodge I could come up with that shuts up the rcu_derefence warning thing. > I would have objections if this ever leaks out onto a non-error code path. Agreed. > There are things that need doing when RCU starts and stops watching, > and this approach omits those things. Which again is OK in this case, > where this code is only ever executed when something is already broken, > but definitely *not* OK when things are not already broken. And agreed. Current version of the bodge looks like so (will repost the whole series a little later today). I managed to tickle the recursion so that it was a test-case for the stack guard... With this on, it prints just the one WARN and lives. --- Subject: bug: Disable rcu_is_watching() during WARN/BUG From: Peter Zijlstra <peterz@infradead.org> Date: Wed Jan 25 13:57:49 CET 2023 In order to avoid WARN/BUG from generating nested or even recursive warnings, force rcu_is_watching() true during WARN/lockdep_rcu_suspicious(). Notably things like unwinding the stack can trigger rcu_dereference() warnings, which then triggers more unwinding which then triggers more warnings etc.. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- include/linux/context_tracking.h | 27 +++++++++++++++++++++++++++ kernel/locking/lockdep.c | 3 +++ kernel/panic.c | 5 +++++ lib/bug.c | 15 ++++++++++++++- 4 files changed, 49 insertions(+), 1 deletion(-) --- a/include/linux/context_tracking.h +++ b/include/linux/context_tracking.h @@ -130,9 +130,36 @@ static __always_inline unsigned long ct_ return arch_atomic_add_return(incby, this_cpu_ptr(&context_tracking.state)); } +static __always_inline bool warn_rcu_enter(void) +{ + bool ret = false; + + /* + * Horrible hack to shut up recursive RCU isn't watching fail since + * lots of the actual reporting also relies on RCU. + */ + preempt_disable_notrace(); + if (rcu_dynticks_curr_cpu_in_eqs()) { + ret = true; + ct_state_inc(RCU_DYNTICKS_IDX); + } + + return ret; +} + +static __always_inline void warn_rcu_exit(bool rcu) +{ + if (rcu) + ct_state_inc(RCU_DYNTICKS_IDX); + preempt_enable_notrace(); +} + #else static inline void ct_idle_enter(void) { } static inline void ct_idle_exit(void) { } + +static __always_inline bool warn_rcu_enter(void) { return false; } +static __always_inline void warn_rcu_exit(bool rcu) { } #endif /* !CONFIG_CONTEXT_TRACKING_IDLE */ #endif --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -55,6 +55,7 @@ #include <linux/rcupdate.h> #include <linux/kprobes.h> #include <linux/lockdep.h> +#include <linux/context_tracking.h> #include <asm/sections.h> @@ -6555,6 +6556,7 @@ void lockdep_rcu_suspicious(const char * { struct task_struct *curr = current; int dl = READ_ONCE(debug_locks); + bool rcu = warn_rcu_enter(); /* Note: the following can be executed concurrently, so be careful. */ pr_warn("\n"); @@ -6595,5 +6597,6 @@ void lockdep_rcu_suspicious(const char * lockdep_print_held_locks(curr); pr_warn("\nstack backtrace:\n"); dump_stack(); + warn_rcu_exit(rcu); } EXPORT_SYMBOL_GPL(lockdep_rcu_suspicious); --- a/kernel/panic.c +++ b/kernel/panic.c @@ -34,6 +34,7 @@ #include <linux/ratelimit.h> #include <linux/debugfs.h> #include <linux/sysfs.h> +#include <linux/context_tracking.h> #include <trace/events/error_report.h> #include <asm/sections.h> @@ -679,6 +680,7 @@ void __warn(const char *file, int line, void warn_slowpath_fmt(const char *file, int line, unsigned taint, const char *fmt, ...) { + bool rcu = warn_rcu_enter(); struct warn_args args; pr_warn(CUT_HERE); @@ -693,11 +695,13 @@ void warn_slowpath_fmt(const char *file, va_start(args.args, fmt); __warn(file, line, __builtin_return_address(0), taint, NULL, &args); va_end(args.args); + warn_rcu_exit(rcu); } EXPORT_SYMBOL(warn_slowpath_fmt); #else void __warn_printk(const char *fmt, ...) { + bool rcu = warn_rcu_enter(); va_list args; pr_warn(CUT_HERE); @@ -705,6 +709,7 @@ void __warn_printk(const char *fmt, ...) va_start(args, fmt); vprintk(fmt, args); va_end(args); + warn_rcu_exit(rcu); } EXPORT_SYMBOL(__warn_printk); #endif --- a/lib/bug.c +++ b/lib/bug.c @@ -47,6 +47,7 @@ #include <linux/sched.h> #include <linux/rculist.h> #include <linux/ftrace.h> +#include <linux/context_tracking.h> extern struct bug_entry __start___bug_table[], __stop___bug_table[]; @@ -153,7 +154,7 @@ struct bug_entry *find_bug(unsigned long return module_find_bug(bugaddr); } -enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs) +static enum bug_trap_type __report_bug(unsigned long bugaddr, struct pt_regs *regs) { struct bug_entry *bug; const char *file; @@ -209,6 +210,18 @@ enum bug_trap_type report_bug(unsigned l return BUG_TRAP_TYPE_BUG; } +enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs) +{ + enum bug_trap_type ret; + bool rcu = false; + + rcu = warn_rcu_enter(); + ret = __report_bug(bugaddr, regs); + warn_rcu_exit(rcu); + + return ret; +} + static void clear_once_table(struct bug_entry *start, struct bug_entry *end) { struct bug_entry *bug; _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next prev parent reply other threads:[~2023-01-26 9:30 UTC|newest] Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-01-23 20:50 [PATCH 0/6] A few cpuidle vs rcu fixes Peter Zijlstra 2023-01-23 20:50 ` Peter Zijlstra 2023-01-23 20:50 ` [PATCH 1/6] x86: Always inline arch_atomic64 Peter Zijlstra 2023-01-23 20:50 ` Peter Zijlstra 2023-01-23 20:50 ` [PATCH 2/6] x86/pvclock: improve atomic update of last_value in pvclock_clocksource_read Peter Zijlstra 2023-01-23 20:50 ` Peter Zijlstra 2023-01-23 20:50 ` [PATCH 3/6] ftrace/x86: Warn and ignore graph tracing when RCU is disabled Peter Zijlstra 2023-01-23 20:50 ` Peter Zijlstra 2023-01-23 21:53 ` Steven Rostedt 2023-01-23 21:53 ` Steven Rostedt 2023-01-23 22:07 ` Steven Rostedt 2023-01-23 22:07 ` Steven Rostedt 2023-01-24 14:44 ` Peter Zijlstra 2023-01-24 14:44 ` Peter Zijlstra 2023-01-24 17:12 ` Mark Rutland 2023-01-24 17:12 ` Mark Rutland 2023-01-25 9:37 ` Peter Zijlstra 2023-01-25 9:37 ` Peter Zijlstra 2023-01-25 10:47 ` Peter Zijlstra 2023-01-25 10:47 ` Peter Zijlstra 2023-01-25 11:32 ` Mark Rutland 2023-01-25 11:32 ` Mark Rutland 2023-01-25 18:46 ` Paul E. McKenney 2023-01-26 9:28 ` Peter Zijlstra [this message] 2023-01-26 9:28 ` Peter Zijlstra 2023-01-28 19:12 ` Paul E. McKenney 2023-01-23 20:50 ` [PATCH 4/6] x86: Mark sched_clock() noinstr Peter Zijlstra 2023-01-23 20:50 ` Peter Zijlstra 2023-01-23 20:50 ` [PATCH 5/6] sched/clock: Make local_clock() noinstr Peter Zijlstra 2023-01-23 20:50 ` Peter Zijlstra 2023-01-23 20:50 ` [PATCH 6/6] cpuidle: Fix poll_idle() noinstr annotation Peter Zijlstra 2023-01-23 20:50 ` Peter Zijlstra 2023-01-24 14:24 ` Rafael J. Wysocki 2023-01-24 14:24 ` Rafael J. Wysocki 2023-01-24 16:34 ` [PATCH 0/6] A few cpuidle vs rcu fixes Mark Rutland 2023-01-24 16:34 ` Mark Rutland 2023-01-24 17:30 ` Mark Rutland 2023-01-24 17:30 ` Mark Rutland 2023-01-24 18:39 ` Mark Rutland 2023-01-24 18:39 ` Mark Rutland 2023-01-25 9:35 ` Peter Zijlstra 2023-01-25 9:35 ` Peter Zijlstra 2023-01-25 9:40 ` Peter Zijlstra 2023-01-25 9:40 ` Peter Zijlstra 2023-01-25 10:23 ` Mark Rutland 2023-01-25 10:23 ` Mark Rutland 2023-01-31 14:22 ` [tip: sched/core] cpuidle: tracing, preempt: Squash _rcuidle tracing tip-bot2 for Peter Zijlstra 2023-01-25 9:31 ` [PATCH 0/6] A few cpuidle vs rcu fixes Peter Zijlstra 2023-01-25 9:31 ` Peter Zijlstra 2023-01-25 9:36 ` Mark Rutland 2023-01-25 9:36 ` Mark Rutland 2023-01-25 15:20 ` Mark Rutland 2023-01-25 15:20 ` Mark Rutland
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=Y9JH0/Z06254ZJ2g@hirez.programming.kicks-ass.net \ --to=peterz@infradead.org \ --cc=amakhalov@vmware.com \ --cc=boqun.feng@gmail.com \ --cc=boris.ostrovsky@oracle.com \ --cc=bp@alien8.de \ --cc=bristot@redhat.com \ --cc=bsegall@google.com \ --cc=daniel.lezcano@linaro.org \ --cc=dave.hansen@linux.intel.com \ --cc=dietmar.eggemann@arm.com \ --cc=fweisbec@gmail.com \ --cc=hpa@zytor.com \ --cc=jgross@suse.com \ --cc=juri.lelli@redhat.com \ --cc=kvm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-trace-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mgorman@suse.de \ --cc=mhiramat@kernel.org \ --cc=mingo@kernel.org \ --cc=paulmck@kernel.org \ --cc=pbonzini@redhat.com \ --cc=pv-drivers@vmware.com \ --cc=rafael@kernel.org \ --cc=rostedt@goodmis.org \ --cc=seanjc@google.com \ --cc=srivatsa@csail.mit.edu \ --cc=tglx@linutronix.de \ --cc=vincent.guittot@linaro.org \ --cc=virtualization@lists.linux-foundation.org \ --cc=vkuznets@redhat.com \ --cc=vschneid@redhat.com \ --cc=wanpengli@tencent.com \ --cc=will@kernel.org \ --cc=x86@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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.