Updates to RCU_FAST_NO_HZ

RFC PATCH In review Paul E. McKenney 3 participants 14 messages Apr 23, 2012 – Apr 26, 2012 Tracing

lore

Share & tools

lore

Paul E. McKenney

author

2012-04-23 16:16 UTC #0 lore raw

cover Updates to RCU_FAST_NO_HZ

Hello!

This series covers enhancements to RCU_FAST_NO_HZ intended for v3.4-rc2. The individual commits are as follows:

1. Add tracing to rcu_cleanup_after_idle().
2. Switch from hrtimer to normal timers to reduce overhead.
3. Upgrade RCU_FAST_NO_HZ state machine to better account for
brief pauses out of idle, which can happen due to power-management actions, Steven Rostedt's new RCU-idle-capable tracing macros, or my RCU_NONIDLE() macro.
4. Use add_timer_on() to avoid timer migration.

Thanx, Paul

 b/include/trace/events/rcu.h |    1 
 b/kernel/rcutree.c           |    2 
 b/kernel/rcutree.h           |    1 
 b/kernel/rcutree_plugin.h    |    1 
 kernel/rcutree_plugin.h      |  112 ++++++++++++++++++++++++++++---------------
 5 files changed, 80 insertions(+), 37 deletions(-)

Paul E. McKenney

author

2012-04-23 16:17 UTC #1 lore raw

RFC PATCH 1/4 rcu: Add RCU_FAST_NO_HZ tracing for idle exit

From: "Paul E. McKenney" <[email protected]>

Traces of rcu_prep_idle events can be confusing because rcu_cleanup_after_idle() does no tracing. This commit therefore adds this tracing.

Signed-off-by Paul E. McKenney <[email protected]>

---
 include/trace/events/rcu.h |    1 +
 kernel/rcutree_plugin.h    |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

include/trace/events/rcu.h +1 -0

