BPF List
 help / color / mirror / Atom feed
* [TEST FAILURE] bpf: s390: missed/kprobe_recursion
@ 2025-01-23 22:32 Martin KaFai Lau
  2025-01-23 22:44 ` Martin KaFai Lau
  0 siblings, 1 reply; 9+ messages in thread
From: Martin KaFai Lau @ 2025-01-23 22:32 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Ilya Leoshkevich, bpf

Hi Jiri,

The "missed/kprobe_recursion" fails consistently on s390. It seems to start 
failing after the recent bpf and bpf-next tree ffwd.

An example:
https://github.com/kernel-patches/bpf/actions/runs/12934431612/job/36076956920

Can you help to take a look?

afaict, it only happens on s390 so far, so cc IIya if there is any recent change 
that may ring the bell.

Thanks,
Martin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [TEST FAILURE] bpf: s390: missed/kprobe_recursion
@ 2025-01-23 22:32 Martin KaFai Lau
  2025-01-24 11:23 ` Jiri Olsa
  0 siblings, 1 reply; 9+ messages in thread
From: Martin KaFai Lau @ 2025-01-23 22:32 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Ilya Leoshkevich, bpf

Hi Jiri,

The "missed/kprobe_recursion" fails consistently on s390. It seems to start 
failing after the recent bpf and bpf-next tree ffwd.

An example:
https://github.com/kernel-patches/bpf/actions/runs/12934431612/job/36076956920

Can you help to take a look?

afaict, it only happens on s390 so far, so cc IIya if there is any recent change 
that may ring the bell.

Thanks,
Martin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [TEST FAILURE] bpf: s390: missed/kprobe_recursion
  2025-01-23 22:32 [TEST FAILURE] bpf: s390: missed/kprobe_recursion Martin KaFai Lau
@ 2025-01-23 22:44 ` Martin KaFai Lau
  0 siblings, 0 replies; 9+ messages in thread
From: Martin KaFai Lau @ 2025-01-23 22:44 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Ilya Leoshkevich, bpf

On 1/23/25 2:32 PM, Martin KaFai Lau wrote:
> Hi Jiri,
> 
> The "missed/kprobe_recursion" fails consistently on s390. It seems to start 
> failing after the recent bpf and bpf-next tree ffwd.
> 
> An example:
> https://github.com/kernel-patches/bpf/actions/runs/12934431612/job/36076956920
> 
> Can you help to take a look?
> 
> afaict, it only happens on s390 so far, so cc IIya if there is any recent change 
> that may ring the bell.

Ignore this thread. Somehow my mail client sent a dup... please continue the 
discussion on the other email.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [TEST FAILURE] bpf: s390: missed/kprobe_recursion
  2025-01-23 22:32 Martin KaFai Lau
@ 2025-01-24 11:23 ` Jiri Olsa
  2025-01-24 15:41   ` Jiri Olsa
  0 siblings, 1 reply; 9+ messages in thread
From: Jiri Olsa @ 2025-01-24 11:23 UTC (permalink / raw)
  To: Martin KaFai Lau; +Cc: Ilya Leoshkevich, bpf

On Thu, Jan 23, 2025 at 02:32:38PM -0800, Martin KaFai Lau wrote:
> Hi Jiri,
> 
> The "missed/kprobe_recursion" fails consistently on s390. It seems to start
> failing after the recent bpf and bpf-next tree ffwd.
> 
> An example:
> https://github.com/kernel-patches/bpf/actions/runs/12934431612/job/36076956920
> 
> Can you help to take a look?
> 
> afaict, it only happens on s390 so far, so cc IIya if there is any recent
> change that may ring the bell.

hi,
I need to check more but I wonder it's the:
  7495e179b478 s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC

which seems to add recursion check and bail out before we have
a chance to trigger it in bpf code

jirka

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [TEST FAILURE] bpf: s390: missed/kprobe_recursion
  2025-01-24 11:23 ` Jiri Olsa
@ 2025-01-24 15:41   ` Jiri Olsa
  2025-01-26 14:40     ` Masami Hiramatsu
  0 siblings, 1 reply; 9+ messages in thread
