public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>,
	linux-kernel@vger.kernel.org, sched-ext@lists.linux.dev
Subject: Re: [PATCH 1/7] sched_ext: Use rhashtable_lookup() instead of rhashtable_lookup_fast()
Date: Wed, 24 Sep 2025 10:26:24 +0200	[thread overview]
Message-ID: <aNOrMNYJipF_nTGD@gpd4> (raw)
In-Reply-To: <aNORzVQ6OVkPHAjS@slm.duckdns.org>

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@kernel.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

  reply	other threads:[~2025-09-24  8:26 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-22  1:32 [PATCH 1/7] sched_ext: Use rhashtable_lookup() instead of rhashtable_lookup_fast() Tejun Heo
2025-09-22  1:32 ` [PATCH 2/7] sched_ext: Improve SCX_KF_DISPATCH comment Tejun Heo
2025-09-23  7:20   ` Andrea Righi
2025-09-22  1:32 ` [PATCH 3/7] sched_ext: Fix stray scx_root usage in task_can_run_on_remote_rq() Tejun Heo
2025-09-23  7:22   ` Andrea Righi
2025-09-22  1:32 ` [PATCH 4/7] sched_ext: Use bitfields for boolean warning flags Tejun Heo
2025-09-23  7:45   ` Andrea Righi
2025-09-23 16:00     ` Tejun Heo
2025-09-23 16:58   ` [PATCH v2 " Tejun Heo
2025-09-23 18:45     ` Andrea Righi
2025-09-22  1:32 ` [PATCH 5/7] sched_ext: Add SCX_EFLAG_INITIALIZED to indicate successful ops.init() Tejun Heo
2025-09-23  8:08   ` Andrea Righi
2025-09-22  1:32 ` [PATCH 6/7] sched_ext: Make qmap dump operation non-destructive Tejun Heo
2025-09-23  8:09   ` Andrea Righi
2025-09-22  1:32 ` [PATCH 7/7] tools/sched_ext: scx_qmap: Make debug output quieter by default Tejun Heo
2025-09-23  8:11   ` Andrea Righi
2025-09-23  7:18 ` [PATCH 1/7] sched_ext: Use rhashtable_lookup() instead of rhashtable_lookup_fast() Andrea Righi
2025-09-23 15:59   ` Tejun Heo
2025-09-23 18:47     ` Andrea Righi
2025-09-23 19:13 ` Tejun Heo
2025-09-24  6:14 ` Andrea Righi
2025-09-24  6:38   ` Tejun Heo
2025-09-24  8:26     ` Andrea Righi [this message]
2025-09-24 18:00       ` Tejun Heo
2025-10-22 21:37 ` Andrea Righi
2025-10-22 21:56 ` Tejun Heo

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=aNOrMNYJipF_nTGD@gpd4 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sched-ext@lists.linux.dev \
    --cc=tj@kernel.org \
    --cc=void@manifault.com \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox