Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-06 Thread Andrii Nakryiko
On Fri, Apr 5, 2024 at 8:41 PM Masami Hiramatsu wrote: > > On Tue, 2 Apr 2024 22:21:00 -0700 > Andrii Nakryiko wrote: > > > On Tue, Apr 2, 2024 at 9:00 PM Andrii Nakryiko > > wrote: > > > > > > On Tue, Apr 2, 2024 at 5:52 PM Steven Rostedt wrote: > > > > > > > > On Wed, 3 Apr 2024 09:40:48

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-05 Thread Google
On Tue, 2 Apr 2024 22:21:00 -0700 Andrii Nakryiko wrote: > On Tue, Apr 2, 2024 at 9:00 PM Andrii Nakryiko > wrote: > > > > On Tue, Apr 2, 2024 at 5:52 PM Steven Rostedt wrote: > > > > > > On Wed, 3 Apr 2024 09:40:48 +0900 > > > Masami Hiramatsu (Google) wrote: > > > > > > > OK, for me, this

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-03 Thread Steven Rostedt
On Tue, 2 Apr 2024 22:21:00 -0700 Andrii Nakryiko wrote: > > I just checked our fleet-wide production data for the last 24 hours. > > Within the kprobe/kretprobe code path (ftrace_trampoline and > > everything called from it), rcu_is_watching (both calls, see below) > > cause 0.484% CPU cycles

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-03 Thread Steven Rostedt
On Tue, 2 Apr 2024 21:00:19 -0700 Andrii Nakryiko wrote: > I just noticed another rcu_is_watching() call, in rethook_try_get(), > which seems to be a similar and complementary validation check to the > one we are putting under CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING option > in this patch. It

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-02 Thread Andrii Nakryiko
On Tue, Apr 2, 2024 at 9:00 PM Andrii Nakryiko wrote: > > On Tue, Apr 2, 2024 at 5:52 PM Steven Rostedt wrote: > > > > On Wed, 3 Apr 2024 09:40:48 +0900 > > Masami Hiramatsu (Google) wrote: > > > > > OK, for me, this last sentence is preferred for the help message. That > > > explains > > >

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-02 Thread Andrii Nakryiko
On Tue, Apr 2, 2024 at 5:52 PM Steven Rostedt wrote: > > On Wed, 3 Apr 2024 09:40:48 +0900 > Masami Hiramatsu (Google) wrote: > > > OK, for me, this last sentence is preferred for the help message. That > > explains > > what this is for. > > > > All callbacks that attach to the function

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-02 Thread Steven Rostedt
On Wed, 3 Apr 2024 09:40:48 +0900 Masami Hiramatsu (Google) wrote: > OK, for me, this last sentence is preferred for the help message. That > explains > what this is for. > > All callbacks that attach to the function tracing have some sort > of protection against recursion.

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-02 Thread Google
On Mon, 1 Apr 2024 22:47:33 -0400 Steven Rostedt wrote: > On Mon, 1 Apr 2024 19:29:46 -0700 > Andrii Nakryiko wrote: > > > On Mon, Apr 1, 2024 at 5:38 PM Masami Hiramatsu wrote: > > > > > > On Mon, 1 Apr 2024 12:09:18 -0400 > > > Steven Rostedt wrote: > > > > > > > On Mon, 1 Apr 2024

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-01 Thread Steven Rostedt
On Mon, 1 Apr 2024 19:29:46 -0700 Andrii Nakryiko wrote: > On Mon, Apr 1, 2024 at 5:38 PM Masami Hiramatsu wrote: > > > > On Mon, 1 Apr 2024 12:09:18 -0400 > > Steven Rostedt wrote: > > > > > On Mon, 1 Apr 2024 20:25:52 +0900 > > > Masami Hiramatsu (Google) wrote: > > > > > > > > Masami,

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-01 Thread Andrii Nakryiko
On Mon, Apr 1, 2024 at 5:38 PM Masami Hiramatsu wrote: > > On Mon, 1 Apr 2024 12:09:18 -0400 > Steven Rostedt wrote: > > > On Mon, 1 Apr 2024 20:25:52 +0900 > > Masami Hiramatsu (Google) wrote: > > > > > > Masami, > > > > > > > > Are you OK with just keeping it set to N. > > > > > > OK, if it

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-01 Thread Google
On Mon, 1 Apr 2024 12:09:18 -0400 Steven Rostedt wrote: > On Mon, 1 Apr 2024 20:25:52 +0900 > Masami Hiramatsu (Google) wrote: > > > > Masami, > > > > > > Are you OK with just keeping it set to N. > > > > OK, if it is only for the debugging, I'm OK to set N this. > > > > > > > > We could

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-01 Thread Steven Rostedt
On Mon, 1 Apr 2024 20:25:52 +0900 Masami Hiramatsu (Google) wrote: > > Masami, > > > > Are you OK with just keeping it set to N. > > OK, if it is only for the debugging, I'm OK to set N this. > > > > > We could have other options like PROVE_LOCKING enable it. > > Agreed (but it should

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-01 Thread Google
On Tue, 26 Mar 2024 15:01:21 -0400 Steven Rostedt wrote: > On Tue, 26 Mar 2024 09:16:33 -0700 > Andrii Nakryiko wrote: > > > > It's no different than lockdep. Test boxes should have it enabled, but > > > there's no reason to have this enabled in a production system. > > > > > > > I tend to

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-29 Thread Andrii Nakryiko
On Tue, Mar 26, 2024 at 11:58 AM Steven Rostedt wrote: > > On Tue, 26 Mar 2024 09:16:33 -0700 > Andrii Nakryiko wrote: > > > > It's no different than lockdep. Test boxes should have it enabled, but > > > there's no reason to have this enabled in a production system. > > > > > > > I tend to agree

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-26 Thread Steven Rostedt
On Tue, 26 Mar 2024 09:16:33 -0700 Andrii Nakryiko wrote: > > It's no different than lockdep. Test boxes should have it enabled, but > > there's no reason to have this enabled in a production system. > > > > I tend to agree with Steven here (which is why I sent this patch as it > is), but I'm

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-26 Thread Andrii Nakryiko
On Mon, Mar 25, 2024 at 3:11 PM Steven Rostedt wrote: > > On Mon, 25 Mar 2024 11:38:48 +0900 > Masami Hiramatsu (Google) wrote: > > > On Fri, 22 Mar 2024 09:03:23 -0700 > > Andrii Nakryiko wrote: > > > > > Introduce CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING config option to > > > control whether

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-25 Thread Steven Rostedt
On Mon, 25 Mar 2024 11:38:48 +0900 Masami Hiramatsu (Google) wrote: > On Fri, 22 Mar 2024 09:03:23 -0700 > Andrii Nakryiko wrote: > > > Introduce CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING config option to > > control whether ftrace low-level code performs additional > > rcu_is_watching()-based

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-25 Thread Andrii Nakryiko
On Sun, Mar 24, 2024 at 7:38 PM Masami Hiramatsu wrote: > > On Fri, 22 Mar 2024 09:03:23 -0700 > Andrii Nakryiko wrote: > > > Introduce CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING config option to > > control whether ftrace low-level code performs additional > > rcu_is_watching()-based validation

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-25 Thread Google
On Fri, 22 Mar 2024 09:03:23 -0700 Andrii Nakryiko wrote: > Introduce CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING config option to > control whether ftrace low-level code performs additional > rcu_is_watching()-based validation logic in an attempt to catch noinstr > violations. > > This check is

[PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-22 Thread Andrii Nakryiko
Introduce CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING config option to control whether ftrace low-level code performs additional rcu_is_watching()-based validation logic in an attempt to catch noinstr violations. This check is expected to never be true in practice and would be best controlled with