On Tue, 17 May 2022 09:49:33 +0200
Jiri Olsa [off-list ref] wrote:

On Mon, May 16, 2022 at 07:54:37PM -0700, Yonghong Song wrote:

quoted

On 5/15/22 1:36 PM, Jiri Olsa wrote:

quoted

Making arch_cpu_idle and rcu_idle_exit noinstr. Both functions run
in rcu 'not watching' context and if there's tracer attached to
them, which uses rcu (e.g. kprobe multi interface) it will hit RCU
warning like:

   [    3.017540] WARNING: suspicious RCU usage
   ...
   [    3.018363]  kprobe_multi_link_handler+0x68/0x1c0
   [    3.018364]  ? kprobe_multi_link_handler+0x3e/0x1c0
   [    3.018366]  ? arch_cpu_idle_dead+0x10/0x10
   [    3.018367]  ? arch_cpu_idle_dead+0x10/0x10
   [    3.018371]  fprobe_handler.part.0+0xab/0x150
   [    3.018374]  0xffffffffa00080c8
   [    3.018393]  ? arch_cpu_idle+0x5/0x10
   [    3.018398]  arch_cpu_idle+0x5/0x10
   [    3.018399]  default_idle_call+0x59/0x90
   [    3.018401]  do_idle+0x1c3/0x1d0

The call path is following:

default_idle_call
   rcu_idle_enter
   arch_cpu_idle
   rcu_idle_exit

The arch_cpu_idle and rcu_idle_exit are the only ones from above
path that are traceble and cause this problem on my setup.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
  arch/x86/kernel/process.c | 2 +-
  kernel/rcu/tree.c         | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index b370767f5b19..1345cb0124a6 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c

@@ -720,7 +720,7 @@ void arch_cpu_idle_dead(void)
  /*
   * Called from the generic idle code.
   */
-void arch_cpu_idle(void)
+void noinstr arch_cpu_idle(void)

noinstr includes a lot of attributes:

#define noinstr                                                         \
        noinline notrace __attribute((__section__(".noinstr.text")))    \
        __no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage

should we use notrace here?

hm right, so notrace should be enough for our case (kprobe_multi)
which is based on ftrace/fprobe jump

noinstr (among other things) adds the function also the kprobes
blacklist, which will prevent standard kprobes to attach

ASAICS standard kprobes use rcu in probe path as well, like in
opt_pre_handler function

so I think we should go with noinstr

Yes, I agree that noinstr is preferrable for these functions.

Thank you!

jirka

quoted

quoted

  {
  	x86_idle();
  }

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index a4b8189455d5..20d529722f51 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c

@@ -896,7 +896,7 @@ static void noinstr rcu_eqs_exit(bool user)
   * If you add or remove a call to rcu_idle_exit(), be sure to test with
   * CONFIG_RCU_EQS_DEBUG=y.
   */
-void rcu_idle_exit(void)
+void noinstr rcu_idle_exit(void)
  {
  	unsigned long flags;

-- 
Masami Hiramatsu [off-list ref]