Re: [PATCH RFC tip/core/rcu 4/4] rcu: Ensure that RCU_FAST_NO_HZtimers expire on correct CPU

[Paul E. McKenney]

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@xxxxxxxxxx>
Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>

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:
  *

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/