All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 0/5 v2] tracing: Add usecase of synchronize_rcu_tasks() and stack_tracer_disable()
Date: Fri, 7 Apr 2017 08:11:46 -0700	[thread overview]
Message-ID: <20170407151146.GW1600@linux.vnet.ibm.com> (raw)
In-Reply-To: <20170407105826.562b2e24@gandalf.local.home>

On Fri, Apr 07, 2017 at 10:58:26AM -0400, Steven Rostedt wrote:
> On Fri, 7 Apr 2017 07:43:35 -0700
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> 
> > On Fri, Apr 07, 2017 at 10:01:06AM -0400, Steven Rostedt wrote:
> > > Paul,
> > > 
> > > Here's my latest. You OK with it?  
> > 
> > Given your update to 3/5, I suspect that we could live with it.  I am
> > expecting some complaints about increases in idle-entry latency, but might
> > be best to wait for complaints rather than complexifying too proactively.
> 
> We only added a this_cpu_inc() and this_cpu_dec() which are very fast
> operations. I highly doubt it will be measurable. Although, I'm talking
> about x86, IIRC, the this_cpu_inc/dec were be poorly written for other
> archs in the past. I'm not sure if that was fixed though.

That is an issue for CPUs that don't have a to-memory increment
instruction.  How about __this_cpu_inc() and __this_cpu_dec(), given
that preemption is disabled?

> > That said, there isn't supposed to be any tracing during the now very
> > small interval where RCU's idle-entry is incomplete.  Mightn't it be
> > better to (under CONFIG_PROVE_RCU or some such) give splats if tracing
> > showed up in that interval?
> 
> Again, tracing is not the issue. I do function tracing in that location
> without any problems. The issue here was the stack tracer.
> 
> Maybe we can create a new variable that is more cache local to the RCU
> code.
> 
> What about calling it "rcu_disabled"? Then tracing that depends on RCU
> can simply check that.
> 
> s/stack_trace_disable/disable_rcu/
> s/stack_trace_enable/enable_rcu/
> 
> export a per cpu variable rcu_disabled
> 
> Then I can have the stack tracer check that variable as well. And we
> could even put in a WARN_ON(this_cpu_read(rcu_disabled) in the
> TRACE_EVENT() macros.
> 
> Thoughts?

At this point, if you can use the "__" versions, the change should be
small.  With that change, if no one else complains, I am OK.

							Thanx, Paul

  reply	other threads:[~2017-04-07 15:12 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-07 14:01 [PATCH 0/5 v2] tracing: Add usecase of synchronize_rcu_tasks() and stack_tracer_disable() Steven Rostedt
2017-04-07 14:01 ` [PATCH 1/5 v2] ftrace: Add use of synchronize_rcu_tasks() with dynamic trampolines Steven Rostedt
2017-04-07 14:01 ` [PATCH 2/5 v2] tracing: Replace the per_cpu() with this_cpu() in trace_stack.c Steven Rostedt
2017-04-07 14:36   ` Paul E. McKenney
2017-04-07 14:48     ` Steven Rostedt
2017-04-07 15:08       ` Paul E. McKenney
2017-04-07 14:01 ` [PATCH 3/5 v2] tracing: Add stack_tracer_disable/enable() functions Steven Rostedt
2017-04-07 14:22   ` Steven Rostedt
2017-04-07 14:25   ` [PATCH 3/5 v2.1] " Steven Rostedt
2017-04-07 14:01 ` [PATCH 4/5 v2] tracing: Rename trace_active to disable_stack_tracer and inline its modification Steven Rostedt
2017-04-07 14:01 ` [PATCH 5/5 v2] rcu: Fix dyntick-idle tracing Steven Rostedt
2017-04-07 14:40   ` Paul E. McKenney
2017-04-07 14:53     ` Steven Rostedt
2017-04-07 15:09       ` Paul E. McKenney
2017-04-07 15:29         ` Steven Rostedt
2017-04-07 14:43 ` [PATCH 0/5 v2] tracing: Add usecase of synchronize_rcu_tasks() and stack_tracer_disable() Paul E. McKenney
2017-04-07 14:58   ` Steven Rostedt
2017-04-07 15:11     ` Paul E. McKenney [this message]
2017-04-07 15:28       ` Steven Rostedt
2017-04-07 16:35 ` [PATCH 6/5]rcu/tracing: Add rcu_disabled to denote when rcu_irq_enter() will not work Steven Rostedt
2017-04-07 16:42   ` Paul E. McKenney
2017-04-07 16:44     ` Steven Rostedt
2017-04-07 16:53       ` Paul E. McKenney
2017-04-07 17:03     ` [PATCH 6/5 v2] rcu/tracing: " Steven Rostedt
2017-04-07 17:15       ` Paul E. McKenney
2017-04-07 17:06 ` [PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Steven Rostedt
2017-04-07 17:15   ` Paul E. McKenney
2017-04-07 17:19   ` Mathieu Desnoyers
2017-04-07 17:26     ` Steven Rostedt
2017-04-07 17:32       ` Steven Rostedt
2017-04-07 17:49       ` Mathieu Desnoyers
2017-04-07 17:55         ` Steven Rostedt
2017-04-07 18:10         ` [PATCH 7/5 v3] " Steven Rostedt
2017-04-07 18:17           ` Mathieu Desnoyers
2017-04-07 19:41             ` [PATCH 7/5 v4] " Steven Rostedt
2017-04-07 19:43               ` Steven Rostedt
2017-04-10 17:11                 ` Mathieu Desnoyers
2017-04-07 17:28     ` [PATCH 7/5] " Steven Rostedt
2017-04-07 17:48     ` [PATCH 7/5 v2] " Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170407151146.GW1600@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.