index 3370997..aaa55e1 100644
@@ -292,6 +292,7 @@ TRACE_EVENT(rcu_dyntick,
  *	"More callbacks": Still more callbacks, try again to clear them out.
  *	"Callbacks drained": All callbacks processed, off to dyntick idle!
  *	"Timer": Timer fired to cause CPU to continue processing callbacks.
+ *	"Cleanup after idle": Idle exited, timer canceled.
  */
 TRACE_EVENT(rcu_prep_idle,

kernel/rcutree_plugin.h +1 -0

index c023464..1e561ab 100644
@@ -2085,6 +2085,7 @@ static void rcu_prepare_for_idle_init(int cpu)
 static void rcu_cleanup_after_idle(int cpu)
 {
 	hrtimer_cancel(&per_cpu(rcu_idle_gp_timer, cpu));
+	trace_rcu_prep_idle("Cleanup after idle");
 }
 
 /*

Signature · 1 line

1.7.8

Paul E. McKenney

author

2012-04-23 16:16 UTC #2 lore raw

RFC PATCH 2/4 rcu: Make RCU_FAST_NO_HZ use timer rather than hrtimer

From: "Paul E. McKenney" <[email protected]>

The RCU_FAST_NO_HZ facility uses an hrtimer to wake up a CPU when it is allowed to go into dyntick-idle mode, which is almost always cancelled soon after. This is not what hrtimers are good at, so this commit switches to the timer wheel.

Reported-by Steven Rostedt <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutree_plugin.h |   40 ++++++++++++----------------------------
 1 files changed, 12 insertions(+), 28 deletions(-)

kernel/rcutree_plugin.h +12 ~~-28~~

index 1e561ab..0f007b3 100644
@@ -1980,9 +1980,7 @@ static void rcu_prepare_for_idle(int cpu)
 
 static DEFINE_PER_CPU(int, rcu_dyntick_drain);
 static DEFINE_PER_CPU(unsigned long, rcu_dyntick_holdoff);
-static DEFINE_PER_CPU(struct hrtimer, rcu_idle_gp_timer);
-static ktime_t rcu_idle_gp_wait;	/* If some non-lazy callbacks. */
-static ktime_t rcu_idle_lazy_gp_wait;	/* If only lazy callbacks. */
+static DEFINE_PER_CPU(struct timer_list, rcu_idle_gp_timer);
 
 /*
  * Allow the CPU to enter dyntick-idle mode if either: (1) There are no

@@ -2051,10 +2049,9 @@ static bool rcu_cpu_has_nonlazy_callbacks(int cpu)
  * real work is done upon re-entry to idle, or by the next scheduling-clock
  * interrupt should idle not be re-entered.
  */
-static enum hrtimer_restart rcu_idle_gp_timer_func(struct hrtimer *hrtp)
+static void rcu_idle_gp_timer_func(unsigned long unused)
 {
 	trace_rcu_prep_idle("Timer");
-	return HRTIMER_NORESTART;
 }
 
 /*

@@ -2062,19 +2059,8 @@ static enum hrtimer_restart rcu_idle_gp_timer_func(struct hrtimer *hrtp)
  */
 static void rcu_prepare_for_idle_init(int cpu)
 {
-	static int firsttime = 1;
-	struct hrtimer *hrtp = &per_cpu(rcu_idle_gp_timer, cpu);
-
-	hrtimer_init(hrtp, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
-	hrtp->function = rcu_idle_gp_timer_func;
-	if (firsttime) {
-		unsigned int upj = jiffies_to_usecs(RCU_IDLE_GP_DELAY);
-
-		rcu_idle_gp_wait = ns_to_ktime(upj * (u64)1000);
-		upj = jiffies_to_usecs(RCU_IDLE_LAZY_GP_DELAY);
-		rcu_idle_lazy_gp_wait = ns_to_ktime(upj * (u64)1000);
-		firsttime = 0;
-	}
+	setup_timer(&per_cpu(rcu_idle_gp_timer, cpu),
+		    rcu_idle_gp_timer_func, 0);
 }
 
 /*

@@ -2084,7 +2070,7 @@ static void rcu_prepare_for_idle_init(int cpu)
  */
 static void rcu_cleanup_after_idle(int cpu)
 {
-	hrtimer_cancel(&per_cpu(rcu_idle_gp_timer, cpu));
+	del_timer(&per_cpu(rcu_idle_gp_timer, cpu));
 	trace_rcu_prep_idle("Cleanup after idle");
 }

@@ -2141,11 +2127,11 @@ static void rcu_prepare_for_idle(int cpu)
 		per_cpu(rcu_dyntick_drain, cpu) = 0;
 		per_cpu(rcu_dyntick_holdoff, cpu) = jiffies;
 		if (rcu_cpu_has_nonlazy_callbacks(cpu))
-			hrtimer_start(&per_cpu(rcu_idle_gp_timer, cpu),
-				      rcu_idle_gp_wait, HRTIMER_MODE_REL);
+			mod_timer(&per_cpu(rcu_idle_gp_timer, cpu),
+					   jiffies + RCU_IDLE_GP_DELAY);
 		else
-			hrtimer_start(&per_cpu(rcu_idle_gp_timer, cpu),
-				      rcu_idle_lazy_gp_wait, HRTIMER_MODE_REL);
+			mod_timer(&per_cpu(rcu_idle_gp_timer, cpu),
+					   jiffies + RCU_IDLE_LAZY_GP_DELAY);
 		return; /* Nothing more to do immediately. */
 	} else if (--per_cpu(rcu_dyntick_drain, cpu) <= 0) {
 		/* We have hit the limit, so time to give up. */

@@ -2193,14 +2179,12 @@ static void rcu_prepare_for_idle(int cpu)
 
 static void print_cpu_stall_fast_no_hz(char *cp, int cpu)
 {
-	struct hrtimer *hrtp = &per_cpu(rcu_idle_gp_timer, cpu);
+	struct timer_list *tltp = &per_cpu(rcu_idle_gp_timer, cpu);
 
-	sprintf(cp, "drain=%d %c timer=%lld",
+	sprintf(cp, "drain=%d %c timer=%lu",
 		per_cpu(rcu_dyntick_drain, cpu),
 		per_cpu(rcu_dyntick_holdoff, cpu) == jiffies ? 'H' : '.',
-		hrtimer_active(hrtp)
-			? ktime_to_us(hrtimer_get_remaining(hrtp))
-			: -1);
+		timer_pending(tltp) ? tltp->expires - jiffies : -1);
 }
 
 #else /* #ifdef CONFIG_RCU_FAST_NO_HZ */

Signature · 1 line

1.7.8

Paul E. McKenney

author

2012-04-23 16:17 UTC #3 lore raw

RFC PATCH 3/4 rcu: Make RCU_FAST_NO_HZ account for pauses out of idle

In reply to Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

Both Steven Rostedt's new idle-capable trace macros and the RCU_NONIDLE() macro can cause RCU to momentarily pause out of idle without the rest of the system being involved. This can cause rcu_prepare_for_idle() to run through its state machine too quickly, which can in turn result in needless scheduling-clock interrupts.

This commit therefore adds code to enable rcu_prepare_for_idle() to distinguish between an initial entry to idle on the one hand (which needs to advance the rcu_prepare_for_idle() state machine) and an idle reentry due to idle-capable trace macros and RCU_NONIDLE() on the other hand (which should avoid advancing the rcu_prepare_for_idle() state machine). Additional state is maintained to allow the timer to be correctly reposted when returning after a momentary pause out of idle, and even more state is maintained to detect when new non-lazy callbacks have been enqueued (which may require re-evaluation of the approach to idleness).

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutree.c        |    2 +
 kernel/rcutree.h        |    1 +
 kernel/rcutree_plugin.h |   57 +++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 56 insertions(+), 4 deletions(-)

kernel/rcutree.c +2 -0

index 1050d6d..403306b 100644
@@ -1829,6 +1829,8 @@ __call_rcu(struct rcu_head *head, void (*func)(struct rcu_head *rcu),
 	rdp->qlen++;
 	if (lazy)
 		rdp->qlen_lazy++;
+	else
+		rcu_idle_count_callbacks_posted();
 
 	if (__is_kfree_rcu_offset((unsigned long)func))
 		trace_rcu_kfree_callback(rsp->name, head, (unsigned long)func,

kernel/rcutree.h +1 -0

index cdd1be0..36ca28e 100644
@@ -471,6 +471,7 @@ static void __cpuinit rcu_prepare_kthreads(int cpu);
 static void rcu_prepare_for_idle_init(int cpu);
 static void rcu_cleanup_after_idle(int cpu);
 static void rcu_prepare_for_idle(int cpu);
+static void rcu_idle_count_callbacks_posted(void);
 static void print_cpu_stall_info_begin(void);
 static void print_cpu_stall_info(struct rcu_state *rsp, int cpu);
 static void print_cpu_stall_info_end(void);

kernel/rcutree_plugin.h +53 -4

94 lines not on this page · raw message

Signature · 1 line

1.7.8

Peter Zijlstra

2012-04-26 13:01 UTC #4 lore raw

Re: [PATCH RFC tip/core/rcu 3/4] rcu: Make RCU_FAST_NO_HZ account for pauses out of idle

kernel/rcutree_plugin.h static void rcu_prepare_for_idle(int cpu) · line 1995

Quoted 7 lines · Paul E. McKenney

static DEFINE_PER_CPU(int, rcu_dyntick_drain); static DEFINE_PER_CPU(unsigned long, rcu_dyntick_holdoff); static DEFINE_PER_CPU(struct timer_list, rcu_idle_gp_timer);
kernel/rcutree_plugin.h +4 -0
+static DEFINE_PER_CPU(unsigned long, rcu_idle_gp_timer_expires);
+static DEFINE_PER_CPU(bool, rcu_idle_first_pass);
+static DEFINE_PER_CPU(unsigned long, rcu_nonlazy_posted);
+static DEFINE_PER_CPU(unsigned long, rcu_nonlazy_posted_snap);

Wouldn't that all be prettier if it were in a struct of sorts?

struct rcu_dyntick {
	int			drain;
	unsigned long		holdoff;
	struct timer_list	gp_timer;
	unsigned long		gp_timer_expires;
	bool			first_pass;
	unsigned long		posted;
	unsigned long		posted_snap;
};

static DEFINE_PER_CPU(struct rcu_dyntick, rcu_dyntick);

( fwiw, bool doesn't have a well specified storage type )

This way you have more control over the placement, variables are forced to be together, instead of at the mercy of whatever per_cpu and the linker do, and you more clearly see the holes in the structure.

All the per_cpu() usage should still work like:

  per_cpu(rcu_dyntick.first_pass, cpu) = 0;

Paul E. McKenney

author

2012-04-26 14:50 UTC #5 lore raw

Re: [PATCH RFC tip/core/rcu 3/4] rcu: Make RCU_FAST_NO_HZ account for pauses out of idle

Quoted 26 lines · Peter Zijlstra

kernel/rcutree_plugin.h static void rcu_prepare_for_idle(int cpu) · line 1995
Quoted 7 lines · Paul E. McKenney
static DEFINE_PER_CPU(int, rcu_dyntick_drain); static DEFINE_PER_CPU(unsigned long, rcu_dyntick_holdoff); static DEFINE_PER_CPU(struct timer_list, rcu_idle_gp_timer);
kernel/rcutree_plugin.h +4 -0
+static DEFINE_PER_CPU(unsigned long, rcu_idle_gp_timer_expires);
+static DEFINE_PER_CPU(bool, rcu_idle_first_pass);
+static DEFINE_PER_CPU(unsigned long, rcu_nonlazy_posted);
+static DEFINE_PER_CPU(unsigned long, rcu_nonlazy_posted_snap);
Wouldn't that all be prettier if it were in a struct of sorts?
struct rcu_dyntick {
	int			drain;
	unsigned long		holdoff;
	struct timer_list	gp_timer;
	unsigned long		gp_timer_expires;
	bool			first_pass;
	unsigned long		posted;
	unsigned long		posted_snap;
};
static DEFINE_PER_CPU(struct rcu_dyntick, rcu_dyntick);
( fwiw, bool doesn't have a well specified storage type )

Quoted 7 lines

This way you have more control over the placement, variables are forced to be together, instead of at the mercy of whatever per_cpu and the linker do, and you more clearly see the holes in the structure.
All the per_cpu() usage should still work like:
  per_cpu(rcu_dyntick.first_pass, cpu) = 0;

Excellent point -- I have added that transformation to the todo list for 3.6.

I must confess that I had no idea that you could say the above. I would have expected to have to say the following:

    per_cpu(rcu_dyntick, cpu).first_pass = 0;

Not much difference either way, though.

Thanx, Paul

Peter Zijlstra

2012-04-26 15:10 UTC #6 lore raw

Re: [PATCH RFC tip/core/rcu 3/4] rcu: Make RCU_FAST_NO_HZ account for pauses out of idle

Quoted 6 lines · Paul E. McKenney

Quoted 1 line
  per_cpu(rcu_dyntick.first_pass, cpu) = 0;
I must confess that I had no idea that you could say the above. I would have expected to have to say the following:
    per_cpu(rcu_dyntick, cpu).first_pass = 0; 

They both work and are afaik identical. The first takes the per-cpu address of rcu_dyntick and adds the offset of first_pass, then transforms the per-cpu address to a linear address. The second transforms the per-cpu address of rcu_dyntick, transforms it to a linear address and then adds the offset of first_pass, IOW:

(&rcu_dyntick + offset) + per_cpu_offset ==
(&rcu_dyntick + per_cpu_offset) + offset

Paul E. McKenney

author

2012-04-23 16:17 UTC #7 lore raw

RFC PATCH 4/4 rcu: Ensure that RCU_FAST_NO_HZ timers expire on correct CPU

In reply to Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

Timers are subject to migration, which can lead to the following system-hang scenario when CONFIG_RCU_FAST_NO_HZ=y:

1. CPU 0 executes synchronize_rcu(), which posts an RCU callback.

2. CPU 0 then goes idle. It cannot immediately invoke the callback,
but there is nothing RCU needs from ti, so it enters dyntick-idle mode after posting a timer.

3. The timer gets migrated to CPU 1.

4. CPU 0 never wakes up, so the synchronize_rcu() never returns, so
the system hangs.

This commit fixes this problem by using add_timer_on() to ensure that the timer remains on the CPU that posted it.

Reported-by Dipankar Sarma <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 kernel/rcutree_plugin.h |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

kernel/rcutree_plugin.h +10 -5

index 50c1797..90b3de5 100644
@@ -2109,6 +2109,8 @@ static void rcu_cleanup_after_idle(int cpu)
  */
 static void rcu_prepare_for_idle(int cpu)
 {
+	struct timer_list *tp;
+
 	/*
 	 * If this is an idle re-entry, for example, due to use of
 	 * RCU_NONIDLE() or the new idle-loop tracing API within the idle

@@ -2120,9 +2122,11 @@ static void rcu_prepare_for_idle(int cpu)
 	if (!per_cpu(rcu_idle_first_pass, cpu) &&
 	    (per_cpu(rcu_nonlazy_posted, cpu) ==
 	     per_cpu(rcu_nonlazy_posted_snap, cpu))) {
-		if (rcu_cpu_has_callbacks(cpu))
-			mod_timer(&per_cpu(rcu_idle_gp_timer, cpu),
-				  per_cpu(rcu_idle_gp_timer_expires, cpu));
+		if (rcu_cpu_has_callbacks(cpu)) {
+			tp = &per_cpu(rcu_idle_gp_timer, cpu);
+			tp->expires = per_cpu(rcu_idle_gp_timer_expires, cpu);
+			add_timer_on(tp, cpu);
+		}
 		return;
 	}
 	per_cpu(rcu_idle_first_pass, cpu) = 0;

@@ -2166,8 +2170,9 @@ static void rcu_prepare_for_idle(int cpu)
 		else
 			per_cpu(rcu_idle_gp_timer_expires, cpu) =
 					   jiffies + RCU_IDLE_LAZY_GP_DELAY;
-		mod_timer(&per_cpu(rcu_idle_gp_timer, cpu),
-			  per_cpu(rcu_idle_gp_timer_expires, cpu));
+		tp = &per_cpu(rcu_idle_gp_timer, cpu);
+		tp->expires = per_cpu(rcu_idle_gp_timer_expires, cpu);
+		add_timer_on(tp, cpu);
 		per_cpu(rcu_nonlazy_posted_snap, cpu) =
 			per_cpu(rcu_nonlazy_posted, cpu);
 		return; /* Nothing more to do immediately. */

Signature · 1 line

1.7.8

Peter Zijlstra

2012-04-26 13:05 UTC #8 lore raw

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

kernel/rcutree_plugin.h static void rcu_prepare_for_idle(int cpu) · line 2128

Quoted 5 lines · Paul E. McKenney

kernel/rcutree_plugin.h +3 -2

-               mod_timer(&per_cpu(rcu_idle_gp_timer, cpu),
-                         per_cpu(rcu_idle_gp_timer_expires, cpu));
+               tp = &per_cpu(rcu_idle_gp_timer, cpu);
+               tp->expires = per_cpu(rcu_idle_gp_timer_expires, cpu);
+               add_timer_on(tp, cpu);

The simpler change looks to use mod_timer_pinned()

Paul E. McKenney

author

2012-04-26 15:55 UTC #9 lore raw

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

Quoted 8 lines · Peter Zijlstra

kernel/rcutree_plugin.h static void rcu_prepare_for_idle(int cpu) · line 2128

Quoted 5 lines · Paul E. McKenney

kernel/rcutree_plugin.h +3 -2

-               mod_timer(&per_cpu(rcu_idle_gp_timer, cpu),
-                         per_cpu(rcu_idle_gp_timer_expires, cpu));
+               tp = &per_cpu(rcu_idle_gp_timer, cpu);
+               tp->expires = per_cpu(rcu_idle_gp_timer_expires, cpu);
+               add_timer_on(tp, cpu);

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.

Thanx, Paul

Steven Rostedt

maintainer

2012-04-26 16:09 UTC #10 lore raw

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

Quoted 8 lines · Paul E. McKenney

Quoted 1 line
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.

Quoted 7 lines

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

-- Steve

Paul E. McKenney

author

2012-04-26 17:56 UTC #11 lore raw

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

Quoted 25 lines · Steven Rostedt

Quoted 8 lines · Paul E. McKenney
Quoted 1 line
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.
Quoted 6 lines
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:

Quoted 4 lines

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 <[email protected]>

kernel/timer.c +6 -1

index a297ffc..7114336 100644
@@ -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:
  *

Steven Rostedt

maintainer Acked-by

2012-04-26 18:10 UTC #12 lore raw

Re: [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)

Quoted 10 lines

------------------------------------------------------------------------
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.

Quoted 5 lines

This commit therefore brings the comment header into alignment with reality.
Signed-off-by Paul E. McKenney <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>

I'm seeing double :-)

kernel/timer.c EXPORT_SYMBOL(mod_timer); · line 864

Quoted 11 lines

kernel/timer.c +1 -1

index a297ffc..7114336 100644
@@ -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.

Add space here.

+ *

kernel/timer.c EXPORT_SYMBOL(mod_timer); · line 872

Quoted 8 lines

kernel/timer.c +5 -0

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

Adding a space will make the "Note" stand out more, and more likely seen by people who use this function.

Other than that:

Acked-by Steven Rostedt <[email protected]>

-- Steve

Paul E. McKenney

author Acked-by Steven Rostedt

2012-04-26 18:46 UTC #13 lore raw

Re: [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)

Quoted 10 lines · Steven Rostedt

Quoted 10 lines
------------------------------------------------------------------------
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.

Quoted 7 lines

Quoted 5 lines
This commit therefore brings the comment header into alignment with reality.
Signed-off-by Paul E. McKenney <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>
I'm seeing double :-)

"Bartender, give me a double!"

Quoted 13 lines

kernel/timer.c EXPORT_SYMBOL(mod_timer); · line 864

Quoted 10 lines

kernel/timer.c +1 -1

index a297ffc..7114336 100644
@@ -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.

Add space here.

Quoted 1 line

+ *

Good point, done!

Quoted 9 lines

kernel/timer.c EXPORT_SYMBOL(mod_timer); · line 872

Quoted 8 lines

kernel/timer.c +5 -0

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

Quoted 6 lines

Adding a space will make the "Note" stand out more, and more likely seen by people who use this function.
Other than that:
Acked-by Steven Rostedt <[email protected]>

Got it, thank you!

Thanx, Paul

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

timer: Fix mod_timer_pinned() header comment

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

Signed-off-by Paul E. McKenney <[email protected]>

Acked-by Steven Rostedt <[email protected]>

kernel/timer.c +7 -1

index a297ffc..837c552 100644
@@ -861,7 +861,13 @@ 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:
  *