[PATCH] Create new LED trigger for CPU activity

[PATCH] Create new LED trigger for CPU activity

[Thomas Tuttle]

[-- Attachment #1.1: Type: text/plain, Size: 427 bytes --]

This patch creates a new LED trigger that triggers whenever the CPU is
active.  It can be configured with module parameters to trigger on any
combination of user, nice, system, or iowait time, and defaults to
including user and system time but not nice or iowait time.  I've tested
it a bit, and it seems to work, but no guarantees.  It's diffed against
2.6.17-git25.

Signed-off-by: Thomas Tuttle <thinkinginbinary@gmail.com>

[-- Attachment #1.2: ledtrig-cpu.patch --]
[-- Type: text/plain, Size: 6222 bytes --]

diff -udrN linux-2.6.17-git25/drivers/leds/Kconfig linux-2.6.17-git25-mine/drivers/leds/Kconfig
--- linux-2.6.17-git25/drivers/leds/Kconfig	2006-07-05 22:11:45.000000000 -0400
+++ linux-2.6.17-git25-mine/drivers/leds/Kconfig	2006-07-06 09:23:18.000000000 -0400
@@ -93,6 +93,13 @@
 	  This allows LEDs to be controlled by IDE disk activity.
 	  If unsure, say Y.
 
+config LEDS_TRIGGER_CPU
+	tristate "LED CPU Trigger"
+	depends LEDS_TRIGGERS
+	help
+	  This allows LEDs to be controlled by CPU activity.
+	  If unsure, say Y.
+
 config LEDS_TRIGGER_HEARTBEAT
 	tristate "LED Heartbeat Trigger"
 	depends LEDS_TRIGGERS
diff -udrN linux-2.6.17-git25/drivers/leds/ledtrig-cpu.c linux-2.6.17-git25-mine/drivers/leds/ledtrig-cpu.c
--- linux-2.6.17-git25/drivers/leds/ledtrig-cpu.c	1969-12-31 19:00:00.000000000 -0500
+++ linux-2.6.17-git25-mine/drivers/leds/ledtrig-cpu.c	2006-07-06 15:11:34.000000000 -0400
@@ -0,0 +1,146 @@
+/*
+ * LED CPU Activity Trigger, revision 3
+ *
+ * Copyright 2006 Thomas Tuttle
+ *
+ * Author: Thomas Tuttle <thinkinginbinary@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+/*
+ * ChangeLog:
+ *
+ * Revision 3 (2006/06/06):
+ *   Added module parameters to configure which time is counted as CPU
+ *     activity. (thanks to Andrew Morton)
+ *
+ * Revision 2 (2006/06/05):
+ *   Diffed against 2.6.17-git25 instead of 2.6.17.1.
+ *   Fixed several code style issues (thanks to Randy Dunlap)
+ *
+ * Revision 1 (2006/06/05):
+ *   Initial patch submitted.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/timer.h>
+#include <linux/leds.h>
+#include <linux/kernel_stat.h>
+#include <asm/cputime.h>
+
+#define UPDATE_INTERVAL (5) /* delay between updates, in ms */
+
+/* This *would* be an enum, but I use them as the private data in the
+ * include_* module parameters, so they know which kind of time to 
+ * include or exclude. */
+
+#define NUM_TIME_TYPES 4
+
+static int USER   = 0;
+static int NICE   = 1;
+static int SYSTEM = 2;
+static int IOWAIT = 3;
+
+static int include_time[NUM_TIME_TYPES] = { 1, 0, 1, 0 };
+
+static void ledtrig_cpu_timerfunc(unsigned long data);
+
+DEFINE_LED_TRIGGER(ledtrig_cpu);
+static DEFINE_TIMER(ledtrig_cpu_timer, ledtrig_cpu_timerfunc, 0, 0);
+
+static cputime64_t cpu_usage(void)
+{
+	int i;
+	cputime64_t time = cputime64_zero;
+
+	for_each_possible_cpu(i) {
+		if (include_time[USER])
+			time = cputime64_add(time, kstat_cpu(i).cpustat.user);
+		if (include_time[NICE])
+			time = cputime64_add(time, kstat_cpu(i).cpustat.nice);
+		if (include_time[SYSTEM])
+			time = cputime64_add(time, kstat_cpu(i).cpustat.system);
+		if (include_time[IOWAIT])
+			time = cputime64_add(time, kstat_cpu(i).cpustat.iowait);
+	}
+
+	return time;
+}
+
+static enum led_brightness last_led_state;
+static cputime64_t last_cputime;
+
+static void ledtrig_cpu_timerfunc(unsigned long data)
+{
+	cputime64_t this_cputime = cpu_usage();
+	/* XXX: This assumes that cputime64_t can be subtracted.
+	 * Nobody has defined cputime64_sub, so I had to do this instead. */
+	cputime64_t used_cputime = this_cputime - last_cputime;
+	enum led_brightness led_state = (used_cputime > 0) ? LED_FULL : LED_OFF;
+	if (led_state != last_led_state)
+		led_trigger_event(ledtrig_cpu, led_state);
+	last_led_state = led_state;
+	last_cputime = cpu_usage();
+
+	mod_timer(&ledtrig_cpu_timer, jiffies + msecs_to_jiffies(UPDATE_INTERVAL));
+}
+
+static int __init ledtrig_cpu_init(void)
+{
+	led_trigger_register_simple("cpu", &ledtrig_cpu);
+	led_trigger_event(ledtrig_cpu, LED_OFF);
+	last_led_state = LED_OFF;
+	last_cputime = cpu_usage();
+	mod_timer(&ledtrig_cpu_timer, jiffies + msecs_to_jiffies(UPDATE_INTERVAL));
+	return 0;
+}
+
+static void __exit ledtrig_cpu_exit(void)
+{
+	del_timer_sync(&ledtrig_cpu_timer);
+	led_trigger_event(ledtrig_cpu, LED_OFF);
+	led_trigger_unregister_simple(ledtrig_cpu);
+}
+
+static int get_include_time(char *buffer, struct kernel_param *kp)
+{
+	int which_time = *((int*)kp->arg);
+	if ((which_time < 0) || (which_time >= NUM_TIME_TYPES))
+		return -EINVAL;
+	sprintf(buffer, "%d", include_time[which_time]);
+	return strlen(buffer);
+}
+
+static int set_include_time(const char *val, struct kernel_param *kp)
+{
+	int which_time = *((int*)kp->arg);
+	int new_value;
+	if ((which_time < 0) || (which_time >= NUM_TIME_TYPES))
+		return -EINVAL;
+	if (sscanf(val, "%d", &new_value) != 1)
+		return -EINVAL;
+	include_time[which_time] = new_value;
+	return 0;
+}
+
+module_param_call(include_user, set_include_time, get_include_time, &USER, 6);
+module_param_call(include_nice, set_include_time, get_include_time, &NICE, 6);
+module_param_call(include_system, set_include_time, get_include_time, &SYSTEM, 6);
+module_param_call(include_iowait, set_include_time, get_include_time, &IOWAIT, 6);
+
+MODULE_AUTHOR("Thomas Tuttle <thinkinginbinary@gmail.com>");
+MODULE_DESCRIPTION("LED CPU Activity Trigger");
+MODULE_PARM_DESC(include_user, "Include user time when measuring CPU activity");
+MODULE_PARM_DESC(include_nice, "Include nice time when measuring CPU activity");
+MODULE_PARM_DESC(include_system, "Include system time when measuring CPU activity");
+MODULE_PARM_DESC(include_iowait, "Include IO wait time when measuring CPU activity");
+MODULE_LICENSE("GPL");
+
+module_init(ledtrig_cpu_init);
+module_exit(ledtrig_cpu_exit);
diff -udrN linux-2.6.17-git25/drivers/leds/Makefile linux-2.6.17-git25-mine/drivers/leds/Makefile
--- linux-2.6.17-git25/drivers/leds/Makefile	2006-07-05 22:11:45.000000000 -0400
+++ linux-2.6.17-git25-mine/drivers/leds/Makefile	2006-07-05 22:40:52.000000000 -0400
@@ -16,4 +16,5 @@
 # LED Triggers
 obj-$(CONFIG_LEDS_TRIGGER_TIMER)	+= ledtrig-timer.o
 obj-$(CONFIG_LEDS_TRIGGER_IDE_DISK)	+= ledtrig-ide-disk.o
+obj-$(CONFIG_LEDS_TRIGGER_CPU)		+= ledtrig-cpu.o
 obj-$(CONFIG_LEDS_TRIGGER_HEARTBEAT)	+= ledtrig-heartbeat.o

[-- Attachment #2: Type: application/pgp-signature, Size: 191 bytes --]

Re: [PATCH] Create new LED trigger for CPU activity

[Pavel Machek]

Hi!

> This patch creates a new LED trigger that triggers whenever the CPU is
> active.  It can be configured with module parameters to trigger on any
> combination of user, nice, system, or iowait time, and defaults to
> including user and system time but not nice or iowait time.  I've tested
> it a bit, and it seems to work, but no guarantees.  It's diffed against
> 2.6.17-git25.
> 
> Signed-off-by: Thomas Tuttle <thinkinginbinary@gmail.com>

I like the idea.. CPU usage led is very useful, and old led system
actually provided it. But...

> +/*
> + * ChangeLog:
> + *
> + * Revision 3 (2006/06/06):
> + *   Added module parameters to configure which time is counted as CPU
> + *     activity. (thanks to Andrew Morton)
> + *
> + * Revision 2 (2006/06/05):
> + *   Diffed against 2.6.17-git25 instead of 2.6.17.1.
> + *   Fixed several code style issues (thanks to Randy Dunlap)
> + *
> + * Revision 1 (2006/06/05):
> + *   Initial patch submitted.
> + */

Please don't include changelogs in sources.

> +#define UPDATE_INTERVAL (5) /* delay between updates, in ms */

Polling every 5 msec is going to cost _lot_ of juice. Is there a
better way?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Create new LED trigger for CPU activity

[Thomas Tuttle]

[-- Attachment #1.1: Type: text/plain, Size: 2227 bytes --]

(Reply comes after ChangeLog and Signed-off-by.)

This patch creates a new LED trigger that triggers whenever the CPU is
active.  It can be configured with module parameters to trigger on any
combination of user, nice, system, or iowait time, and defaults to
including user and system time but not nice or iowait time.  I've tested
it a bit, and it seems to work, but no guarantees.  It's diffed against
2.6.17-git25.

Signed-off-by: Thomas Tuttle <thinkinginbinary@gmail.com>

On July 06 at 19:52 EDT, Pavel Machek hastily scribbled:
> Please don't include changelogs in sources.
Okay.  I've attached an updated patch.

> > +#define UPDATE_INTERVAL (5) /* delay between updates, in ms */
> Polling every 5 msec is going to cost _lot_ of juice. Is there a
> better way?
This is a sticky issue.  My first idea was to wire something into
schedule() so that when the idle process was selected, it would switch
the LED off, and when another process was selected, it would switch it
on, but that would have been really hairy.  In addition, it wouldn't
allow the trigger to differentiate between user, system, nice, and
iowait time, which it can now.

I don't think it's that much of a performance hit (the calculations are
fairly short, and a trigger event isn't sent unless the LED state
changed), but if you disagree, I can do a couple of things:

  * Lower the frequency to every 10ms (100Hz) or 40ms (25Hz).  This will
	  reduce the performance hit, but also reduce the accuracy of the LED.
		Right now, it flickers nicely when the CPU is somewhat busy, but if
		I lower the frequency, it will more often stay on rather than
		flickering.  I guess it's a matter of taste.

	* Make the frequency configurable by a module parameter.  This way,
	  users who don't mind the performance hit can increase the speed, and
		those who do mind can lower it.
	
	* Try to hook it in to the scheduler, so it actually knows the moment
	  the CPU idles or starts doing something.  This will make it
		impossible to differentiate user/system time, and hard to
		differentiate user/nice time.

I personally think it's okay as it is, but I'm willing to make changes
if you prefer.

--Thomas Tuttle

[-- Attachment #1.2: ledtrig-cpu.patch --]
[-- Type: text/plain, Size: 5819 bytes --]

diff -udrN linux-2.6.17-git25/drivers/leds/Kconfig linux-2.6.17-git25-mine/drivers/leds/Kconfig
--- linux-2.6.17-git25/drivers/leds/Kconfig	2006-07-05 22:11:45.000000000 -0400
+++ linux-2.6.17-git25-mine/drivers/leds/Kconfig	2006-07-06 09:23:18.000000000 -0400
@@ -93,6 +93,13 @@
 	  This allows LEDs to be controlled by IDE disk activity.
 	  If unsure, say Y.
 
+config LEDS_TRIGGER_CPU
+	tristate "LED CPU Trigger"
+	depends LEDS_TRIGGERS
+	help
+	  This allows LEDs to be controlled by CPU activity.
+	  If unsure, say Y.
+
 config LEDS_TRIGGER_HEARTBEAT
 	tristate "LED Heartbeat Trigger"
 	depends LEDS_TRIGGERS
diff -udrN linux-2.6.17-git25/drivers/leds/ledtrig-cpu.c linux-2.6.17-git25-mine/drivers/leds/ledtrig-cpu.c
--- linux-2.6.17-git25/drivers/leds/ledtrig-cpu.c	1969-12-31 19:00:00.000000000 -0500
+++ linux-2.6.17-git25-mine/drivers/leds/ledtrig-cpu.c	2006-07-06 21:50:18.000000000 -0400
@@ -0,0 +1,131 @@
+/*
+ * LED CPU Activity Trigger, revision 3.5
+ *
+ * Copyright 2006 Thomas Tuttle
+ *
+ * Author: Thomas Tuttle <thinkinginbinary@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/timer.h>
+#include <linux/leds.h>
+#include <linux/kernel_stat.h>
+#include <asm/cputime.h>
+
+#define UPDATE_INTERVAL (5) /* delay between updates, in ms */
+
+/* This *would* be an enum, but I use them as the private data in the
+ * include_* module parameters, so they know which kind of time to 
+ * include or exclude. */
+
+#define NUM_TIME_TYPES 4
+
+static int USER   = 0;
+static int NICE   = 1;
+static int SYSTEM = 2;
+static int IOWAIT = 3;
+
+static int include_time[NUM_TIME_TYPES] = { 1, 0, 1, 0 };
+
+static void ledtrig_cpu_timerfunc(unsigned long data);
+
+DEFINE_LED_TRIGGER(ledtrig_cpu);
+static DEFINE_TIMER(ledtrig_cpu_timer, ledtrig_cpu_timerfunc, 0, 0);
+
+static cputime64_t cpu_usage(void)
+{
+	int i;
+	cputime64_t time = cputime64_zero;
+
+	for_each_possible_cpu(i) {
+		if (include_time[USER])
+			time = cputime64_add(time, kstat_cpu(i).cpustat.user);
+		if (include_time[NICE])
+			time = cputime64_add(time, kstat_cpu(i).cpustat.nice);
+		if (include_time[SYSTEM])
+			time = cputime64_add(time, kstat_cpu(i).cpustat.system);
+		if (include_time[IOWAIT])
+			time = cputime64_add(time, kstat_cpu(i).cpustat.iowait);
+	}
+
+	return time;
+}
+
+static enum led_brightness last_led_state;
+static cputime64_t last_cputime;
+
+static void ledtrig_cpu_timerfunc(unsigned long data)
+{
+	cputime64_t this_cputime = cpu_usage();
+	/* XXX: This assumes that cputime64_t can be subtracted.
+	 * Nobody has defined cputime64_sub, so I had to do this instead. */
+	cputime64_t used_cputime = this_cputime - last_cputime;
+	enum led_brightness led_state = (used_cputime > 0) ? LED_FULL : LED_OFF;
+	if (led_state != last_led_state)
+		led_trigger_event(ledtrig_cpu, led_state);
+	last_led_state = led_state;
+	last_cputime = cpu_usage();
+
+	mod_timer(&ledtrig_cpu_timer, jiffies + msecs_to_jiffies(UPDATE_INTERVAL));
+}
+
+static int __init ledtrig_cpu_init(void)
+{
+	led_trigger_register_simple("cpu", &ledtrig_cpu);
+	led_trigger_event(ledtrig_cpu, LED_OFF);
+	last_led_state = LED_OFF;
+	last_cputime = cpu_usage();
+	mod_timer(&ledtrig_cpu_timer, jiffies + msecs_to_jiffies(UPDATE_INTERVAL));
+	return 0;
+}
+
+static void __exit ledtrig_cpu_exit(void)
+{
+	del_timer_sync(&ledtrig_cpu_timer);
+	led_trigger_event(ledtrig_cpu, LED_OFF);
+	led_trigger_unregister_simple(ledtrig_cpu);
+}
+
+static int get_include_time(char *buffer, struct kernel_param *kp)
+{
+	int which_time = *((int*)kp->arg);
+	if ((which_time < 0) || (which_time >= NUM_TIME_TYPES))
+		return -EINVAL;
+	sprintf(buffer, "%d", include_time[which_time]);
+	return strlen(buffer);
+}
+
+static int set_include_time(const char *val, struct kernel_param *kp)
+{
+	int which_time = *((int*)kp->arg);
+	int new_value;
+	if ((which_time < 0) || (which_time >= NUM_TIME_TYPES))
+		return -EINVAL;
+	if (sscanf(val, "%d", &new_value) != 1)
+		return -EINVAL;
+	include_time[which_time] = new_value;
+	return 0;
+}
+
+module_param_call(include_user, set_include_time, get_include_time, &USER, 6);
+module_param_call(include_nice, set_include_time, get_include_time, &NICE, 6);
+module_param_call(include_system, set_include_time, get_include_time, &SYSTEM, 6);
+module_param_call(include_iowait, set_include_time, get_include_time, &IOWAIT, 6);
+
+MODULE_AUTHOR("Thomas Tuttle <thinkinginbinary@gmail.com>");
+MODULE_DESCRIPTION("LED CPU Activity Trigger");
+MODULE_PARM_DESC(include_user, "Include user time when measuring CPU activity");
+MODULE_PARM_DESC(include_nice, "Include nice time when measuring CPU activity");
+MODULE_PARM_DESC(include_system, "Include system time when measuring CPU activity");
+MODULE_PARM_DESC(include_iowait, "Include IO wait time when measuring CPU activity");
+MODULE_LICENSE("GPL");
+
+module_init(ledtrig_cpu_init);
+module_exit(ledtrig_cpu_exit);
diff -udrN linux-2.6.17-git25/drivers/leds/Makefile linux-2.6.17-git25-mine/drivers/leds/Makefile
--- linux-2.6.17-git25/drivers/leds/Makefile	2006-07-05 22:11:45.000000000 -0400
+++ linux-2.6.17-git25-mine/drivers/leds/Makefile	2006-07-05 22:40:52.000000000 -0400
@@ -16,4 +16,5 @@
 # LED Triggers
 obj-$(CONFIG_LEDS_TRIGGER_TIMER)	+= ledtrig-timer.o
 obj-$(CONFIG_LEDS_TRIGGER_IDE_DISK)	+= ledtrig-ide-disk.o
+obj-$(CONFIG_LEDS_TRIGGER_CPU)		+= ledtrig-cpu.o
 obj-$(CONFIG_LEDS_TRIGGER_HEARTBEAT)	+= ledtrig-heartbeat.o

[-- Attachment #2: Type: application/pgp-signature, Size: 191 bytes --]

Re: [PATCH] Create new LED trigger for CPU activity

[Richard Purdie]

On Thu, 2006-07-06 at 21:58 -0400, Thomas Tuttle wrote:
> On July 06 at 19:52 EDT, Pavel Machek hastily scribbled:
> > > +#define UPDATE_INTERVAL (5) /* delay between updates, in ms */
> > Polling every 5 msec is going to cost _lot_ of juice. Is there a
> > better way?
> This is a sticky issue.  My first idea was to wire something into
> schedule() so that when the idle process was selected, it would switch
> the LED off, and when another process was selected, it would switch it
> on, but that would have been really hairy.  In addition, it wouldn't
> allow the trigger to differentiate between user, system, nice, and
> iowait time, which it can now.

CPU activity was always going to be desired but tricky. At the moment, I
know of at least three ways people have implemented it, yours, one from
the NSLU2 people and the existing old ARM LED infrastructure.

For the old ARM approach, see arch/arm/kernel/process.c
NSLU2 patch:
http://trac.nslu2-linux.org/kernel/browser/trunk/patches/2.6.17/50-leds-arm-cpu-activity.patch

These other two patches are good in that they hook into the idle handler
and only do things at cpu idle time rather than running every 5ms.
Running every 5ms is not going to work well on my arm handheld (for
example). The existing ARM code is the simplest but doesn't give the
amount of configuration the NSLU2 people want.

I've just been asked for a trigger with the same behaviour as the old
ARM LED code to complicate matters (then we have no reason people can't
port their LED code over to the new interface). For that reason I'll be
writing a trigger to emulate the old ARM approach.

> I don't think it's that much of a performance hit (the calculations are
> fairly short, and a trigger event isn't sent unless the LED state
> changed), but if you disagree, I can do a couple of things:

Running every 5ms like this *is* going to hit performance, particularly
on lower CPU powered devices.

>   * Lower the frequency to every 10ms (100Hz) or 40ms (25Hz).  This will
> 	  reduce the performance hit, but also reduce the accuracy of the LED.
> 		Right now, it flickers nicely when the CPU is somewhat busy, but if
> 		I lower the frequency, it will more often stay on rather than
> 		flickering.  I guess it's a matter of taste.

At the moment you check (used_cputime > 0). If you compared that to some
small value instead of zero, you might reduce the always on threshold.
Finding that threshold might be tricky though.

> 	* Try to hook it in to the scheduler, so it actually knows the moment
> 	  the CPU idles or starts doing something.  This will make it
> 		impossible to differentiate user/system time, and hard to
> 		differentiate user/nice time.

Is that differentiation useful in practise? (I'll have to try it and see
I guess)

There is no right answer here. I want to test the different approaches
before any of these triggers get into mainline as we don't want a great
stack of different cpu activity triggers. Perhaps you could do the same
and see how you feel about the other approaches. 

Richard