From: Jiri Olsa @ 2025-01-24 15:41 UTC (permalink / raw)
  To: Jiri Olsa, Sven Schnelle, Masami Hiramatsu
  Cc: Martin KaFai Lau, Ilya Leoshkevich, bpf

On Fri, Jan 24, 2025 at 12:23:35PM +0100, Jiri Olsa wrote:
> On Thu, Jan 23, 2025 at 02:32:38PM -0800, Martin KaFai Lau wrote:
> > Hi Jiri,
> > 
> > The "missed/kprobe_recursion" fails consistently on s390. It seems to start
> > failing after the recent bpf and bpf-next tree ffwd.
> > 
> > An example:
> > https://github.com/kernel-patches/bpf/actions/runs/12934431612/job/36076956920
> > 
> > Can you help to take a look?
> > 
> > afaict, it only happens on s390 so far, so cc IIya if there is any recent
> > change that may ring the bell.
> 
> hi,
> I need to check more but I wonder it's the:
>   7495e179b478 s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC
> 
> which seems to add recursion check and bail out before we have
> a chance to trigger it in bpf code

so the test attaches bpf program test1 to bpf_fentry_test1 via kprobe.multi

	SEC("kprobe.multi/bpf_fentry_test1")
	int test1(struct pt_regs *ctx)
	{
		bpf_kfunc_common_test();
		return 0;
	}

and several other programs are attached to bpf_kfunc_common_test function


I can't test this on s390, but looks like following is happening:

kprobe.multi uses fprobe, so the test kernel path goes:

    bpf_fentry_test1
      ftrace_graph_func
        function_graph_enter_regs
	   fprobe_entry
	     kprobe_multi_link_prog_run
	       test1 (bpf program)
	         bpf_kfunc_common_test
		   kprobe_ftrace_handler
		     kprobe_perf_func
		       trace_call_bpf
		         -> bpf_prog_active check fails, missed count is incremented


kprobe_ftrace_handler calls/takes ftrace_test_recursion_trylock (ftrace recursion lock)

but s390 now calls/takes ftrace_test_recursion_trylock already in ftrace_graph_func,
so s390 stops at kprobe_ftrace_handler and does not get to trace_call_bpf to increment
prog->missed counters

adding Sven, Masami, any idea?

if the ftrace_test_recursion_trylock is needed ftrace_graph_func on s390, then
I think we will need to fix our test to skip s390 arch

