linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org, mingo@kernel.org,
	jiangshanlai@gmail.com, dipankar@in.ibm.com,
	akpm@linux-foundation.org, mathieu.desnoyers@efficios.com,
	josh@joshtriplett.org, tglx@linutronix.de, peterz@infradead.org,
	dhowells@redhat.com, edumazet@google.com, fweisbec@gmail.com,
	oleg@redhat.com
Subject: Re: [PATCH tip/core/rcu 02/15] rcu: Use timer as backstop for NOCB deferred wakeups
Date: Wed, 26 Jul 2017 17:18:01 -0400	[thread overview]
Message-ID: <20170726171801.5da044c3@vmware.local.home> (raw)
In-Reply-To: <20170726000540.GE3730@linux.vnet.ibm.com>

On Tue, 25 Jul 2017 17:05:40 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> On Tue, Jul 25, 2017 at 06:17:10PM -0400, Steven Rostedt wrote:
> > On Tue, 25 Jul 2017 12:18:14 -0700
> > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> >   
> > > On Tue, Jul 25, 2017 at 02:12:20PM -0400, Steven Rostedt wrote:  
> > > > On Mon, 24 Jul 2017 14:44:31 -0700
> > > > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > > >     
> > > > > The handling of RCU's no-CBs CPUs has a maintenance headache, namely
> > > > > that if call_rcu() is invoked with interrupts disabled, the rcuo kthread
> > > > > wakeup must be defered to a point where we can be sure that scheduler
> > > > > locks are not held.  Of course, there are a lot of code paths leading
> > > > > from an interrupts-disabled invocation of call_rcu(), and missing any
> > > > > one of these can result in excessive callback-invocation latency, and
> > > > > potentially even system hangs.    
> > > > 
> > > > What about using irq_work? That's what perf and ftrace use for such a
> > > > case.    
> > > 
> > > I hadn't looked at irq_work before, thank you for the pointer!
> > > 
> > > I nevertheless believe that timers work better in this particular case
> > > because they can be cancelled (which appears to be the common case), they  
> > 
> > Is the common case here that it doesn't trigger? That is, the
> > del_timer() will be called?  
> 
> If you have lots of call_rcu() invocations, many of them will be invoked
> with interrupts enabled, and a later one with interrupts enabled will
> take care of things for the earlier ones.  So there can be workloads
> where this is the case.

Note, only the first irq_work called will take action. The other
callers will see that a irq_work is pending and will not reivoke one.

> 
> > > normally are not at all time-critical, and because running in softirq
> > > is just fine -- no need to run out of the scheduling-clock interrupt.  
> > 
> > irq_work doesn't always use the scheduling clock. IIRC, it will simply
> > trigger a interrupt (if the arch supports it), and the work will be
> > done when interrupts are enabled (the interrupt that will do the work
> > will trigger)  
> 
> Ah, OK, so scheduling clock is just the backstop.  Still, softirq
> is a bit nicer to manage than hardirq.

Still requires a hard interrupt (timer) (thinking of NOHZ FULL where
this does matter).

> 
> > > Seem reasonable?  
> > 
> > Don't know. With irq_work, you just call it and forget about it. No
> > need to mod or del timers.  
> 
> But I could have a series of call_rcu() invocations with interrupts
> disabled, so I would need to interact somehow with the irq_work handler.
> Either that or dynamically allocate the needed data structure.
> 
> Or am I missing something here?

You treat it just like you are with the timer code. You have a irq_work
struct attached to your rdp descriptor. And call irq_work_run() when
interrupts are disabled. If it hasn't already been invoked it will
invoke one. Then the irq_work handler will look at the rdp attached to
the irq_work (container_of()), and then wake the associated thread.

It is much lighter weight than a timer setup.

-- Steve

  reply	other threads:[~2017-07-26 21:18 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-24 21:44 [PATCH tip/core/rcu 0/15] General fixes Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 01/15] sched,rcu: Make cond_resched() provide RCU quiescent state Paul E. McKenney
2017-08-15 16:19   ` [PATCH v5 " Paul E. McKenney
2017-08-17  8:22     ` Ingo Molnar
2017-08-17 12:40       ` Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 02/15] rcu: Use timer as backstop for NOCB deferred wakeups Paul E. McKenney
2017-07-25 18:12   ` Steven Rostedt
2017-07-25 19:18     ` Paul E. McKenney
2017-07-25 22:17       ` Steven Rostedt
2017-07-26  0:05         ` Paul E. McKenney
2017-07-26 21:18           ` Steven Rostedt [this message]
2017-07-26 21:47             ` Paul E. McKenney
2017-07-26 23:09               ` Steven Rostedt
2017-07-27 17:33                 ` Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 03/15] rcu: Drive TASKS_RCU directly off of PREEMPT Paul E. McKenney
2017-07-25 18:14   ` Steven Rostedt
2017-07-25 19:19     ` Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 04/15] rcu: Create reasonable API for do_exit() TASKS_RCU processing Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 05/15] rcu: Add TPS() to event-traced strings Paul E. McKenney
2017-07-28  1:32   ` Steven Rostedt
2017-07-24 21:44 ` [PATCH tip/core/rcu 06/15] rcu: Move rcu.h to new trivial-function style Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 07/15] rcu: Add event tracing to ->gp_tasks update at GP start Paul E. McKenney
2017-07-28  1:38   ` Steven Rostedt
2017-07-28  3:22     ` Paul E. McKenney
2017-07-28 12:18       ` Steven Rostedt
2017-07-28 17:13         ` Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 08/15] swait: add idle variants which don't contribute to load average Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 09/15] rcu: use idle versions of swait to make idle-hack clear Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 10/15] rcu: Add TPS() protection for _rcu_barrier_trace strings Paul E. McKenney
2017-07-28  1:40   ` Steven Rostedt
2017-07-24 21:44 ` [PATCH tip/core/rcu 11/15] rcu/tracing: Set disable_rcu_irq_enter on rcu_eqs_exit() Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 12/15] rcu: Add assertions verifying blocked-tasks list Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 13/15] rcu: Make rcu_idle_enter() rely on callers disabling irqs Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 14/15] rcu: Add warning to rcu_idle_enter() for irqs enabled Paul E. McKenney
2017-07-24 21:44 ` [PATCH tip/core/rcu 15/15] rcu: Remove exports from rcu_idle_exit() and rcu_idle_enter() Paul E. McKenney

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=20170726171801.5da044c3@vmware.local.home \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=dipankar@in.ibm.com \
    --cc=edumazet@google.com \
    --cc=fweisbec@gmail.com \
    --cc=jiangshanlai@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).