* [BUG BISECTED] Missing RCU reader in perf_event_setup_cpumask()
@ 2024-09-13 8:00 Paul E. McKenney
2024-09-13 10:47 ` Peter Zijlstra
0 siblings, 1 reply; 4+ messages in thread
From: Paul E. McKenney @ 2024-09-13 8:00 UTC (permalink / raw)
To: linux-kernel, linux-perf-users
Cc: kan.liang, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter
Hello!
On next-20240912 running rcutorture scenario TREE05, I see this
deterministically:
[ 32.603233] =============================
[ 32.604594] WARNING: suspicious RCU usage
[ 32.605928] 6.11.0-rc5-00040-g4ba4f1afb6a9 #55238 Not tainted
[ 32.607812] -----------------------------
[ 32.609140] kernel/events/core.c:13946 RCU-list traversed in non-reader section!!
[ 32.611595]
[ 32.611595] other info that might help us debug this:
[ 32.611595]
[ 32.614247]
[ 32.614247] rcu_scheduler_active = 2, debug_locks = 1
[ 32.616392] 3 locks held by cpuhp/4/35:
[ 32.617687] #0: ffffffffb666a650 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x4e/0x200
[ 32.620563] #1: ffffffffb666cd20 (cpuhp_state-down){+.+.}-{0:0}, at: cpuhp_thread_fun+0x4e/0x200
[ 32.623412] #2: ffffffffb677c288 (pmus_lock){+.+.}-{3:3}, at: perf_event_exit_cpu_context+0x32/0x2f0
[ 32.626399]
[ 32.626399] stack backtrace:
[ 32.627848] CPU: 4 UID: 0 PID: 35 Comm: cpuhp/4 Not tainted 6.11.0-rc5-00040-g4ba4f1afb6a9 #55238
[ 32.628832] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
[ 32.628832] Call Trace:
[ 32.628832] <TASK>
[ 32.628832] dump_stack_lvl+0x83/0xa0
[ 32.628832] lockdep_rcu_suspicious+0x143/0x1a0
[ 32.628832] perf_event_exit_cpu_context+0x2e5/0x2f0
[ 32.628832] ? __pfx_perf_event_exit_cpu+0x10/0x10
[ 32.628832] perf_event_exit_cpu+0x9/0x10
[ 32.628832] cpuhp_invoke_callback+0x130/0x2a0
[ 32.628832] ? lock_release+0xc7/0x290
[ 32.628832] ? cpuhp_thread_fun+0x4e/0x200
[ 32.628832] cpuhp_thread_fun+0x183/0x200
[ 32.628832] smpboot_thread_fn+0xd8/0x1d0
[ 32.628832] ? __pfx_smpboot_thread_fn+0x10/0x10
[ 32.628832] kthread+0xd4/0x100
[ 32.628832] ? __pfx_kthread+0x10/0x10
[ 32.628832] ret_from_fork+0x2f/0x50
[ 32.628832] ? __pfx_kthread+0x10/0x10
[ 32.628832] ret_from_fork_asm+0x1a/0x30
[ 32.628832] </TASK>
I bisected this to:
4ba4f1afb6a9 ("perf: Generic hotplug support for a PMU with a scope")
This adds a perf_event_setup_cpumask() function that uses
list_for_each_entry_rcu() without an obvious RCU read-side critical
section, so the fix might be as simple as adding rcu_read_lock() and
rcu_read_unlock(). In the proper places, of course. ;-)
Thanx, Paul
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [BUG BISECTED] Missing RCU reader in perf_event_setup_cpumask()
2024-09-13 8:00 [BUG BISECTED] Missing RCU reader in perf_event_setup_cpumask() Paul E. McKenney
@ 2024-09-13 10:47 ` Peter Zijlstra
2024-09-13 12:09 ` Paul E. McKenney
2024-09-13 15:51 ` Liang, Kan
0 siblings, 2 replies; 4+ messages in thread
From: Peter Zijlstra @ 2024-09-13 10:47 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, linux-perf-users, kan.liang, mingo, acme, namhyung,
mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter
On Fri, Sep 13, 2024 at 01:00:44AM -0700, Paul E. McKenney wrote:
> Hello!
>
> On next-20240912 running rcutorture scenario TREE05, I see this
> deterministically:
>
> [ 32.603233] =============================
> [ 32.604594] WARNING: suspicious RCU usage
> [ 32.605928] 6.11.0-rc5-00040-g4ba4f1afb6a9 #55238 Not tainted
> [ 32.607812] -----------------------------
> [ 32.609140] kernel/events/core.c:13946 RCU-list traversed in non-reader section!!
> [ 32.611595]
> [ 32.611595] other info that might help us debug this:
> [ 32.611595]
> [ 32.614247]
> [ 32.614247] rcu_scheduler_active = 2, debug_locks = 1
> [ 32.616392] 3 locks held by cpuhp/4/35:
> [ 32.617687] #0: ffffffffb666a650 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x4e/0x200
> [ 32.620563] #1: ffffffffb666cd20 (cpuhp_state-down){+.+.}-{0:0}, at: cpuhp_thread_fun+0x4e/0x200
> [ 32.623412] #2: ffffffffb677c288 (pmus_lock){+.+.}-{3:3}, at: perf_event_exit_cpu_context+0x32/0x2f0
> [ 32.626399]
> [ 32.626399] stack backtrace:
> [ 32.627848] CPU: 4 UID: 0 PID: 35 Comm: cpuhp/4 Not tainted 6.11.0-rc5-00040-g4ba4f1afb6a9 #55238
> [ 32.628832] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> [ 32.628832] Call Trace:
> [ 32.628832] <TASK>
> [ 32.628832] dump_stack_lvl+0x83/0xa0
> [ 32.628832] lockdep_rcu_suspicious+0x143/0x1a0
> [ 32.628832] perf_event_exit_cpu_context+0x2e5/0x2f0
> [ 32.628832] ? __pfx_perf_event_exit_cpu+0x10/0x10
> [ 32.628832] perf_event_exit_cpu+0x9/0x10
> [ 32.628832] cpuhp_invoke_callback+0x130/0x2a0
> [ 32.628832] ? lock_release+0xc7/0x290
> [ 32.628832] ? cpuhp_thread_fun+0x4e/0x200
> [ 32.628832] cpuhp_thread_fun+0x183/0x200
> [ 32.628832] smpboot_thread_fn+0xd8/0x1d0
> [ 32.628832] ? __pfx_smpboot_thread_fn+0x10/0x10
> [ 32.628832] kthread+0xd4/0x100
> [ 32.628832] ? __pfx_kthread+0x10/0x10
> [ 32.628832] ret_from_fork+0x2f/0x50
> [ 32.628832] ? __pfx_kthread+0x10/0x10
> [ 32.628832] ret_from_fork_asm+0x1a/0x30
> [ 32.628832] </TASK>
>
> I bisected this to:
>
> 4ba4f1afb6a9 ("perf: Generic hotplug support for a PMU with a scope")
>
> This adds a perf_event_setup_cpumask() function that uses
> list_for_each_entry_rcu() without an obvious RCU read-side critical
> section, so the fix might be as simple as adding rcu_read_lock() and
> rcu_read_unlock(). In the proper places, of course. ;-)
IIRC that condition should be:
lockdep_is_held(&pmus_srcu) || lockdep_is_held(&pmus_lock)
And at this pooint we actually do hold pmus_lock.
But that all begs the question why we're using RCU iteration here to
begin with, as this code seems to be only called from this context.
Kan, is the simple fix to do:
- list_for_each_entry_rcu(pmu, &pmus, entry, lockdep_is_held(&pmus_srcu)) {
+ list_for_each_entry(pmu, &pmus, entry) {
?
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [BUG BISECTED] Missing RCU reader in perf_event_setup_cpumask()
2024-09-13 10:47 ` Peter Zijlstra
@ 2024-09-13 12:09 ` Paul E. McKenney
2024-09-13 15:51 ` Liang, Kan
1 sibling, 0 replies; 4+ messages in thread
From: Paul E. McKenney @ 2024-09-13 12:09 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-perf-users, kan.liang, mingo, acme, namhyung,
mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter
On Fri, Sep 13, 2024 at 12:47:52PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 13, 2024 at 01:00:44AM -0700, Paul E. McKenney wrote:
> > Hello!
> >
> > On next-20240912 running rcutorture scenario TREE05, I see this
> > deterministically:
> >
> > [ 32.603233] =============================
> > [ 32.604594] WARNING: suspicious RCU usage
> > [ 32.605928] 6.11.0-rc5-00040-g4ba4f1afb6a9 #55238 Not tainted
> > [ 32.607812] -----------------------------
> > [ 32.609140] kernel/events/core.c:13946 RCU-list traversed in non-reader section!!
> > [ 32.611595]
> > [ 32.611595] other info that might help us debug this:
> > [ 32.611595]
> > [ 32.614247]
> > [ 32.614247] rcu_scheduler_active = 2, debug_locks = 1
> > [ 32.616392] 3 locks held by cpuhp/4/35:
> > [ 32.617687] #0: ffffffffb666a650 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x4e/0x200
> > [ 32.620563] #1: ffffffffb666cd20 (cpuhp_state-down){+.+.}-{0:0}, at: cpuhp_thread_fun+0x4e/0x200
> > [ 32.623412] #2: ffffffffb677c288 (pmus_lock){+.+.}-{3:3}, at: perf_event_exit_cpu_context+0x32/0x2f0
> > [ 32.626399]
> > [ 32.626399] stack backtrace:
> > [ 32.627848] CPU: 4 UID: 0 PID: 35 Comm: cpuhp/4 Not tainted 6.11.0-rc5-00040-g4ba4f1afb6a9 #55238
> > [ 32.628832] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> > [ 32.628832] Call Trace:
> > [ 32.628832] <TASK>
> > [ 32.628832] dump_stack_lvl+0x83/0xa0
> > [ 32.628832] lockdep_rcu_suspicious+0x143/0x1a0
> > [ 32.628832] perf_event_exit_cpu_context+0x2e5/0x2f0
> > [ 32.628832] ? __pfx_perf_event_exit_cpu+0x10/0x10
> > [ 32.628832] perf_event_exit_cpu+0x9/0x10
> > [ 32.628832] cpuhp_invoke_callback+0x130/0x2a0
> > [ 32.628832] ? lock_release+0xc7/0x290
> > [ 32.628832] ? cpuhp_thread_fun+0x4e/0x200
> > [ 32.628832] cpuhp_thread_fun+0x183/0x200
> > [ 32.628832] smpboot_thread_fn+0xd8/0x1d0
> > [ 32.628832] ? __pfx_smpboot_thread_fn+0x10/0x10
> > [ 32.628832] kthread+0xd4/0x100
> > [ 32.628832] ? __pfx_kthread+0x10/0x10
> > [ 32.628832] ret_from_fork+0x2f/0x50
> > [ 32.628832] ? __pfx_kthread+0x10/0x10
> > [ 32.628832] ret_from_fork_asm+0x1a/0x30
> > [ 32.628832] </TASK>
> >
> > I bisected this to:
> >
> > 4ba4f1afb6a9 ("perf: Generic hotplug support for a PMU with a scope")
> >
> > This adds a perf_event_setup_cpumask() function that uses
> > list_for_each_entry_rcu() without an obvious RCU read-side critical
> > section, so the fix might be as simple as adding rcu_read_lock() and
> > rcu_read_unlock(). In the proper places, of course. ;-)
>
> IIRC that condition should be:
>
> lockdep_is_held(&pmus_srcu) || lockdep_is_held(&pmus_lock)
>
> And at this pooint we actually do hold pmus_lock.
>
> But that all begs the question why we're using RCU iteration here to
> begin with, as this code seems to be only called from this context.
>
> Kan, is the simple fix to do:
>
> - list_for_each_entry_rcu(pmu, &pmus, entry, lockdep_is_held(&pmus_srcu)) {
> + list_for_each_entry(pmu, &pmus, entry) {
>
> ?
It does pass a quick test, for whatever that might be worth. ;-)
So if this turns out to be the right approach:
Tested-by: Paul E. McKenney <paulmck@kernel.org>
Thanx, Paul
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [BUG BISECTED] Missing RCU reader in perf_event_setup_cpumask()
2024-09-13 10:47 ` Peter Zijlstra
2024-09-13 12:09 ` Paul E. McKenney
@ 2024-09-13 15:51 ` Liang, Kan
1 sibling, 0 replies; 4+ messages in thread
From: Liang, Kan @ 2024-09-13 15:51 UTC (permalink / raw)
To: Peter Zijlstra, Paul E. McKenney
Cc: linux-kernel, linux-perf-users, mingo, acme, namhyung,
mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter
On 2024-09-13 6:47 a.m., Peter Zijlstra wrote:
> On Fri, Sep 13, 2024 at 01:00:44AM -0700, Paul E. McKenney wrote:
>> Hello!
>>
>> On next-20240912 running rcutorture scenario TREE05, I see this
>> deterministically:
>>
>> [ 32.603233] =============================
>> [ 32.604594] WARNING: suspicious RCU usage
>> [ 32.605928] 6.11.0-rc5-00040-g4ba4f1afb6a9 #55238 Not tainted
>> [ 32.607812] -----------------------------
>> [ 32.609140] kernel/events/core.c:13946 RCU-list traversed in non-reader section!!
>> [ 32.611595]
>> [ 32.611595] other info that might help us debug this:
>> [ 32.611595]
>> [ 32.614247]
>> [ 32.614247] rcu_scheduler_active = 2, debug_locks = 1
>> [ 32.616392] 3 locks held by cpuhp/4/35:
>> [ 32.617687] #0: ffffffffb666a650 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x4e/0x200
>> [ 32.620563] #1: ffffffffb666cd20 (cpuhp_state-down){+.+.}-{0:0}, at: cpuhp_thread_fun+0x4e/0x200
>> [ 32.623412] #2: ffffffffb677c288 (pmus_lock){+.+.}-{3:3}, at: perf_event_exit_cpu_context+0x32/0x2f0
>> [ 32.626399]
>> [ 32.626399] stack backtrace:
>> [ 32.627848] CPU: 4 UID: 0 PID: 35 Comm: cpuhp/4 Not tainted 6.11.0-rc5-00040-g4ba4f1afb6a9 #55238
>> [ 32.628832] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
>> [ 32.628832] Call Trace:
>> [ 32.628832] <TASK>
>> [ 32.628832] dump_stack_lvl+0x83/0xa0
>> [ 32.628832] lockdep_rcu_suspicious+0x143/0x1a0
>> [ 32.628832] perf_event_exit_cpu_context+0x2e5/0x2f0
>> [ 32.628832] ? __pfx_perf_event_exit_cpu+0x10/0x10
>> [ 32.628832] perf_event_exit_cpu+0x9/0x10
>> [ 32.628832] cpuhp_invoke_callback+0x130/0x2a0
>> [ 32.628832] ? lock_release+0xc7/0x290
>> [ 32.628832] ? cpuhp_thread_fun+0x4e/0x200
>> [ 32.628832] cpuhp_thread_fun+0x183/0x200
>> [ 32.628832] smpboot_thread_fn+0xd8/0x1d0
>> [ 32.628832] ? __pfx_smpboot_thread_fn+0x10/0x10
>> [ 32.628832] kthread+0xd4/0x100
>> [ 32.628832] ? __pfx_kthread+0x10/0x10
>> [ 32.628832] ret_from_fork+0x2f/0x50
>> [ 32.628832] ? __pfx_kthread+0x10/0x10
>> [ 32.628832] ret_from_fork_asm+0x1a/0x30
>> [ 32.628832] </TASK>
>>
>> I bisected this to:
>>
>> 4ba4f1afb6a9 ("perf: Generic hotplug support for a PMU with a scope")
>>
>> This adds a perf_event_setup_cpumask() function that uses
>> list_for_each_entry_rcu() without an obvious RCU read-side critical
>> section, so the fix might be as simple as adding rcu_read_lock() and
>> rcu_read_unlock(). In the proper places, of course. ;-)
>
> IIRC that condition should be:
>
> lockdep_is_held(&pmus_srcu) || lockdep_is_held(&pmus_lock)
>
> And at this pooint we actually do hold pmus_lock.
>
> But that all begs the question why we're using RCU iteration here to
> begin with, as this code seems to be only called from this context.
I think I just copied and paste the PMU iterate code here, and forget to
add the srcu_read_lock(). Sorry for it.
>
> Kan, is the simple fix to do:
>
> - list_for_each_entry_rcu(pmu, &pmus, entry, lockdep_is_held(&pmus_srcu)) {
> + list_for_each_entry(pmu, &pmus, entry) {
>
> ?
>
Yes, the &pmus_lock protect is good enough. we don't need the rcu here.
I will post a patch with the suggested fix.
Thanks,
Kan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-09-13 15:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-13 8:00 [BUG BISECTED] Missing RCU reader in perf_event_setup_cpumask() Paul E. McKenney
2024-09-13 10:47 ` Peter Zijlstra
2024-09-13 12:09 ` Paul E. McKenney
2024-09-13 15:51 ` Liang, Kan
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).