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:   Fri, 7 Feb 2020 16:43:57 -0500
From:   Joel Fernandes <joel@joelfernandes.org>
To:     "Paul E. McKenney" <paulmck@kernel.org>
Cc:     linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
        Ingo Molnar <mingo@redhat.com>,
        Richard Fontana <rfontana@redhat.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Josh Triplett <josh@joshtriplett.org>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Lai Jiangshan <jiangshanlai@gmail.com>
Subject: Re: [RFC 0/3] Revert SRCU from tracepoint infrastructure
Message-ID: <20200207214357.GA75841@google.com>
References: <20200207205656.61938-1-joel@joelfernandes.org>
 <20200207212450.GP2935@paulmck-ThinkPad-P72>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200207212450.GP2935@paulmck-ThinkPad-P72>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Feb 07, 2020 at 01:24:50PM -0800, Paul E. McKenney wrote:
> On Fri, Feb 07, 2020 at 03:56:53PM -0500, Joel Fernandes (Google) wrote:
> > Hi,
> > These patches remove SRCU usage from tracepoints. The reason for proposing the
> > reverts is because the whole point of SRCU was to avoid having to call
> > rcu_irq_enter_irqson(). However this was added back in 865e63b04e9b2 ("tracing:
> > Add back in rcu_irq_enter/exit_irqson() for rcuidle tracepoints") because perf
> > was breaking..
> > 
> > Further it occurs to me that, by using SRCU for tracepoints, we forgot that RCU
> > is not really watching the tracepoint callbacks. This means that anyone doing
> > preempt_disable() in their tracepoint callback, and expecting RCU to listen to
> > them is in for a big surprise. When RCU is not watching, it does not care about
> > preempt-disable sections on CPUs as you can see in the forced-quiescent state loop.
> > 
> > Since SRCU is not providing any benefit because of 865e63b04e9b2 anyway, let us
> > revert SRCU tracepoint code to maintain the sanity of potential
> > tracepoint callback registerers.
> 
> For whatever it is worth, SRCU is the exception to the "RCU needs to
> be watching" rule.  You can have SRCU readers on idle CPUs, offline
> CPUs, CPUs executing in userspace, whatever.

Yes sure. My concern was that callbacks are still using regular RCU somewhere
and RCU isn't watching. I believe BPF is using RCU that way (not sure). But
could be other out-of-tree kernel modules etc.

thanks,

 - Joel

> 
> 							Thanx, Paul
> 
> > Joel Fernandes (Google) (3):
> > Revert "tracepoint: Use __idx instead of idx in DO_TRACE macro to make
> > it unique"
> > Revert "tracing: Add back in rcu_irq_enter/exit_irqson() for rcuidle
> > tracepoints"
> > Revert "tracepoint: Make rcuidle tracepoint callers use SRCU"
> > 
> > include/linux/tracepoint.h | 40 ++++++--------------------------------
> > kernel/tracepoint.c        | 10 +---------
> > 2 files changed, 7 insertions(+), 43 deletions(-)
> > 
> > --
> > 2.25.0.341.g760bfbb309-goog
> >