Re: [BUG BISECTED] Missing RCU reader in perf_event_setup_cpumask()

[Peter Zijlstra]

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) {

?