Message ID | 20241003181629.36209057@gandalf.local.home (mailing list archive) |
---|---|
State | Accepted |
Commit | 48bcda6848232667f13b4e97588de488c83c37d4 |
Headers | show |
Series | [v2] tracing: Remove definition of trace_*_rcuidle() | expand |
On Thu, Oct 03, 2024 at 06:16:29PM -0400, Steven Rostedt wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > The trace_*_rcuidle() variant of a tracepoint was to handle places where a > tracepoint was located but RCU was not "watching". All those locations > have been removed, and RCU should be watching where all tracepoints are > located. We can now remove the trace_*_rcuidle() variant. > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- W00t Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Hi Steven, On Fri, Oct 4, 2024 at 12:15 AM Steven Rostedt <rostedt@goodmis.org> wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > The trace_*_rcuidle() variant of a tracepoint was to handle places where a > tracepoint was located but RCU was not "watching". All those locations > have been removed, and RCU should be watching where all tracepoints are > located. We can now remove the trace_*_rcuidle() variant. > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > Changes since v1: https://lore.kernel.org/20241003173051.6b178bb3@gandalf.local.home > > - Silly me didn't remove the rcuidle logic from __DO_TRACE() Thanks for your patch, which is now commit 48bcda6848232667 ("tracing: Remove definition of trace_*_rcuidle()") in v6.13-rc1. This is causing warnings during s2ram on various ARM platforms. E.g. on Koelsch: Freezing user space processes Freezing user space processes completed (elapsed 0.001 seconds) OOM killer disabled. Freezing remaining freezable tasks -Freezing remaining freezable tasks completed (elapsed 0.001 seconds) -PM: suspend devices took 0.110 seconds +Freezing remaining freezable tasks completed (elapsed 0.003 seconds) +------------[ cut here ]------------ +------------[ cut here ]------------ +WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:36 __irq_svc+0x48/0xac +WARNING: CPU: 1 PID: 0 at include/trace/events/preemptirq.h:40 trace_hardirqs_on+0x40/0x7c +RCU not watching for tracepoint +RCU not watching for tracepoint +Modules linked in: +Modules linked in: +CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G W N 6.12.0-rc2-koelsch-00005-g48bcda684823 #2094 + +Tainted: [W]=WARN, [N]=TEST +Hardware name: Generic R-Car Gen2 (Flattened Device Tree) +Call trace: + unwind_backtrace from show_stack+0x10/0x14 + show_stack from dump_stack_lvl+0x78/0xa8 + dump_stack_lvl from __warn+0x80/0x198 + __warn from warn_slowpath_fmt+0xbc/0x120 + warn_slowpath_fmt from __irq_svc+0x48/0xac +Exception stack(0xc1201f20 to 0xc1201f68) +1f20: c026570c ebf8fe7c 00000000 00000000 c120c0c0 c026570c c120905c c1209020 +1f40: c120c0c0 c106d640 c120905c c10438f8 fffffff8 c1201f70 c0a1c4fc c0a1c510 +1f60: 60000013 ffffffff + __irq_svc from cpu_idle_poll+0xd0/0x130 + cpu_idle_poll from do_idle+0xc0/0x2a0 + do_idle from cpu_startup_entry+0x28/0x2c + cpu_startup_entry from rest_init+0x150/0x178 + rest_init from start_kernel+0x57c/0x610 +irq event stamp: 24900 +CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Tainted: G W N 6.12.0-rc2-koelsch-00005-g48bcda684823 #2094 +hardirqs last enabled at (24899): [<c0a1c658>] default_idle_call+0xe8/0x104 +Tainted: [W]=WARN, [N]=TEST +hardirqs last disabled at (24900): [<c0200b68>] __irq_svc+0x48/0xac +Hardware name: Generic R-Car Gen2 (Flattened Device Tree) +softirqs last enabled at (24832): [<c0226bd4>] handle_softirqs+0x170/0x3cc +Call trace: +softirqs last disabled at (24821): [<c0227400>] __irq_exit_rcu+0xf0/0x148 + unwind_backtrace from show_stack+0x10/0x14 +---[ end trace 0000000000000000 ]--- + show_stack from dump_stack_lvl+0x78/0xa8 + dump_stack_lvl from __warn+0x80/0x198 + __warn from warn_slowpath_fmt+0xbc/0x120 + warn_slowpath_fmt from trace_hardirqs_on+0x40/0x7c + trace_hardirqs_on from __irq_svc+0x94/0xac +Exception stack(0xf0861f48 to 0xf0861f90) +1f40: c026570c ebfa1e7c 00000000 00000001 c2220000 c026570c +1f60: c120905c c1209020 c2220000 c106d640 c120905c 00000000 fffffff8 f0861f98 +1f80: c0a1c4fc c0a1c554 20000113 ffffffff + __irq_svc from cpu_idle_poll+0x114/0x130 + cpu_idle_poll from do_idle+0xc0/0x2a0 + do_idle from cpu_startup_entry+0x28/0x2c + cpu_startup_entry from secondary_start_kernel+0xec/0x104 + secondary_start_kernel from 0x40201334 +irq event stamp: 16791 +hardirqs last enabled at (16791): [<c0265764>] do_idle+0x118/0x2a0 +hardirqs last disabled at (16790): [<c02656d8>] do_idle+0x8c/0x2a0 +softirqs last enabled at (16782): [<c0226bd4>] handle_softirqs+0x170/0x3cc +softirqs last disabled at (16765): [<c0227400>] __irq_exit_rcu+0xf0/0x148 +---[ end trace 0000000000000000 ]--- +PM: suspend devices took 0.430 seconds Disabling non-boot CPUs ... Enabling non-boot CPUs ... CPU1 is up Unfortunately it cannot be reverted easily. Gr{oetje,eeting}s, Geert
Hi Steven, On Thu, Oct 03, 2024 at 06:16:29PM -0400, Steven Rostedt wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > The trace_*_rcuidle() variant of a tracepoint was to handle places where a > tracepoint was located but RCU was not "watching". All those locations > have been removed, and RCU should be watching where all tracepoints are > located. We can now remove the trace_*_rcuidle() variant. > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- This patch triggers backtraces with mips qemu emulations. WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:36 handle_int+0x128/0x178 RCU not watching for tracepoint ... Call Trace: [<ffffffff8012a89c>] show_stack+0x64/0x158 [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128 [<ffffffff801550f4>] __warn+0xa4/0x1f8 [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0 [<ffffffff801233e8>] handle_int+0x128/0x178 [<ffffffff80f25ff8>] r4k_wait+0x30/0x40 [<ffffffff80f26448>] default_idle_call+0xa0/0x330 [<ffffffff801b6ccc>] do_idle+0xe4/0x1b8 [<ffffffff801b71c4>] cpu_startup_entry+0x34/0x48 [<ffffffff80f2689c>] kernel_init+0x0/0x110 WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:40 trace_hardirqs_on+0x184/0x240 RCU not watching for tracepoint ... Call Trace: [<ffffffff8012a89c>] show_stack+0x64/0x158 [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128 [<ffffffff801550f4>] __warn+0xa4/0x1f8 [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0 [<ffffffff8028fb74>] trace_hardirqs_on+0x184/0x240 [<ffffffff80123030>] restore_partial+0x6c/0x13c Bisect log is attached. Guenter --- # bad: [f6420e2ee9caa3ecf226d0db22572db0d9451977] Merge branch 'fixes-v6.13' into testing # good: [adc218676eef25575469234709c2d87185ca223a] Linux 6.12 git bisect start 'f6420e2ee9ca' 'v6.12' # good: [6e95ef0258ff4ee23ae3b06bf6b00b33dbbd5ef7] Merge tag 'bpf-next-6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next git bisect good 6e95ef0258ff4ee23ae3b06bf6b00b33dbbd5ef7 # good: [071b34dcf71523a559b6c39f5d21a268a9531b50] Merge tag 'sound-6.13-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound git bisect good 071b34dcf71523a559b6c39f5d21a268a9531b50 # good: [8a07b2623e7ff04856f8d4470d002675049b2065] Merge tag 'drm-misc-next-2024-10-31' of https://gitlab.freedesktop.org/drm/misc/kernel into drm-next git bisect good 8a07b2623e7ff04856f8d4470d002675049b2065 # bad: [80739fd00c7ea1315d362ce889bef499452913ef] Merge tag 'mfd-next-6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd git bisect bad 80739fd00c7ea1315d362ce889bef499452913ef # good: [e175800137f588688ac6aae30ce491e098f30c45] Merge tag 'exynos-drm-next-for-v6.13-v2' of git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-next git bisect good e175800137f588688ac6aae30ce491e098f30c45 # good: [ade5add00da20de40f63d097345bddea24d924f4] Merge tag 'amd-drm-next-6.13-2024-11-15' of https://gitlab.freedesktop.org/agd5f/linux into drm-next git bisect good ade5add00da20de40f63d097345bddea24d924f4 # bad: [06afb0f36106ecb839c5e2509905e68c1e2677de] Merge tag 'trace-v6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace git bisect bad 06afb0f36106ecb839c5e2509905e68c1e2677de # good: [f1db825805d48cee6826b7dc082a04112c1f0c8d] Merge tag 'trace-ring-buffer-v6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace git bisect good f1db825805d48cee6826b7dc082a04112c1f0c8d # bad: [61c6fefa92bb4ed7a34163b94f6ffac628237a29] bpf: decouple BPF link/attach hook and BPF program sleepable semantics git bisect bad 61c6fefa92bb4ed7a34163b94f6ffac628237a29 # bad: [cdb537ac417938408ee819992f432c410f2d01a2] tracing/perf: Add might_fault check to syscall probes git bisect bad cdb537ac417938408ee819992f432c410f2d01a2 # bad: [e53244e2c8931f9e80c1841293aea86ef8ad32a3] tracepoint: Remove SRCU protection git bisect bad e53244e2c8931f9e80c1841293aea86ef8ad32a3 # good: [49e4154f4b16345da5e219b23ed9737a6e735bc1] tracing: Remove TRACE_EVENT_FL_FILTERED logic git bisect good 49e4154f4b16345da5e219b23ed9737a6e735bc1 # bad: [48bcda6848232667f13b4e97588de488c83c37d4] tracing: Remove definition of trace_*_rcuidle() git bisect bad 48bcda6848232667f13b4e97588de488c83c37d4 # good: [4a8840af5f53f2902eba91130fae650879f18e7a] tracepoints: Use new static branch API git bisect good 4a8840af5f53f2902eba91130fae650879f18e7a # first bad commit: [48bcda6848232667f13b4e97588de488c83c37d4] tracing: Remove definition of trace_*_rcuidle()
[ Adding MIPS maintainers ] On Tue, 3 Dec 2024 12:39:08 -0800 Guenter Roeck <linux@roeck-us.net> wrote: > Hi Steven, > > On Thu, Oct 03, 2024 at 06:16:29PM -0400, Steven Rostedt wrote: > > From: Steven Rostedt <rostedt@goodmis.org> > > > > The trace_*_rcuidle() variant of a tracepoint was to handle places where a > > tracepoint was located but RCU was not "watching". All those locations > > have been removed, and RCU should be watching where all tracepoints are > > located. We can now remove the trace_*_rcuidle() variant. > > > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > > --- > > This patch triggers backtraces with mips qemu emulations. > > WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:36 handle_int+0x128/0x178 > RCU not watching for tracepoint > ... > Call Trace: > [<ffffffff8012a89c>] show_stack+0x64/0x158 > [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128 > [<ffffffff801550f4>] __warn+0xa4/0x1f8 > [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0 > [<ffffffff801233e8>] handle_int+0x128/0x178 > [<ffffffff80f25ff8>] r4k_wait+0x30/0x40 > [<ffffffff80f26448>] default_idle_call+0xa0/0x330 > [<ffffffff801b6ccc>] do_idle+0xe4/0x1b8 > [<ffffffff801b71c4>] cpu_startup_entry+0x34/0x48 > [<ffffffff80f2689c>] kernel_init+0x0/0x110 > > WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:40 trace_hardirqs_on+0x184/0x240 > RCU not watching for tracepoint > ... > Call Trace: > [<ffffffff8012a89c>] show_stack+0x64/0x158 > [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128 > [<ffffffff801550f4>] __warn+0xa4/0x1f8 > [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0 > [<ffffffff8028fb74>] trace_hardirqs_on+0x184/0x240 > [<ffffffff80123030>] restore_partial+0x6c/0x13c > > Bisect log is attached. I'm guessing MIPS needs to implement something like what arm64 did with: 7cd1ea1010acb ("arm64: entry: fix non-NMI kernel<->kernel transitions") Because we do not want to bring back the trace_*_rcuidle() crap. -- Steve > > Guenter > > --- > # bad: [f6420e2ee9caa3ecf226d0db22572db0d9451977] Merge branch 'fixes-v6.13' into testing > # good: [adc218676eef25575469234709c2d87185ca223a] Linux 6.12 > git bisect start 'f6420e2ee9ca' 'v6.12' > # good: [6e95ef0258ff4ee23ae3b06bf6b00b33dbbd5ef7] Merge tag 'bpf-next-6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next > git bisect good 6e95ef0258ff4ee23ae3b06bf6b00b33dbbd5ef7 > # good: [071b34dcf71523a559b6c39f5d21a268a9531b50] Merge tag 'sound-6.13-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound > git bisect good 071b34dcf71523a559b6c39f5d21a268a9531b50 > # good: [8a07b2623e7ff04856f8d4470d002675049b2065] Merge tag 'drm-misc-next-2024-10-31' of https://gitlab.freedesktop.org/drm/misc/kernel into drm-next > git bisect good 8a07b2623e7ff04856f8d4470d002675049b2065 > # bad: [80739fd00c7ea1315d362ce889bef499452913ef] Merge tag 'mfd-next-6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd > git bisect bad 80739fd00c7ea1315d362ce889bef499452913ef > # good: [e175800137f588688ac6aae30ce491e098f30c45] Merge tag 'exynos-drm-next-for-v6.13-v2' of git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-next > git bisect good e175800137f588688ac6aae30ce491e098f30c45 > # good: [ade5add00da20de40f63d097345bddea24d924f4] Merge tag 'amd-drm-next-6.13-2024-11-15' of https://gitlab.freedesktop.org/agd5f/linux into drm-next > git bisect good ade5add00da20de40f63d097345bddea24d924f4 > # bad: [06afb0f36106ecb839c5e2509905e68c1e2677de] Merge tag 'trace-v6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace > git bisect bad 06afb0f36106ecb839c5e2509905e68c1e2677de > # good: [f1db825805d48cee6826b7dc082a04112c1f0c8d] Merge tag 'trace-ring-buffer-v6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace > git bisect good f1db825805d48cee6826b7dc082a04112c1f0c8d > # bad: [61c6fefa92bb4ed7a34163b94f6ffac628237a29] bpf: decouple BPF link/attach hook and BPF program sleepable semantics > git bisect bad 61c6fefa92bb4ed7a34163b94f6ffac628237a29 > # bad: [cdb537ac417938408ee819992f432c410f2d01a2] tracing/perf: Add might_fault check to syscall probes > git bisect bad cdb537ac417938408ee819992f432c410f2d01a2 > # bad: [e53244e2c8931f9e80c1841293aea86ef8ad32a3] tracepoint: Remove SRCU protection > git bisect bad e53244e2c8931f9e80c1841293aea86ef8ad32a3 > # good: [49e4154f4b16345da5e219b23ed9737a6e735bc1] tracing: Remove TRACE_EVENT_FL_FILTERED logic > git bisect good 49e4154f4b16345da5e219b23ed9737a6e735bc1 > # bad: [48bcda6848232667f13b4e97588de488c83c37d4] tracing: Remove definition of trace_*_rcuidle() > git bisect bad 48bcda6848232667f13b4e97588de488c83c37d4 > # good: [4a8840af5f53f2902eba91130fae650879f18e7a] tracepoints: Use new static branch API > git bisect good 4a8840af5f53f2902eba91130fae650879f18e7a > # first bad commit: [48bcda6848232667f13b4e97588de488c83c37d4] tracing: Remove definition of trace_*_rcuidle()
On 12/3/24 12:55, Steven Rostedt wrote: > > [ Adding MIPS maintainers ] > > On Tue, 3 Dec 2024 12:39:08 -0800 > Guenter Roeck <linux@roeck-us.net> wrote: > >> Hi Steven, >> >> On Thu, Oct 03, 2024 at 06:16:29PM -0400, Steven Rostedt wrote: >>> From: Steven Rostedt <rostedt@goodmis.org> >>> >>> The trace_*_rcuidle() variant of a tracepoint was to handle places where a >>> tracepoint was located but RCU was not "watching". All those locations >>> have been removed, and RCU should be watching where all tracepoints are >>> located. We can now remove the trace_*_rcuidle() variant. >>> >>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> >>> --- >> >> This patch triggers backtraces with mips qemu emulations. >> >> WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:36 handle_int+0x128/0x178 >> RCU not watching for tracepoint >> ... >> Call Trace: >> [<ffffffff8012a89c>] show_stack+0x64/0x158 >> [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128 >> [<ffffffff801550f4>] __warn+0xa4/0x1f8 >> [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0 >> [<ffffffff801233e8>] handle_int+0x128/0x178 >> [<ffffffff80f25ff8>] r4k_wait+0x30/0x40 >> [<ffffffff80f26448>] default_idle_call+0xa0/0x330 >> [<ffffffff801b6ccc>] do_idle+0xe4/0x1b8 >> [<ffffffff801b71c4>] cpu_startup_entry+0x34/0x48 >> [<ffffffff80f2689c>] kernel_init+0x0/0x110 >> >> WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:40 trace_hardirqs_on+0x184/0x240 >> RCU not watching for tracepoint >> ... >> Call Trace: >> [<ffffffff8012a89c>] show_stack+0x64/0x158 >> [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128 >> [<ffffffff801550f4>] __warn+0xa4/0x1f8 >> [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0 >> [<ffffffff8028fb74>] trace_hardirqs_on+0x184/0x240 >> [<ffffffff80123030>] restore_partial+0x6c/0x13c >> >> Bisect log is attached. > > I'm guessing MIPS needs to implement something like what arm64 did with: > > 7cd1ea1010acb ("arm64: entry: fix non-NMI kernel<->kernel transitions") > > Because we do not want to bring back the trace_*_rcuidle() crap. > Hmm. If you say so. Note that powerpc has the same or a similar problem. [ 0.142039][ T0] RCU not watching for tracepoint [ 0.142488][ T0] [ 0.142659][ T0] ============================= [ 0.142755][ T0] WARNING: suspicious RCU usage [ 0.142914][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted [ 0.143082][ T0] ----------------------------- [ 0.143178][ T0] kernel/notifier.c:586 notify_die called but RCU thinks we're quiescent! [ 0.152733][ T0] RCU not watching for tracepoint [ 0.152770][ T0] [ 0.152995][ T0] ============================= [ 0.153092][ T0] WARNING: suspicious RCU usage [ 0.153187][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted [ 0.153301][ T0] ----------------------------- [ 0.153394][ T0] include/linux/rcupdate.h:850 rcu_read_lock() used illegally while idle! [ 0.165396][ T0] RCU not watching for tracepoint [ 0.165540][ T0] [ 0.165712][ T0] ============================= [ 0.165811][ T0] WARNING: suspicious RCU usage [ 0.165909][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted [ 0.166026][ T0] ----------------------------- [ 0.166122][ T0] include/linux/rcupdate.h:878 rcu_read_unlock() used illegally while idle! and many more. Guenter
On Tue, 3 Dec 2024 17:48:33 -0800 Guenter Roeck <linux@roeck-us.net> wrote: > Hmm. If you say so. Note that powerpc has the same or a similar problem. > > [ 0.142039][ T0] RCU not watching for tracepoint > [ 0.142488][ T0] > [ 0.142659][ T0] ============================= > [ 0.142755][ T0] WARNING: suspicious RCU usage > [ 0.142914][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted > [ 0.143082][ T0] ----------------------------- > [ 0.143178][ T0] kernel/notifier.c:586 notify_die called but RCU thinks we're quiescent! > > > [ 0.152733][ T0] RCU not watching for tracepoint > [ 0.152770][ T0] > [ 0.152995][ T0] ============================= > [ 0.153092][ T0] WARNING: suspicious RCU usage > [ 0.153187][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted > [ 0.153301][ T0] ----------------------------- > [ 0.153394][ T0] include/linux/rcupdate.h:850 rcu_read_lock() used illegally while idle! > > [ 0.165396][ T0] RCU not watching for tracepoint > [ 0.165540][ T0] > [ 0.165712][ T0] ============================= > [ 0.165811][ T0] WARNING: suspicious RCU usage > [ 0.165909][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted > [ 0.166026][ T0] ----------------------------- > [ 0.166122][ T0] include/linux/rcupdate.h:878 rcu_read_unlock() used illegally while idle! > > and many more. Grumble. It's just that one file. I wonder if we could just do a hack like this? Paul? diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c index 5c03633316a6..58098873efa9 100644 --- a/kernel/trace/trace_preemptirq.c +++ b/kernel/trace/trace_preemptirq.c @@ -10,11 +10,42 @@ #include <linux/module.h> #include <linux/ftrace.h> #include <linux/kprobes.h> +#include <linux/hardirq.h> #include "trace.h" #define CREATE_TRACE_POINTS #include <trace/events/preemptirq.h> +/* + * Use regular trace points on architectures that implement noinstr + * tooling: these calls will only happen with RCU enabled, which can + * use a regular tracepoint. + * + * On older architectures, RCU may not be watching in idle. In that + * case, wake up RCU to watch while calling the tracepoint. These + * aren't NMI-safe - so exclude NMI contexts: + */ +#ifdef CONFIG_ARCH_WANTS_NO_INSTR +#define trace(point, args) trace_##point(args) +#else +#define trace(point, args) \ + do { \ + if (trace_##point##_enabled()) { \ + bool exit_rcu = false; \ + if (in_nmi()) \ + break; \ + if (!IS_ENABLED(CONFIG_TINY_RCU) && \ + is_idle_task(current)) { \ + ct_irq_enter(); \ + exit_rcu = true; \ + } \ + trace_##point(args); \ + if (exit_rcu) \ + ct_irq_exit(); \ + } \ + } while (0) +#endif + #ifdef CONFIG_TRACE_IRQFLAGS /* Per-cpu variable to prevent redundant calls when IRQs already off */ static DEFINE_PER_CPU(int, tracing_irq_cpu); @@ -28,7 +59,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu); void trace_hardirqs_on_prepare(void) { if (this_cpu_read(tracing_irq_cpu)) { - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1); + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); this_cpu_write(tracing_irq_cpu, 0); } @@ -39,7 +70,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare); void trace_hardirqs_on(void) { if (this_cpu_read(tracing_irq_cpu)) { - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1); + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); this_cpu_write(tracing_irq_cpu, 0); } @@ -61,7 +92,7 @@ void trace_hardirqs_off_finish(void) if (!this_cpu_read(tracing_irq_cpu)) { this_cpu_write(tracing_irq_cpu, 1); tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1); + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); } } @@ -75,7 +106,7 @@ void trace_hardirqs_off(void) if (!this_cpu_read(tracing_irq_cpu)) { this_cpu_write(tracing_irq_cpu, 1); tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1); + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); } } EXPORT_SYMBOL(trace_hardirqs_off); @@ -86,13 +117,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off); void trace_preempt_on(unsigned long a0, unsigned long a1) { - trace_preempt_enable(a0, a1); + trace(preempt_enable, TP_ARGS(a0, a1)); tracer_preempt_on(a0, a1); } void trace_preempt_off(unsigned long a0, unsigned long a1) { - trace_preempt_disable(a0, a1); + trace(preempt_disable, TP_ARGS(a0, a1)); tracer_preempt_off(a0, a1); } #endif I tested this by forcing x86 to use this code, and it appeared to work. -- Steve
On Tue, Dec 03, 2024 at 10:01:53PM -0500, Steven Rostedt wrote: > On Tue, 3 Dec 2024 17:48:33 -0800 > Guenter Roeck <linux@roeck-us.net> wrote: > > > Hmm. If you say so. Note that powerpc has the same or a similar problem. > > > > [ 0.142039][ T0] RCU not watching for tracepoint > > [ 0.142488][ T0] > > [ 0.142659][ T0] ============================= > > [ 0.142755][ T0] WARNING: suspicious RCU usage > > [ 0.142914][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted > > [ 0.143082][ T0] ----------------------------- > > [ 0.143178][ T0] kernel/notifier.c:586 notify_die called but RCU thinks we're quiescent! > > > > > > [ 0.152733][ T0] RCU not watching for tracepoint > > [ 0.152770][ T0] > > [ 0.152995][ T0] ============================= > > [ 0.153092][ T0] WARNING: suspicious RCU usage > > [ 0.153187][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted > > [ 0.153301][ T0] ----------------------------- > > [ 0.153394][ T0] include/linux/rcupdate.h:850 rcu_read_lock() used illegally while idle! > > > > [ 0.165396][ T0] RCU not watching for tracepoint > > [ 0.165540][ T0] > > [ 0.165712][ T0] ============================= > > [ 0.165811][ T0] WARNING: suspicious RCU usage > > [ 0.165909][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted > > [ 0.166026][ T0] ----------------------------- > > [ 0.166122][ T0] include/linux/rcupdate.h:878 rcu_read_unlock() used illegally while idle! > > > > and many more. > > Grumble. It's just that one file. I wonder if we could just do a hack like > this? > > Paul? Looks plausible to me, though I don't understand why the introduction of trace() doesn't permit removal of the corresponding current code. (Or did I miss a previous patch that did just that?) Thanx, Paul > diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c > index 5c03633316a6..58098873efa9 100644 > --- a/kernel/trace/trace_preemptirq.c > +++ b/kernel/trace/trace_preemptirq.c > @@ -10,11 +10,42 @@ > #include <linux/module.h> > #include <linux/ftrace.h> > #include <linux/kprobes.h> > +#include <linux/hardirq.h> > #include "trace.h" > > #define CREATE_TRACE_POINTS > #include <trace/events/preemptirq.h> > > +/* > + * Use regular trace points on architectures that implement noinstr > + * tooling: these calls will only happen with RCU enabled, which can > + * use a regular tracepoint. > + * > + * On older architectures, RCU may not be watching in idle. In that > + * case, wake up RCU to watch while calling the tracepoint. These > + * aren't NMI-safe - so exclude NMI contexts: > + */ > +#ifdef CONFIG_ARCH_WANTS_NO_INSTR > +#define trace(point, args) trace_##point(args) > +#else > +#define trace(point, args) \ > + do { \ > + if (trace_##point##_enabled()) { \ > + bool exit_rcu = false; \ > + if (in_nmi()) \ > + break; \ > + if (!IS_ENABLED(CONFIG_TINY_RCU) && \ > + is_idle_task(current)) { \ > + ct_irq_enter(); \ > + exit_rcu = true; \ > + } \ > + trace_##point(args); \ > + if (exit_rcu) \ > + ct_irq_exit(); \ > + } \ > + } while (0) > +#endif > + > #ifdef CONFIG_TRACE_IRQFLAGS > /* Per-cpu variable to prevent redundant calls when IRQs already off */ > static DEFINE_PER_CPU(int, tracing_irq_cpu); > @@ -28,7 +59,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu); > void trace_hardirqs_on_prepare(void) > { > if (this_cpu_read(tracing_irq_cpu)) { > - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); > this_cpu_write(tracing_irq_cpu, 0); > } > @@ -39,7 +70,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare); > void trace_hardirqs_on(void) > { > if (this_cpu_read(tracing_irq_cpu)) { > - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); > this_cpu_write(tracing_irq_cpu, 0); > } > @@ -61,7 +92,7 @@ void trace_hardirqs_off_finish(void) > if (!this_cpu_read(tracing_irq_cpu)) { > this_cpu_write(tracing_irq_cpu, 1); > tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); > - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > } > > } > @@ -75,7 +106,7 @@ void trace_hardirqs_off(void) > if (!this_cpu_read(tracing_irq_cpu)) { > this_cpu_write(tracing_irq_cpu, 1); > tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); > - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > } > } > EXPORT_SYMBOL(trace_hardirqs_off); > @@ -86,13 +117,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off); > > void trace_preempt_on(unsigned long a0, unsigned long a1) > { > - trace_preempt_enable(a0, a1); > + trace(preempt_enable, TP_ARGS(a0, a1)); > tracer_preempt_on(a0, a1); > } > > void trace_preempt_off(unsigned long a0, unsigned long a1) > { > - trace_preempt_disable(a0, a1); > + trace(preempt_disable, TP_ARGS(a0, a1)); > tracer_preempt_off(a0, a1); > } > #endif > > > I tested this by forcing x86 to use this code, and it appeared to work. > > -- Steve
On 12/3/24 19:01, Steven Rostedt wrote: > On Tue, 3 Dec 2024 17:48:33 -0800 > Guenter Roeck <linux@roeck-us.net> wrote: > >> Hmm. If you say so. Note that powerpc has the same or a similar problem. >> >> [ 0.142039][ T0] RCU not watching for tracepoint >> [ 0.142488][ T0] >> [ 0.142659][ T0] ============================= >> [ 0.142755][ T0] WARNING: suspicious RCU usage >> [ 0.142914][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted >> [ 0.143082][ T0] ----------------------------- >> [ 0.143178][ T0] kernel/notifier.c:586 notify_die called but RCU thinks we're quiescent! >> >> >> [ 0.152733][ T0] RCU not watching for tracepoint >> [ 0.152770][ T0] >> [ 0.152995][ T0] ============================= >> [ 0.153092][ T0] WARNING: suspicious RCU usage >> [ 0.153187][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted >> [ 0.153301][ T0] ----------------------------- >> [ 0.153394][ T0] include/linux/rcupdate.h:850 rcu_read_lock() used illegally while idle! >> >> [ 0.165396][ T0] RCU not watching for tracepoint >> [ 0.165540][ T0] >> [ 0.165712][ T0] ============================= >> [ 0.165811][ T0] WARNING: suspicious RCU usage >> [ 0.165909][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted >> [ 0.166026][ T0] ----------------------------- >> [ 0.166122][ T0] include/linux/rcupdate.h:878 rcu_read_unlock() used illegally while idle! >> >> and many more. > > Grumble. It's just that one file. I wonder if we could just do a hack like > this? > The code below fixes the problem for both mips and powerpc. Tested-by: Guenter Roeck <linux@roeck-us.net> Thanks, Guenter > Paul? > > diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c > index 5c03633316a6..58098873efa9 100644 > --- a/kernel/trace/trace_preemptirq.c > +++ b/kernel/trace/trace_preemptirq.c > @@ -10,11 +10,42 @@ > #include <linux/module.h> > #include <linux/ftrace.h> > #include <linux/kprobes.h> > +#include <linux/hardirq.h> > #include "trace.h" > > #define CREATE_TRACE_POINTS > #include <trace/events/preemptirq.h> > > +/* > + * Use regular trace points on architectures that implement noinstr > + * tooling: these calls will only happen with RCU enabled, which can > + * use a regular tracepoint. > + * > + * On older architectures, RCU may not be watching in idle. In that > + * case, wake up RCU to watch while calling the tracepoint. These > + * aren't NMI-safe - so exclude NMI contexts: > + */ > +#ifdef CONFIG_ARCH_WANTS_NO_INSTR > +#define trace(point, args) trace_##point(args) > +#else > +#define trace(point, args) \ > + do { \ > + if (trace_##point##_enabled()) { \ > + bool exit_rcu = false; \ > + if (in_nmi()) \ > + break; \ > + if (!IS_ENABLED(CONFIG_TINY_RCU) && \ > + is_idle_task(current)) { \ > + ct_irq_enter(); \ > + exit_rcu = true; \ > + } \ > + trace_##point(args); \ > + if (exit_rcu) \ > + ct_irq_exit(); \ > + } \ > + } while (0) > +#endif > + > #ifdef CONFIG_TRACE_IRQFLAGS > /* Per-cpu variable to prevent redundant calls when IRQs already off */ > static DEFINE_PER_CPU(int, tracing_irq_cpu); > @@ -28,7 +59,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu); > void trace_hardirqs_on_prepare(void) > { > if (this_cpu_read(tracing_irq_cpu)) { > - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); > this_cpu_write(tracing_irq_cpu, 0); > } > @@ -39,7 +70,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare); > void trace_hardirqs_on(void) > { > if (this_cpu_read(tracing_irq_cpu)) { > - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); > this_cpu_write(tracing_irq_cpu, 0); > } > @@ -61,7 +92,7 @@ void trace_hardirqs_off_finish(void) > if (!this_cpu_read(tracing_irq_cpu)) { > this_cpu_write(tracing_irq_cpu, 1); > tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); > - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > } > > } > @@ -75,7 +106,7 @@ void trace_hardirqs_off(void) > if (!this_cpu_read(tracing_irq_cpu)) { > this_cpu_write(tracing_irq_cpu, 1); > tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); > - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > } > } > EXPORT_SYMBOL(trace_hardirqs_off); > @@ -86,13 +117,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off); > > void trace_preempt_on(unsigned long a0, unsigned long a1) > { > - trace_preempt_enable(a0, a1); > + trace(preempt_enable, TP_ARGS(a0, a1)); > tracer_preempt_on(a0, a1); > } > > void trace_preempt_off(unsigned long a0, unsigned long a1) > { > - trace_preempt_disable(a0, a1); > + trace(preempt_disable, TP_ARGS(a0, a1)); > tracer_preempt_off(a0, a1); > } > #endif > > > I tested this by forcing x86 to use this code, and it appeared to work. > > -- Steve
Hi Steven, On Wed, Dec 4, 2024 at 4:31 AM Steven Rostedt <rostedt@goodmis.org> wrote: > On Tue, 3 Dec 2024 17:48:33 -0800 > Guenter Roeck <linux@roeck-us.net> wrote: > > > Hmm. If you say so. Note that powerpc has the same or a similar problem. > > > > [ 0.142039][ T0] RCU not watching for tracepoint > > [ 0.142488][ T0] > > [ 0.142659][ T0] ============================= > > [ 0.142755][ T0] WARNING: suspicious RCU usage > > [ 0.142914][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted > > [ 0.143082][ T0] ----------------------------- > > [ 0.143178][ T0] kernel/notifier.c:586 notify_die called but RCU thinks we're quiescent! > > > > > > [ 0.152733][ T0] RCU not watching for tracepoint > > [ 0.152770][ T0] > > [ 0.152995][ T0] ============================= > > [ 0.153092][ T0] WARNING: suspicious RCU usage > > [ 0.153187][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted > > [ 0.153301][ T0] ----------------------------- > > [ 0.153394][ T0] include/linux/rcupdate.h:850 rcu_read_lock() used illegally while idle! > > > > [ 0.165396][ T0] RCU not watching for tracepoint > > [ 0.165540][ T0] > > [ 0.165712][ T0] ============================= > > [ 0.165811][ T0] WARNING: suspicious RCU usage > > [ 0.165909][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted > > [ 0.166026][ T0] ----------------------------- > > [ 0.166122][ T0] include/linux/rcupdate.h:878 rcu_read_unlock() used illegally while idle! > > > > and many more. > > Grumble. It's just that one file. I wonder if we could just do a hack like > this? Thanks, this fixes the issue during ARM s2ram, so Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/kernel/trace/trace_preemptirq.c > +++ b/kernel/trace/trace_preemptirq.c > @@ -10,11 +10,42 @@ > #include <linux/module.h> > #include <linux/ftrace.h> > #include <linux/kprobes.h> > +#include <linux/hardirq.h> > #include "trace.h" > > #define CREATE_TRACE_POINTS > #include <trace/events/preemptirq.h> > > +/* > + * Use regular trace points on architectures that implement noinstr > + * tooling: these calls will only happen with RCU enabled, which can > + * use a regular tracepoint. > + * > + * On older architectures, RCU may not be watching in idle. In that > + * case, wake up RCU to watch while calling the tracepoint. These > + * aren't NMI-safe - so exclude NMI contexts: > + */ > +#ifdef CONFIG_ARCH_WANTS_NO_INSTR > +#define trace(point, args) trace_##point(args) > +#else > +#define trace(point, args) \ > + do { \ > + if (trace_##point##_enabled()) { \ > + bool exit_rcu = false; \ > + if (in_nmi()) \ > + break; \ > + if (!IS_ENABLED(CONFIG_TINY_RCU) && \ > + is_idle_task(current)) { \ > + ct_irq_enter(); \ > + exit_rcu = true; \ > + } \ > + trace_##point(args); \ > + if (exit_rcu) \ > + ct_irq_exit(); \ > + } \ > + } while (0) > +#endif > + > #ifdef CONFIG_TRACE_IRQFLAGS > /* Per-cpu variable to prevent redundant calls when IRQs already off */ > static DEFINE_PER_CPU(int, tracing_irq_cpu); > @@ -28,7 +59,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu); > void trace_hardirqs_on_prepare(void) > { > if (this_cpu_read(tracing_irq_cpu)) { > - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); > this_cpu_write(tracing_irq_cpu, 0); > } > @@ -39,7 +70,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare); > void trace_hardirqs_on(void) > { > if (this_cpu_read(tracing_irq_cpu)) { > - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); > this_cpu_write(tracing_irq_cpu, 0); > } > @@ -61,7 +92,7 @@ void trace_hardirqs_off_finish(void) > if (!this_cpu_read(tracing_irq_cpu)) { > this_cpu_write(tracing_irq_cpu, 1); > tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); > - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > } > > } > @@ -75,7 +106,7 @@ void trace_hardirqs_off(void) > if (!this_cpu_read(tracing_irq_cpu)) { > this_cpu_write(tracing_irq_cpu, 1); > tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); > - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1); > + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1)); > } > } > EXPORT_SYMBOL(trace_hardirqs_off); > @@ -86,13 +117,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off); > > void trace_preempt_on(unsigned long a0, unsigned long a1) > { > - trace_preempt_enable(a0, a1); > + trace(preempt_enable, TP_ARGS(a0, a1)); > tracer_preempt_on(a0, a1); > } > > void trace_preempt_off(unsigned long a0, unsigned long a1) > { > - trace_preempt_disable(a0, a1); > + trace(preempt_disable, TP_ARGS(a0, a1)); > tracer_preempt_off(a0, a1); > } > #endif Gr{oetje,eeting}s, Geert
On Tue, 3 Dec 2024 21:01:59 -0800 "Paul E. McKenney" <paulmck@kernel.org> wrote: > > Paul? > > Looks plausible to me, though I don't understand why the introduction > of trace() doesn't permit removal of the corresponding current code. > (Or did I miss a previous patch that did just that?) > I removed the trace_*_rcuidle() code, but this file still used it. I didn't realize that removing the trace_*_rcuidle() in this file would break other architectures. This patch is a work around to not need to re-introduce the trace_*_rcuidle() code. -- Steve
On Wed, Dec 04, 2024 at 09:33:56AM -0500, Steven Rostedt wrote: > On Tue, 3 Dec 2024 21:01:59 -0800 > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > Paul? > > > > Looks plausible to me, though I don't understand why the introduction > > of trace() doesn't permit removal of the corresponding current code. > > (Or did I miss a previous patch that did just that?) > > > > I removed the trace_*_rcuidle() code, but this file still used it. I didn't > realize that removing the trace_*_rcuidle() in this file would break other > architectures. > > This patch is a work around to not need to re-introduce the > trace_*_rcuidle() code. Works for me! Acked-by: Paul E. McKenney <paulmck@kernel.org> Thanx, Paul
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h index 93a9f3070b48..9b85a02b8575 100644 --- a/include/linux/tracepoint.h +++ b/include/linux/tracepoint.h @@ -196,67 +196,25 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) #define __DO_TRACE_CALL(name, args) __traceiter_##name(NULL, args) #endif /* CONFIG_HAVE_STATIC_CALL */ -/* - * ARCH_WANTS_NO_INSTR archs are expected to have sanitized entry and idle - * code that disallow any/all tracing/instrumentation when RCU isn't watching. - */ -#ifdef CONFIG_ARCH_WANTS_NO_INSTR -#define RCUIDLE_COND(rcuidle) (rcuidle) -#else -/* srcu can't be used from NMI */ -#define RCUIDLE_COND(rcuidle) (rcuidle && in_nmi()) -#endif - /* * it_func[0] is never NULL because there is at least one element in the array * when the array itself is non NULL. */ -#define __DO_TRACE(name, args, cond, rcuidle) \ +#define __DO_TRACE(name, args, cond) \ do { \ int __maybe_unused __idx = 0; \ \ if (!(cond)) \ return; \ \ - if (WARN_ONCE(RCUIDLE_COND(rcuidle), \ - "Bad RCU usage for tracepoint")) \ - return; \ - \ /* keep srcu and sched-rcu usage consistent */ \ preempt_disable_notrace(); \ \ - /* \ - * For rcuidle callers, use srcu since sched-rcu \ - * doesn't work from the idle path. \ - */ \ - if (rcuidle) { \ - __idx = srcu_read_lock_notrace(&tracepoint_srcu);\ - ct_irq_enter_irqson(); \ - } \ - \ __DO_TRACE_CALL(name, TP_ARGS(args)); \ \ - if (rcuidle) { \ - ct_irq_exit_irqson(); \ - srcu_read_unlock_notrace(&tracepoint_srcu, __idx);\ - } \ - \ preempt_enable_notrace(); \ } while (0) -#ifndef MODULE -#define __DECLARE_TRACE_RCU(name, proto, args, cond) \ - static inline void trace_##name##_rcuidle(proto) \ - { \ - if (static_key_false(&__tracepoint_##name.key)) \ - __DO_TRACE(name, \ - TP_ARGS(args), \ - TP_CONDITION(cond), 1); \ - } -#else -#define __DECLARE_TRACE_RCU(name, proto, args, cond) -#endif - /* * Make sure the alignment of the structure in the __tracepoints section will * not add unwanted padding between the beginning of the section and the @@ -277,14 +235,12 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) if (static_key_false(&__tracepoint_##name.key)) \ __DO_TRACE(name, \ TP_ARGS(args), \ - TP_CONDITION(cond), 0); \ + TP_CONDITION(cond)); \ if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \ WARN_ONCE(!rcu_is_watching(), \ "RCU not watching for tracepoint"); \ } \ } \ - __DECLARE_TRACE_RCU(name, PARAMS(proto), PARAMS(args), \ - PARAMS(cond)) \ static inline int \ register_trace_##name(void (*probe)(data_proto), void *data) \ { \ @@ -375,8 +331,6 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) #define __DECLARE_TRACE(name, proto, args, cond, data_proto) \ static inline void trace_##name(proto) \ { } \ - static inline void trace_##name##_rcuidle(proto) \ - { } \ static inline int \ register_trace_##name(void (*probe)(data_proto), \ void *data) \ diff --git a/include/trace/events/preemptirq.h b/include/trace/events/preemptirq.h index 3f249e150c0c..f99562d2b496 100644 --- a/include/trace/events/preemptirq.h +++ b/include/trace/events/preemptirq.h @@ -43,8 +43,6 @@ DEFINE_EVENT(preemptirq_template, irq_enable, #else #define trace_irq_enable(...) #define trace_irq_disable(...) -#define trace_irq_enable_rcuidle(...) -#define trace_irq_disable_rcuidle(...) #endif #ifdef CONFIG_TRACE_PREEMPT_TOGGLE @@ -58,8 +56,6 @@ DEFINE_EVENT(preemptirq_template, preempt_enable, #else #define trace_preempt_enable(...) #define trace_preempt_disable(...) -#define trace_preempt_enable_rcuidle(...) -#define trace_preempt_disable_rcuidle(...) #endif #endif /* _TRACE_PREEMPTIRQ_H */ @@ -69,10 +65,6 @@ DEFINE_EVENT(preemptirq_template, preempt_enable, #else /* !CONFIG_PREEMPTIRQ_TRACEPOINTS */ #define trace_irq_enable(...) #define trace_irq_disable(...) -#define trace_irq_enable_rcuidle(...) -#define trace_irq_disable_rcuidle(...) #define trace_preempt_enable(...) #define trace_preempt_disable(...) -#define trace_preempt_enable_rcuidle(...) -#define trace_preempt_disable_rcuidle(...) #endif diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c index e37446f7916e..5c03633316a6 100644 --- a/kernel/trace/trace_preemptirq.c +++ b/kernel/trace/trace_preemptirq.c @@ -15,20 +15,6 @@ #define CREATE_TRACE_POINTS #include <trace/events/preemptirq.h> -/* - * Use regular trace points on architectures that implement noinstr - * tooling: these calls will only happen with RCU enabled, which can - * use a regular tracepoint. - * - * On older architectures, use the rcuidle tracing methods (which - * aren't NMI-safe - so exclude NMI contexts): - */ -#ifdef CONFIG_ARCH_WANTS_NO_INSTR -#define trace(point) trace_##point -#else -#define trace(point) if (!in_nmi()) trace_##point##_rcuidle -#endif - #ifdef CONFIG_TRACE_IRQFLAGS /* Per-cpu variable to prevent redundant calls when IRQs already off */ static DEFINE_PER_CPU(int, tracing_irq_cpu); @@ -42,7 +28,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu); void trace_hardirqs_on_prepare(void) { if (this_cpu_read(tracing_irq_cpu)) { - trace(irq_enable)(CALLER_ADDR0, CALLER_ADDR1); + trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1); tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); this_cpu_write(tracing_irq_cpu, 0); } @@ -53,7 +39,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare); void trace_hardirqs_on(void) { if (this_cpu_read(tracing_irq_cpu)) { - trace(irq_enable)(CALLER_ADDR0, CALLER_ADDR1); + trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1); tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); this_cpu_write(tracing_irq_cpu, 0); } @@ -75,7 +61,7 @@ void trace_hardirqs_off_finish(void) if (!this_cpu_read(tracing_irq_cpu)) { this_cpu_write(tracing_irq_cpu, 1); tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); - trace(irq_disable)(CALLER_ADDR0, CALLER_ADDR1); + trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1); } } @@ -89,7 +75,7 @@ void trace_hardirqs_off(void) if (!this_cpu_read(tracing_irq_cpu)) { this_cpu_write(tracing_irq_cpu, 1); tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); - trace(irq_disable)(CALLER_ADDR0, CALLER_ADDR1); + trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1); } } EXPORT_SYMBOL(trace_hardirqs_off); @@ -100,13 +86,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off); void trace_preempt_on(unsigned long a0, unsigned long a1) { - trace(preempt_enable)(a0, a1); + trace_preempt_enable(a0, a1); tracer_preempt_on(a0, a1); } void trace_preempt_off(unsigned long a0, unsigned long a1) { - trace(preempt_disable)(a0, a1); + trace_preempt_disable(a0, a1); tracer_preempt_off(a0, a1); } #endif diff --git a/scripts/tags.sh b/scripts/tags.sh index 191e0461d6d5..0d01c1cafb70 100755 --- a/scripts/tags.sh +++ b/scripts/tags.sh @@ -152,9 +152,7 @@ regex_c=( '/^BPF_CALL_[0-9]([[:space:]]*\([[:alnum:]_]*\).*/\1/' '/^COMPAT_SYSCALL_DEFINE[0-9]([[:space:]]*\([[:alnum:]_]*\).*/compat_sys_\1/' '/^TRACE_EVENT([[:space:]]*\([[:alnum:]_]*\).*/trace_\1/' - '/^TRACE_EVENT([[:space:]]*\([[:alnum:]_]*\).*/trace_\1_rcuidle/' '/^DEFINE_EVENT([^,)]*,[[:space:]]*\([[:alnum:]_]*\).*/trace_\1/' - '/^DEFINE_EVENT([^,)]*,[[:space:]]*\([[:alnum:]_]*\).*/trace_\1_rcuidle/' '/^DEFINE_INSN_CACHE_OPS([[:space:]]*\([[:alnum:]_]*\).*/get_\1_slot/' '/^DEFINE_INSN_CACHE_OPS([[:space:]]*\([[:alnum:]_]*\).*/free_\1_slot/' '/^PAGEFLAG([[:space:]]*\([[:alnum:]_]*\).*/Page\1/'