Thread (19 messages) flat view 19 messages, 6 authors, 2012-02-08
STALE5296d

[PATCH RFC idle 2/3] arm: Avoid invoking RCU when CPU is idle

From: Paul E. McKenney <hidden>
Date: 2012-02-03 19:40:05
Also in: linux-omap, linux-samsung-soc

On Fri, Feb 03, 2012 at 01:55:09PM -0500, Steven Rostedt wrote:
On Thu, 2012-02-02 at 22:04 -0800, Paul E. McKenney wrote:
quoted
On Thu, Feb 02, 2012 at 09:45:31PM -0500, Steven Rostedt wrote:
quoted
It is an atomic instruction or two, plus some memory barriers.  Entering
idle is more heavyweight for RCU_FAST_NO_HZ.  But as you say, it is
entering and exiting idle.

But should I make an empty definition of RCU_NONIDLE() for some #define
or another?

	#ifdef CONFIG_YOU_TELL_ME
	#define RCU_NONIDLE(a) \
		do { \
			rcu_idle_exit(); \
			do { a; } while (0); \
			rcu_idle_enter(); \
		} while (0)
	#else /* #ifdef CONFIG_YOU_TELL_ME */
	#define RCU_NONIDLE(a) do { } while (0);
	#endif /* #else #ifdef CONFIG_YOU_TELL_ME */

Or is event tracing unconditional these days?
I don't like it. As it binds the RCU_NONIDLE to tracepoints only without
any annotation that they are bound. Still doesn't help when tracepoints
are configured but not enabled.

I have no problem in making a special TRACE_EVENT_IDLE() that does this
inside the jump label. Basically what we have today is:


	if (static_branch(tracepoint_key)) {
		rcu_read_lock_sched_notrace();
		for (all attached tracepoints) {
			[...]
		}
		rcu_read_unlock_sched_notrace();
	}

Ideally we want the enter/exit idle inside that static_branch()
condition:

	if (static_branch(tracepoint_key)) {
		rcu_idle_exit();
		rcu_read_lock_sched_notrace();
		for (all attached tracepoints) {
			[...]
		}
		rcu_read_unlock_sched_notrace();
		rcu_idle_enter();
	}

The static_branch() is the jump label code when it's a nop when disabled
and a jump to the tracing code when enabled:

	nop; /* or jmp 2f */  <<--- jump label
1:	[ normal code ]
	ret;

2:	[trace code]
	jmp 1b


The jump label when disabled is just a nop that ignores the trace code
(although current gcc has a bug that it currently doesn't do it this
elegantly). When tracing is enabled the nop is converted to a jump to
the tracing code. This makes tracepoints very light weight in hot paths.

Ideally, we want the exit/enter rcu idle with in the [trace code], which
makes it not used when not needed.
So the idea is that if you have a trace event that is to be used in idle,
you use TRACE_EVENT_IDLE() rather than TRACE_EVENT() to declare that
trace event?  That would work for me, and might make for fewer changes
for the architecture guys.  Also, this should address the code-size
concerns we discussed yesterday.

So sounds good!

Is a DEFINE_EVENT_IDLE() also needed?  Or prehaps a
DECLARE_EVENT_CLASS_IDLE()?  My guess is "yes" for at least one of the
two based on include/trace/events/power.h.

I will keep RCU_NONIDLE() for at least a little while (reworking comments
to point out TRACE_EVENT_IDLE() and friends) in case there turn out to
be non-tracepoint uses of RCU in the idle loop.

							Thanx, Paul
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help