CPU LED trigger: add hooks to generic code so that it works on x86

CPU LED trigger: add hooks to generic code so that it works on x86

[Pavel Machek]

CPU LED trigger hooks are currently hidden in arm-specific code, which
means that this trigger only works on arm. Add it to the generic
code, so that it works on x86, too.

Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index fefcb1f..087acd6 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -8,6 +8,7 @@
 #include <linux/mm.h>
 #include <linux/stackprotector.h>
 #include <linux/suspend.h>
+#include <linux/leds.h>
 
 #include <asm/tlb.h>
 
@@ -232,6 +233,7 @@ static void cpu_idle_loop(void)
 			}
 
 			local_irq_disable();
+			ledtrig_cpu(CPU_LED_IDLE_START);
 			arch_cpu_idle_enter();
 
 			/*
@@ -249,6 +251,7 @@ static void cpu_idle_loop(void)
 				cpuidle_idle_call();
 
 			arch_cpu_idle_exit();
+			ledtrig_cpu(CPU_LED_IDLE_END);
 		}
 
 		/*

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: CPU LED trigger: add hooks to generic code so that it works on x86

[Peter Zijlstra]

On Wed, May 27, 2015 at 08:57:12AM +0200, Pavel Machek wrote:
> 
> CPU LED trigger hooks are currently hidden in arm-specific code, which
> means that this trigger only works on arm. Add it to the generic
> code, so that it works on x86, too.

And why do we want to go and slow down the idle loop for everyone?

Re: CPU LED trigger: add hooks to generic code so that it works on x86

[Pavel Machek]

On Wed 2015-05-27 09:43:43, Peter Zijlstra wrote:
> On Wed, May 27, 2015 at 08:57:12AM +0200, Pavel Machek wrote:
> > 
> > CPU LED trigger hooks are currently hidden in arm-specific code, which
> > means that this trigger only works on arm. Add it to the generic
> > code, so that it works on x86, too.
> 
> And why do we want to go and slow down the idle loop for everyone?

If the trigger is not configured, this is not slowing down anyone.

If the trigger is configured, well, then the user requested the
functionality. So we are not slowing down _everyone_, just the people
that want the blinking.

								Pavel

include/linux/leds.h:

#ifdef CONFIG_LEDS_TRIGGER_CPU
extern void ledtrig_cpu(enum cpu_led_event evt);
#else
static inline void ledtrig_cpu(enum cpu_led_event evt)
{
	return;
	}
#endif
	

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: CPU LED trigger: add hooks to generic code so that it works on x86

[Peter Zijlstra]

On Wed, May 27, 2015 at 09:47:57AM +0200, Pavel Machek wrote:
> On Wed 2015-05-27 09:43:43, Peter Zijlstra wrote:
> > On Wed, May 27, 2015 at 08:57:12AM +0200, Pavel Machek wrote:
> > > 
> > > CPU LED trigger hooks are currently hidden in arm-specific code, which
> > > means that this trigger only works on arm. Add it to the generic
> > > code, so that it works on x86, too.
> > 
> > And why do we want to go and slow down the idle loop for everyone?
> 
> If the trigger is not configured, this is not slowing down anyone.
> 
> If the trigger is configured, well, then the user requested the
> functionality. So we are not slowing down _everyone_, just the people
> that want the blinking.

What kind of x86 hardware will support this? If its _everything_ distros
will have to enable this and it suddenly becomes a lot of people.

Re: CPU LED trigger: add hooks to generic code so that it works on x86

[Pavel Machek]

On Wed 2015-05-27 10:08:31, Peter Zijlstra wrote:
> On Wed, May 27, 2015 at 09:47:57AM +0200, Pavel Machek wrote:
> > On Wed 2015-05-27 09:43:43, Peter Zijlstra wrote:
> > > On Wed, May 27, 2015 at 08:57:12AM +0200, Pavel Machek wrote:
> > > > 
> > > > CPU LED trigger hooks are currently hidden in arm-specific code, which
> > > > means that this trigger only works on arm. Add it to the generic
> > > > code, so that it works on x86, too.
> > > 
> > > And why do we want to go and slow down the idle loop for everyone?
> > 
> > If the trigger is not configured, this is not slowing down anyone.
> > 
> > If the trigger is configured, well, then the user requested the
> > functionality. So we are not slowing down _everyone_, just the people
> > that want the blinking.
> 
> What kind of x86 hardware will support this? If its _everything_ distros
> will have to enable this and it suddenly becomes a lot of people.

It needs directly connected leds. It is useful on thinkpad t40p, for
example.

Distros should not enable this, IMO. But I don't think overhead is
that big in either case.

But I wanted to enable it, and result was trigger that can be
configured, enabled, and just plain does not work (which is nasty).
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: CPU LED trigger: add hooks to generic code so that it works on x86

[Peter Zijlstra]

On Wed, May 27, 2015 at 10:47:18AM +0200, Pavel Machek wrote:
> 
> Distros should not enable this, IMO. But I don't think overhead is
> that big in either case.

Coming back from idle there might not be any cache, and you're adding at
least 1 more cache miss for loading cpu_trig -- those hurt. Looking at
that code, in case of !trig, you do a pointless function call before
bailing.

The thing is, there already is too much code in the wake from idle path,
adding more is not my favourite thing.

One thing you could do is wrap the call to ledtrig_cpu() in a
static_key, that way all we do is add some i$ pollution, but at least we
avoid the data cache miss when there's nothing to do.