thanks,
jirka

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [TEST FAILURE] bpf: s390: missed/kprobe_recursion
  2025-01-24 15:41   ` Jiri Olsa
@ 2025-01-26 14:40     ` Masami Hiramatsu
  2025-01-26 22:06       ` Jiri Olsa
  0 siblings, 1 reply; 9+ messages in thread
From: Masami Hiramatsu @ 2025-01-26 14:40 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Sven Schnelle, Martin KaFai Lau, Ilya Leoshkevich, bpf

On Fri, 24 Jan 2025 16:41:38 +0100
Jiri Olsa <olsajiri@gmail.com> wrote:

> On Fri, Jan 24, 2025 at 12:23:35PM +0100, Jiri Olsa wrote:
> > On Thu, Jan 23, 2025 at 02:32:38PM -0800, Martin KaFai Lau wrote:
> > > Hi Jiri,
> > > 
> > > The "missed/kprobe_recursion" fails consistently on s390. It seems to start
> > > failing after the recent bpf and bpf-next tree ffwd.
> > > 
> > > An example:
> > > https://github.com/kernel-patches/bpf/actions/runs/12934431612/job/36076956920
> > > 
> > > Can you help to take a look?
> > > 
> > > afaict, it only happens on s390 so far, so cc IIya if there is any recent
> > > change that may ring the bell.
> > 
> > hi,
> > I need to check more but I wonder it's the:
> >   7495e179b478 s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC
> > 
> > which seems to add recursion check and bail out before we have
> > a chance to trigger it in bpf code
> 
> so the test attaches bpf program test1 to bpf_fentry_test1 via kprobe.multi
> 
> 	SEC("kprobe.multi/bpf_fentry_test1")
> 	int test1(struct pt_regs *ctx)
> 	{
> 		bpf_kfunc_common_test();
> 		return 0;
> 	}
> 
> and several other programs are attached to bpf_kfunc_common_test function
> 
> 
> I can't test this on s390, but looks like following is happening:
> 
> kprobe.multi uses fprobe, so the test kernel path goes:
> 
>     bpf_fentry_test1
>       ftrace_graph_func
>         function_graph_enter_regs
> 	   fprobe_entry
> 	     kprobe_multi_link_prog_run
> 	       test1 (bpf program)
> 	         bpf_kfunc_common_test
> 		   kprobe_ftrace_handler
> 		     kprobe_perf_func
> 		       trace_call_bpf
> 		         -> bpf_prog_active check fails, missed count is incremented
> 
> 
> kprobe_ftrace_handler calls/takes ftrace_test_recursion_trylock (ftrace recursion lock)
> 
> but s390 now calls/takes ftrace_test_recursion_trylock already in ftrace_graph_func,
> so s390 stops at kprobe_ftrace_handler and does not get to trace_call_bpf to increment
> prog->missed counters

Oops, good catch! I missed to remove it from s390. We've already moved it
in function_graph_enter_regs().


> 
> adding Sven, Masami, any idea?
> 
> if the ftrace_test_recursion_trylock is needed ftrace_graph_func on s390, then
> I think we will need to fix our test to skip s390 arch

Yes. Please try this patch;


From 12fcda79d0b1082449d5f7cfb8039b0237cf246d Mon Sep 17 00:00:00 2001
From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Date: Sun, 26 Jan 2025 23:38:59 +0900
Subject: [PATCH] s390: fgraph: Fix to remove ftrace_test_recursion_trylock()

Fix to remove ftrace_test_recursion_trylock() from ftrace_graph_func()
because commit d576aec24df9 ("fgraph: Get ftrace recursion lock in
function_graph_enter") has been moved it to function_graph_enter_regs()
already.

Reported-by: Jiri Olsa <olsajiri@gmail.com>
Fixes: d576aec24df9 ("fgraph: Get ftrace recursion lock in function_graph_enter")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 arch/s390/kernel/ftrace.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index c0b2c97efefb..63ba6306632e 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -266,18 +266,13 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 		       struct ftrace_ops *op, struct ftrace_regs *fregs)
 {
 	unsigned long *parent = &arch_ftrace_regs(fregs)->regs.gprs[14];
-	int bit;
 
 	if (unlikely(ftrace_graph_is_dead()))
 		return;
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		return;
-	bit = ftrace_test_recursion_trylock(ip, *parent);
-	if (bit < 0)
-		return;
 	if (!function_graph_enter_regs(*parent, ip, 0, parent, fregs))
 		*parent = (unsigned long)&return_to_handler;
-	ftrace_test_recursion_unlock(bit);
 }
 
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-- 
2.43.0

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [TEST FAILURE] bpf: s390: missed/kprobe_recursion
  2025-01-26 14:40     ` Masami Hiramatsu
@ 2025-01-26 22:06       ` Jiri Olsa
  2025-01-27 19:09         ` Andrii Nakryiko
  0 siblings, 1 reply; 9+ messages in thread
From: Jiri Olsa @ 2025-01-26 22:06 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Jiri Olsa, Sven Schnelle, Martin KaFai Lau, Ilya Leoshkevich, bpf

