Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 9 Mar 2020 14:52:40 -0400 (EDT)
From:   Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To:     rostedt <rostedt@goodmis.org>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        paulmck <paulmck@kernel.org>,
        "Joel Fernandes, Google" <joel@joelfernandes.org>,
        Frederic Weisbecker <frederic@kernel.org>
Message-ID: <1851876075.21840.1583779960064.JavaMail.zimbra@efficios.com>
In-Reply-To: <20200309144427.0ce0eabc@gandalf.local.home>
References: <87mu8p797b.fsf@nanos.tec.linutronix.de> <1403546357.21810.1583779060302.JavaMail.zimbra@efficios.com> <20200309144427.0ce0eabc@gandalf.local.home>
Subject: Re: Instrumentation and RCU
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Thread-Topic: Instrumentation and RCU
Thread-Index: NW7KV4uXEwGhR5W3UrelonrIpWyYaQ==
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

----- On Mar 9, 2020, at 2:44 PM, rostedt rostedt@goodmis.org wrote:

> On Mon, 9 Mar 2020 14:37:40 -0400 (EDT)
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
>> So I think we could go back to plain RCU for rcuidle tracepoints if we do
>> the cheaper "rcu_is_watching()" check rather than invoking
>> rcu_irq_{enter,exit}_irqson() unconditionally.
> 
> You mean something like this?

I have a hard time applying this patch to any tree I can fetch, so
I will use caution and say: probably not. ;-)

And when I say "go back to plain RCU", I really mean removing use of SRCU
from the tracepoints until we have other purposes for it (e.g. taking
faults within specific tracepoint probes such as syscall enter/exit).

Thanks,

Mathieu

> 
> -- Steve
> 
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 5f4de82ffa0f..1904dbb3a921 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -164,7 +164,7 @@ static inline struct tracepoint
> *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> 		struct tracepoint_func *it_func_ptr;			\
> 		void *it_func;						\
> 		void *__data;						\
> -		int __maybe_unused __idx = 0;				\
> +		int __maybe_unused __idx = -1;				\
> 									\
> 		if (!(cond))						\
> 			return;						\
> @@ -179,8 +179,8 @@ static inline struct tracepoint
> *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> 		 * For rcuidle callers, use srcu since sched-rcu	\
> 		 * doesn't work from the idle path.			\
> 		 */							\
> -		if (rcuidle)						\
> -			__idx = srcu_read_lock_notrace(&tracepoint_srcu);\
> +		if (rcuidle && !rcu_is_watching())			\
> +			__idx = srcu_read_lock_notrace(&tracepoint_srcu); \
> 									\
> 		it_func_ptr = rcu_dereference_raw((tp)->funcs);		\
> 									\
> @@ -199,7 +199,7 @@ static inline struct tracepoint
> *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> 			} while ((++it_func_ptr)->func);		\
> 		}							\
> 									\
> -		if (rcuidle)						\
> +		if (rcuidle && __idx != -1)				\
> 			rcu_irq_exit_irqson();				\
> 									\
>  		preempt_enable_notrace();				\

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com