public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org, mingo@elte.hu,
	laijs@cn.fujitsu.com, dipankar@in.ibm.com,
	akpm@linux-foundation.org, mathieu.desnoyers@polymtl.ca,
	josh@joshtriplett.org, niv@us.ibm.com, tglx@linutronix.de,
	Valdis.Kletnieks@vt.edu, dhowells@redhat.com,
	eric.dumazet@gmail.com, darren@dvhart.com, fweisbec@gmail.com,
	patches@linaro.org, "Paul E. McKenney" <paul.mckenney@linaro.org>
Subject: Re: [PATCH RFC tip/core/rcu 4/4] rcu: Ensure that RCU_FAST_NO_HZ timers expire on correct CPU
Date: Thu, 26 Apr 2012 10:56:08 -0700	[thread overview]
Message-ID: <20120426175608.GG2407@linux.vnet.ibm.com> (raw)
In-Reply-To: <1335456539.28106.185.camel@gandalf.stny.rr.com>

On Thu, Apr 26, 2012 at 12:08:59PM -0400, Steven Rostedt wrote:
> On Thu, 2012-04-26 at 08:54 -0700, Paul E. McKenney wrote:
> 
> > > The simpler change looks to use mod_timer_pinned()
> > 
> > Good point!
> > 
> > Except...  Now that you mention it, I don't see how mod_timer_pinned()
> > actually helps.  It looks to me like a CPU-hotplug operation will
> > migrate the timers anyway.
> > 
> > This is actually (in theory) harmless in the RCU_FAST_NO_HZ case, because
> > the CPU_DYING stuff will force a wakeup of the CPU in question, which
> > will cancel the timer.  But still, mod_timer_pinned() has a rather
> > misleading name.  ;-)
> > 
> > But a line is a line, so I made this change.
> 
> It's expected that if you use this (or anything else pinned to a CPU)
> that you add the hotplug hooks to handle a CPU going down.
> 
> There's only two users of this that I see. One is
> arch/x86/kernel/apic/x2apic_uv_x.c, that has the hotplug handling. The
> other is drivers/net/ethernet/tile/tilepro.c, that does not have hotplug
> handling, but the tile arch does not support hotplug anyway:
> 
> arch/tile/kernel/process.c: cpu_idle()
> 
> 			if (cpu_is_offline(cpu))
> 				BUG();  /* no HOTPLUG_CPU */

So what mod_timer_pinned() is really doing is ensuring that the timer
is registered on the current CPU instead of being registered on some
other CPU due to idleness considerations.  As you say, unless you do
something in CPU-hotplug notifiers, the timer -will- be migrated
at CPU-hotplug time.

Given that, wouldn't it make sense for the mod_timer_pinned() block
comment to say that?

							Thanx, Paul

------------------------------------------------------------------------

timer: Fix mod_timer_pinned() header comment

The mod_timer_pinned() header comment states that it prevents timers
from being migrated to a different CPU.  This is not the case, instead,
it ensures that the timer is posted to the current CPU, but does nothing
to prevent CPU-hotplug operations from migrating the timer.

This commit therefore brings the comment header into alignment with
reality.

Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/kernel/timer.c b/kernel/timer.c
index a297ffc..7114336 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -861,7 +861,12 @@ EXPORT_SYMBOL(mod_timer);
  *
  * mod_timer_pinned() is a way to update the expire field of an
  * active timer (if the timer is inactive it will be activated)
- * and not allow the timer to be migrated to a different CPU.
+ * and to ensure that the timer is scheduled on the current CPU.
+ * Note that this does not prevent the timer from being migrated
+ * when the current CPU goes offline.  If this is a problem for
+ * you, use CPU-hotplug notifiers to handle it correctly, for
+ * example, cancelling the timer when the corresponding CPU goes
+ * offline.
  *
  * mod_timer_pinned(timer, expires) is equivalent to:
  *


  reply	other threads:[~2012-04-26 17:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-23 16:15 [PATCH RFC tip/core/rcu 0/4] Updates to RCU_FAST_NO_HZ Paul E. McKenney
2012-04-23 16:15 ` [PATCH RFC tip/core/rcu 1/4] rcu: Add RCU_FAST_NO_HZ tracing for idle exit Paul E. McKenney
2012-04-23 16:15   ` [PATCH RFC tip/core/rcu 2/4] rcu: Make RCU_FAST_NO_HZ use timer rather than hrtimer Paul E. McKenney
2012-04-23 16:16   ` [PATCH RFC tip/core/rcu 3/4] rcu: Make RCU_FAST_NO_HZ account for pauses out of idle Paul E. McKenney
2012-04-26 13:00     ` Peter Zijlstra
2012-04-26 14:49       ` Paul E. McKenney
2012-04-26 15:09         ` Peter Zijlstra
2012-04-23 16:16   ` [PATCH RFC tip/core/rcu 4/4] rcu: Ensure that RCU_FAST_NO_HZ timers expire on correct CPU Paul E. McKenney
2012-04-26 13:04     ` Peter Zijlstra
2012-04-26 15:54       ` Paul E. McKenney
2012-04-26 16:08         ` Steven Rostedt
2012-04-26 17:56           ` Paul E. McKenney [this message]
2012-04-26 18:10             ` [PATCH] timer: Fix mod_timer_pinned() header commen (was: [PATCH RFC tip/core/rcu 4/4] rcu: Ensure that RCU_FAST_NO_HZ timers expire on correct CPU) Steven Rostedt
2012-04-26 18:36               ` 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=20120426175608.GG2407@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=akpm@linux-foundation.org \
    --cc=darren@dvhart.com \
    --cc=dhowells@redhat.com \
    --cc=dipankar@in.ibm.com \
    --cc=eric.dumazet@gmail.com \
    --cc=fweisbec@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@polymtl.ca \
    --cc=mingo@elte.hu \
    --cc=niv@us.ibm.com \
    --cc=patches@linaro.org \
    --cc=paul.mckenney@linaro.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.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