[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aNOrMNYJipF_nTGD@gpd4>
Date: Wed, 24 Sep 2025 10:26:24 +0200
From: Andrea Righi <arighi@...dia.com>
To: Tejun Heo <tj@...nel.org>
Cc: David Vernet <void@...ifault.com>, Changwoo Min <changwoo@...lia.com>,
linux-kernel@...r.kernel.org, sched-ext@...ts.linux.dev
Subject: Re: [PATCH 1/7] sched_ext: Use rhashtable_lookup() instead of
rhashtable_lookup_fast()
On Tue, Sep 23, 2025 at 08:38:05PM -1000, Tejun Heo wrote:
> Hello,
>
> On Wed, Sep 24, 2025 at 08:14:03AM +0200, Andrea Righi wrote:
> > Hi Tejun
> >
> > On Sun, Sep 21, 2025 at 03:32:40PM -1000, Tejun Heo wrote:
> > > The find_user_dsq() function is called from contexts that are already
> > > under RCU read lock protection. Switch from rhashtable_lookup_fast() to
> > > rhashtable_lookup() to avoid redundant RCU locking.
> > >
> > > Signed-off-by: Tejun Heo <tj@...nel.org>
> >
> > It looks like the ttwu_queue() path isn't RCU read lock protected.
> > With this applied:
> >
> > [ 6.647598] =============================
> > [ 6.647603] WARNING: suspicious RCU usage
> > [ 6.647605] 6.17.0-rc7-virtme #1 Not tainted
> > [ 6.647608] -----------------------------
> > [ 6.647608] ./include/linux/rhashtable.h:602 suspicious rcu_dereference_check() usage!
> > [ 6.647610]
> > [ 6.647610] other info that might help us debug this:
> > [ 6.647610]
> > [ 6.647612]
> > [ 6.647612] rcu_scheduler_active = 2, debug_locks = 1
> > [ 6.647613] 1 lock held by swapper/10/0:
> > [ 6.647614] #0: ffff8b14bbb3cc98 (&rq->__lock){-.-.}-{2:2}, at: raw_spin_rq_lock_nested+0x20/0x90
> > [ 6.647630]
> > [ 6.647630] stack backtrace:
> > [ 6.647633] CPU: 10 UID: 0 PID: 0 Comm: swapper/10 Not tainted 6.17.0-rc7-virtme #1 PREEMPT(full)
> > [ 6.647643] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> > [ 6.647646] Sched_ext: beerland_1.0.2_g27d63fc3_x86_64_unknown_linux_gnu (enabled+all)
> > [ 6.647648] Call Trace:
> > [ 6.647652] <IRQ>
> > [ 6.647655] dump_stack_lvl+0x78/0xe0
> > [ 6.647665] lockdep_rcu_suspicious+0x14a/0x1b0
> > [ 6.647672] __rhashtable_lookup.constprop.0+0x1d5/0x250
> > [ 6.647680] find_dsq_for_dispatch+0xbc/0x190
> > [ 6.647684] do_enqueue_task+0x25b/0x550
> > [ 6.647689] enqueue_task_scx+0x21d/0x360
> > [ 6.647692] ? trace_lock_acquire+0x22/0xb0
> > [ 6.647695] enqueue_task+0x2e/0xd0
> > [ 6.647698] ttwu_do_activate+0xa2/0x290
> > [ 6.647703] sched_ttwu_pending+0xfd/0x250
> > [ 6.647706] __flush_smp_call_function_queue+0x1cd/0x610
> > [ 6.647714] __sysvec_call_function_single+0x34/0x150
> > [ 6.647720] sysvec_call_function_single+0x6e/0x80
> > [ 6.647726] </IRQ>
> > [ 6.647726] <TASK>
> > [ 6.647727] asm_sysvec_call_function_single+0x1a/0x20
> >
> > Should we revert this?
>
> IRQ is disabled, so it is in RCU protected region but the lockdep annotation
> isn't happy with it. :-( I'll revert the patch for now.
We just need this: https://lore.kernel.org/all/aL_4gCJibYMW0J98@gondor.apana.org.au/
With this one lockdep is happy.
-Andrea
Powered by blists - more mailing lists