On Sun, Jan 26, 2025 at 11:40:05PM +0900, Masami Hiramatsu wrote:
> On Fri, 24 Jan 2025 16:41:38 +0100
> Jiri Olsa <olsajiri@gmail.com> wrote:
> 
> > On Fri, Jan 24, 2025 at 12:23:35PM +0100, Jiri Olsa wrote:
> > > On Thu, Jan 23, 2025 at 02:32:38PM -0800, Martin KaFai Lau wrote:
> > > > Hi Jiri,
> > > > 
> > > > The "missed/kprobe_recursion" fails consistently on s390. It seems to start
> > > > failing after the recent bpf and bpf-next tree ffwd.
> > > > 
> > > > An example:
> > > > https://github.com/kernel-patches/bpf/actions/runs/12934431612/job/36076956920
> > > > 
> > > > Can you help to take a look?
> > > > 
> > > > afaict, it only happens on s390 so far, so cc IIya if there is any recent
> > > > change that may ring the bell.
> > > 
> > > hi,
> > > I need to check more but I wonder it's the:
> > >   7495e179b478 s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC
> > > 
> > > which seems to add recursion check and bail out before we have
> > > a chance to trigger it in bpf code
> > 
> > so the test attaches bpf program test1 to bpf_fentry_test1 via kprobe.multi
> > 
> > 	SEC("kprobe.multi/bpf_fentry_test1")
> > 	int test1(struct pt_regs *ctx)
> > 	{
> > 		bpf_kfunc_common_test();
> > 		return 0;
> > 	}
> > 
> > and several other programs are attached to bpf_kfunc_common_test function
> > 
> > 
> > I can't test this on s390, but looks like following is happening:
> > 
> > kprobe.multi uses fprobe, so the test kernel path goes:
> > 
> >     bpf_fentry_test1
> >       ftrace_graph_func
> >         function_graph_enter_regs
> > 	   fprobe_entry
> > 	     kprobe_multi_link_prog_run
> > 	       test1 (bpf program)
> > 	         bpf_kfunc_common_test
> > 		   kprobe_ftrace_handler
> > 		     kprobe_perf_func
> > 		       trace_call_bpf
> > 		         -> bpf_prog_active check fails, missed count is incremented
> > 
> > 
> > kprobe_ftrace_handler calls/takes ftrace_test_recursion_trylock (ftrace recursion lock)
> > 
> > but s390 now calls/takes ftrace_test_recursion_trylock already in ftrace_graph_func,
> > so s390 stops at kprobe_ftrace_handler and does not get to trace_call_bpf to increment
> > prog->missed counters
> 
> Oops, good catch! I missed to remove it from s390. We've already moved it
> in function_graph_enter_regs().
> 
> 
> > 
> > adding Sven, Masami, any idea?
> > 
> > if the ftrace_test_recursion_trylock is needed ftrace_graph_func on s390, then
> > I think we will need to fix our test to skip s390 arch
> 
> Yes. Please try this patch;
> 
> 
> From 12fcda79d0b1082449d5f7cfb8039b0237cf246d Mon Sep 17 00:00:00 2001
> From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
> Date: Sun, 26 Jan 2025 23:38:59 +0900
> Subject: [PATCH] s390: fgraph: Fix to remove ftrace_test_recursion_trylock()
> 
> Fix to remove ftrace_test_recursion_trylock() from ftrace_graph_func()
> because commit d576aec24df9 ("fgraph: Get ftrace recursion lock in
> function_graph_enter") has been moved it to function_graph_enter_regs()
> already.
> 
> Reported-by: Jiri Olsa <olsajiri@gmail.com>
> Fixes: d576aec24df9 ("fgraph: Get ftrace recursion lock in function_graph_enter")
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

great, ci is passing with this fix

Tested-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka


> ---
>  arch/s390/kernel/ftrace.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
> index c0b2c97efefb..63ba6306632e 100644
> --- a/arch/s390/kernel/ftrace.c
> +++ b/arch/s390/kernel/ftrace.c
> @@ -266,18 +266,13 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>  		       struct ftrace_ops *op, struct ftrace_regs *fregs)
>  {
>  	unsigned long *parent = &arch_ftrace_regs(fregs)->regs.gprs[14];
> -	int bit;
>  
>  	if (unlikely(ftrace_graph_is_dead()))
>  		return;
>  	if (unlikely(atomic_read(&current->tracing_graph_pause)))
>  		return;
> -	bit = ftrace_test_recursion_trylock(ip, *parent);
> -	if (bit < 0)
> -		return;
>  	if (!function_graph_enter_regs(*parent, ip, 0, parent, fregs))
>  		*parent = (unsigned long)&return_to_handler;
> -	ftrace_test_recursion_unlock(bit);
>  }
>  
>  #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
> -- 
> 2.43.0
> 
> Thank you,
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [TEST FAILURE] bpf: s390: missed/kprobe_recursion
  2025-01-26 22:06       ` Jiri Olsa
@ 2025-01-27 19:09         ` Andrii Nakryiko
  2025-01-28  4:57           ` Masami Hiramatsu
  0 siblings, 1 reply; 9+ messages in thread
From: Andrii Nakryiko @ 2025-01-27 19:09 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Masami Hiramatsu, Sven Schnelle, Martin KaFai Lau,
	Ilya Leoshkevich, bpf

On Sun, Jan 26, 2025 at 2:06 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Sun, Jan 26, 2025 at 11:40:05PM +0900, Masami Hiramatsu wrote:
> > On Fri, 24 Jan 2025 16:41:38 +0100
> > Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > > On Fri, Jan 24, 2025 at 12:23:35PM +0100, Jiri Olsa wrote:
> > > > On Thu, Jan 23, 2025 at 02:32:38PM -0800, Martin KaFai Lau wrote:
> > > > > Hi Jiri,
> > > > >
> > > > > The "missed/kprobe_recursion" fails consistently on s390. It seems to start
> > > > > failing after the recent bpf and bpf-next tree ffwd.
> > > > >
> > > > > An example:
> > > > > https://github.com/kernel-patches/bpf/actions/runs/12934431612/job/36076956920
> > > > >
> > > > > Can you help to take a look?
> > > > >
> > > > > afaict, it only happens on s390 so far, so cc IIya if there is any recent
> > > > > change that may ring the bell.
> > > >
> > > > hi,
> > > > I need to check more but I wonder it's the:
> > > >   7495e179b478 s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC
> > > >
> > > > which seems to add recursion check and bail out before we have
> > > > a chance to trigger it in bpf code
> > >
> > > so the test attaches bpf program test1 to bpf_fentry_test1 via kprobe.multi
> > >
> > >     SEC("kprobe.multi/bpf_fentry_test1")
> > >     int test1(struct pt_regs *ctx)
> > >     {
> > >             bpf_kfunc_common_test();
> > >             return 0;
> > >     }
> > >
> > > and several other programs are attached to bpf_kfunc_common_test function
> > >
> > >
> > > I can't test this on s390, but looks like following is happening:
> > >
> > > kprobe.multi uses fprobe, so the test kernel path goes:
> > >
> > >     bpf_fentry_test1
> > >       ftrace_graph_func
> > >         function_graph_enter_regs
> > >        fprobe_entry
> > >          kprobe_multi_link_prog_run
> > >            test1 (bpf program)
> > >              bpf_kfunc_common_test
> > >                kprobe_ftrace_handler
> > >                  kprobe_perf_func
> > >                    trace_call_bpf
> > >                      -> bpf_prog_active check fails, missed count is incremented
> > >
> > >
> > > kprobe_ftrace_handler calls/takes ftrace_test_recursion_trylock (ftrace recursion lock)
> > >
> > > but s390 now calls/takes ftrace_test_recursion_trylock already in ftrace_graph_func,
> > > so s390 stops at kprobe_ftrace_handler and does not get to trace_call_bpf to increment
> > > prog->missed counters
> >
> > Oops, good catch! I missed to remove it from s390. We've already moved it
> > in function_graph_enter_regs().
> >
> >
> > >
> > > adding Sven, Masami, any idea?
> > >
> > > if the ftrace_test_recursion_trylock is needed ftrace_graph_func on s390, then
> > > I think we will need to fix our test to skip s390 arch
> >
> > Yes. Please try this patch;
> >
> >
> > From 12fcda79d0b1082449d5f7cfb8039b0237cf246d Mon Sep 17 00:00:00 2001
> > From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
> > Date: Sun, 26 Jan 2025 23:38:59 +0900
> > Subject: [PATCH] s390: fgraph: Fix to remove ftrace_test_recursion_trylock()
> >
> > Fix to remove ftrace_test_recursion_trylock() from ftrace_graph_func()
> > because commit d576aec24df9 ("fgraph: Get ftrace recursion lock in
> > function_graph_enter") has been moved it to function_graph_enter_regs()
> > already.
> >
> > Reported-by: Jiri Olsa <olsajiri@gmail.com>
> > Fixes: d576aec24df9 ("fgraph: Get ftrace recursion lock in function_graph_enter")
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> great, ci is passing with this fix
>
> Tested-by: Jiri Olsa <jolsa@kernel.org>

Masami,

Are you going to land this fix in your tree? We can create a temporary
patch for BPF CI once you have the commit in the tree.

>
> thanks,
> jirka
>
>
> > ---
> >  arch/s390/kernel/ftrace.c | 5 -----
> >  1 file changed, 5 deletions(-)
> >
> > diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
> > index c0b2c97efefb..63ba6306632e 100644
> > --- a/arch/s390/kernel/ftrace.c
> > +++ b/arch/s390/kernel/ftrace.c
> > @@ -266,18 +266,13 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> >                      struct ftrace_ops *op, struct ftrace_regs *fregs)
> >  {
> >       unsigned long *parent = &arch_ftrace_regs(fregs)->regs.gprs[14];
> > -     int bit;
> >
> >       if (unlikely(ftrace_graph_is_dead()))
> >               return;
> >       if (unlikely(atomic_read(&current->tracing_graph_pause)))
> >               return;
> > -     bit = ftrace_test_recursion_trylock(ip, *parent);
> > -     if (bit < 0)
> > -             return;
> >       if (!function_graph_enter_regs(*parent, ip, 0, parent, fregs))
> >               *parent = (unsigned long)&return_to_handler;
> > -     ftrace_test_recursion_unlock(bit);
> >  }
> >
> >  #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
> > --
> > 2.43.0
> >
> > Thank you,
> >
> > --
> > Masami Hiramatsu (Google) <mhiramat@kernel.org>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [TEST FAILURE] bpf: s390: missed/kprobe_recursion
  2025-01-27 19:09         ` Andrii Nakryiko
