On 2025/8/29 10:23 Steven Rostedt [off-list ref] write:

quoted

On Fri, 29 Aug 2025 10:14:36 +0800
Menglong Dong [off-list ref] wrote:

quoted

rcu_read_lock() is not needed in fprobe_entry, but rcu_dereference_check()
is used in rhltable_lookup(), which causes suspicious RCU usage warning:

  WARNING: suspicious RCU usage
  6.17.0-rc1-00001-gdfe0d675df82 #1 Tainted: G S
  -----------------------------
  include/linux/rhashtable.h:602 suspicious rcu_dereference_check() usage!
  ......
  stack backtrace:
  CPU: 1 UID: 0 PID: 4652 Comm: ftracetest Tainted: G S
  Tainted: [S]=CPU_OUT_OF_SPEC, [I]=FIRMWARE_WORKAROUND
  Hardware name: Dell Inc. OptiPlex 7040/0Y7WYT, BIOS 1.1.1 10/07/2015
  Call Trace:
   <TASK>
   dump_stack_lvl+0x7c/0x90
   lockdep_rcu_suspicious+0x14f/0x1c0
   __rhashtable_lookup+0x1e0/0x260
   ? __pfx_kernel_clone+0x10/0x10
   fprobe_entry+0x9a/0x450
   ? __lock_acquire+0x6b0/0xca0
   ? find_held_lock+0x2b/0x80
   ? __pfx_fprobe_entry+0x10/0x10
   ? __pfx_kernel_clone+0x10/0x10
   ? lock_acquire+0x14c/0x2d0
   ? __might_fault+0x74/0xc0
   function_graph_enter_regs+0x2a0/0x550
   ? __do_sys_clone+0xb5/0x100
   ? __pfx_function_graph_enter_regs+0x10/0x10
   ? _copy_to_user+0x58/0x70
   ? __pfx_kernel_clone+0x10/0x10
   ? __x64_sys_rt_sigprocmask+0x114/0x180
   ? __pfx___x64_sys_rt_sigprocmask+0x10/0x10
   ? __pfx_kernel_clone+0x10/0x10
   ftrace_graph_func+0x87/0xb0

Fix this by using rcu_read_lock() for rhltable_lookup(). Alternatively, we
can use rcu_lock_acquire(&rcu_lock_map) here to obtain better performance.
However, it's not a common usage :/

So this is needed even though it's called under preempt_disable().

It is needed when the lock debug configs are enabled.

quoted

Paul, do we need to add an rcu_read_lock() because the code in rht
(rhashtable) requires RCU read lock?

I thought that rcu_read_lock() and preempt_disable() have been merged?

Maybe we can do some adjustment do rcu_read_lock_held_common()
like this:

diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index c912b594ba98..280fa4d2fc79 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c

@@ -114,6 +114,10 @@ static bool rcu_read_lock_held_common(bool *ret)
                *ret = false;
                return true;
        }
+       if (!preemptible()) {
+               *ret = true;
+               return true;
+       }
        return false;
 }
 

@@ -123,7 +127,7 @@ int rcu_read_lock_sched_held(void)
 
        if (rcu_read_lock_held_common(&ret))
                return ret;
-       return lock_is_held(&rcu_sched_lock_map) || !preemptible();
+       return lock_is_held(&rcu_sched_lock_map);
 }
 EXPORT_SYMBOL(rcu_read_lock_sched_held);
 #endif

I think it's a bad idea, as !preemptiable() has different semantic
with rcu_read_lock() :(