@ 2025-01-28  4:57           ` Masami Hiramatsu
  0 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2025-01-28  4:57 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Jiri Olsa, Masami Hiramatsu, Sven Schnelle, Martin KaFai Lau,
	Ilya Leoshkevich, bpf

On Mon, 27 Jan 2025 11:09:27 -0800
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> On Sun, Jan 26, 2025 at 2:06 PM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Sun, Jan 26, 2025 at 11:40:05PM +0900, Masami Hiramatsu wrote:
> > > On Fri, 24 Jan 2025 16:41:38 +0100
> > > Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > > On Fri, Jan 24, 2025 at 12:23:35PM +0100, Jiri Olsa wrote:
> > > > > On Thu, Jan 23, 2025 at 02:32:38PM -0800, Martin KaFai Lau wrote:
> > > > > > Hi Jiri,
> > > > > >
> > > > > > The "missed/kprobe_recursion" fails consistently on s390. It seems to start
> > > > > > failing after the recent bpf and bpf-next tree ffwd.
> > > > > >
> > > > > > An example:
> > > > > > https://github.com/kernel-patches/bpf/actions/runs/12934431612/job/36076956920
> > > > > >
> > > > > > Can you help to take a look?
> > > > > >
> > > > > > afaict, it only happens on s390 so far, so cc IIya if there is any recent
> > > > > > change that may ring the bell.
> > > > >
> > > > > hi,
> > > > > I need to check more but I wonder it's the:
> > > > >   7495e179b478 s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC
> > > > >
> > > > > which seems to add recursion check and bail out before we have
> > > > > a chance to trigger it in bpf code
> > > >
> > > > so the test attaches bpf program test1 to bpf_fentry_test1 via kprobe.multi
> > > >
> > > >     SEC("kprobe.multi/bpf_fentry_test1")
> > > >     int test1(struct pt_regs *ctx)
> > > >     {
> > > >             bpf_kfunc_common_test();
> > > >             return 0;
> > > >     }
> > > >
> > > > and several other programs are attached to bpf_kfunc_common_test function
> > > >
> > > >
> > > > I can't test this on s390, but looks like following is happening:
> > > >
> > > > kprobe.multi uses fprobe, so the test kernel path goes:
> > > >
> > > >     bpf_fentry_test1
> > > >       ftrace_graph_func
> > > >         function_graph_enter_regs
> > > >        fprobe_entry
> > > >          kprobe_multi_link_prog_run
> > > >            test1 (bpf program)
> > > >              bpf_kfunc_common_test
> > > >                kprobe_ftrace_handler
> > > >                  kprobe_perf_func
> > > >                    trace_call_bpf
> > > >                      -> bpf_prog_active check fails, missed count is incremented
> > > >
> > > >
> > > > kprobe_ftrace_handler calls/takes ftrace_test_recursion_trylock (ftrace recursion lock)
> > > >
> > > > but s390 now calls/takes ftrace_test_recursion_trylock already in ftrace_graph_func,
> > > > so s390 stops at kprobe_ftrace_handler and does not get to trace_call_bpf to increment
> > > > prog->missed counters
> > >
> > > Oops, good catch! I missed to remove it from s390. We've already moved it
> > > in function_graph_enter_regs().
> > >
> > >
> > > >
> > > > adding Sven, Masami, any idea?
> > > >
> > > > if the ftrace_test_recursion_trylock is needed ftrace_graph_func on s390, then
> > > > I think we will need to fix our test to skip s390 arch
> > >
> > > Yes. Please try this patch;
> > >
> > >
> > > From 12fcda79d0b1082449d5f7cfb8039b0237cf246d Mon Sep 17 00:00:00 2001
> > > From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
> > > Date: Sun, 26 Jan 2025 23:38:59 +0900
> > > Subject: [PATCH] s390: fgraph: Fix to remove ftrace_test_recursion_trylock()
> > >
> > > Fix to remove ftrace_test_recursion_trylock() from ftrace_graph_func()
> > > because commit d576aec24df9 ("fgraph: Get ftrace recursion lock in
> > > function_graph_enter") has been moved it to function_graph_enter_regs()
> > > already.
> > >
> > > Reported-by: Jiri Olsa <olsajiri@gmail.com>
> > > Fixes: d576aec24df9 ("fgraph: Get ftrace recursion lock in function_graph_enter")
> > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >
> > great, ci is passing with this fix
> >
> > Tested-by: Jiri Olsa <jolsa@kernel.org>

Thanks for testing!

> 
> Masami,
> 
> Are you going to land this fix in your tree? We can create a temporary
> patch for BPF CI once you have the commit in the tree.

I think this should be a fix from linux-trace tree. I also found
another issue on s390. (s390 does not implemented )
Let me resend it because I missed to Cc to linux-trace ML.

Thank you,
> 
> >
> > thanks,
> > jirka
> >
> >
> > > ---
> > >  arch/s390/kernel/ftrace.c | 5 -----
> > >  1 file changed, 5 deletions(-)
> > >
> > > diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
> > > index c0b2c97efefb..63ba6306632e 100644
> > > --- a/arch/s390/kernel/ftrace.c
> > > +++ b/arch/s390/kernel/ftrace.c
> > > @@ -266,18 +266,13 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > >                      struct ftrace_ops *op, struct ftrace_regs *fregs)
> > >  {
> > >       unsigned long *parent = &arch_ftrace_regs(fregs)->regs.gprs[14];
> > > -     int bit;
> > >
> > >       if (unlikely(ftrace_graph_is_dead()))
> > >               return;
> > >       if (unlikely(atomic_read(&current->tracing_graph_pause)))
> > >               return;
> > > -     bit = ftrace_test_recursion_trylock(ip, *parent);
> > > -     if (bit < 0)
> > > -             return;
> > >       if (!function_graph_enter_regs(*parent, ip, 0, parent, fregs))
> > >               *parent = (unsigned long)&return_to_handler;
> > > -     ftrace_test_recursion_unlock(bit);
> > >  }
> > >
> > >  #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
> > > --
> > > 2.43.0
> > >
> > > Thank you,
> > >
> > > --
> > > Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-01-28  4:57 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-23 22:32 [TEST FAILURE] bpf: s390: missed/kprobe_recursion Martin KaFai Lau
2025-01-23 22:44 ` Martin KaFai Lau
  -- strict thread matches above, loose matches on Subject: below --
2025-01-23 22:32 Martin KaFai Lau
2025-01-24 11:23 ` Jiri Olsa
2025-01-24 15:41   ` Jiri Olsa
2025-01-26 14:40     ` Masami Hiramatsu
2025-01-26 22:06       ` Jiri Olsa
2025-01-27 19:09         ` Andrii Nakryiko
2025-01-28  4:57           ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox