PATCH: Create new LED trigger for CPU activity (ledtrig-cpu)

PATCH: Create new LED trigger for CPU activity (ledtrig-cpu)

[Thomas Tuttle]

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

I'd like to apologize profusely for the incompetence of my previous
mailer.  Rest assured, the programmers responsible will be sacked, if I
ever meet them.

Here's a version that's plain text, and against 2.6.17-git25.  It *should*
simply be a plain text MIME attachment.

I'd appreciate any comments, now that you can read my patch. :-\

Thanks,

Thomas Tuttle

[-- Attachment #1.2: ledtrig-cpu.patch --]
[-- Type: text/plain, Size: 4875 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-05 22:42:58.000000000 -0400
@@ -93,6 +93,41 @@
 	  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_CPU_INCLUDE_USER
+	bool "Include user time in CPU trigger"
+	depends LEDS_TRIGGER_CPU
+	default y
+	help
+	  This option makes user CPU time cause the CPU trigger to activate.
+
+config LEDS_TRIGGER_CPU_INCLUDE_NICE
+	bool "Include nice time in CPU trigger"
+	depends LEDS_TRIGGER_CPU
+	default n
+	help
+	  This option makes nice CPU time cause the CPU trigger to activate.
+
+config LEDS_TRIGGER_CPU_INCLUDE_SYSTEM
+	bool "Include system time in CPU trigger"
+	depends LEDS_TRIGGER_CPU
+	default y
+	help
+	  This option makes system CPU time cause the CPU trigger to activate.
+
+config LEDS_TRIGGER_CPU_INCLUDE_IOWAIT
+	bool "Include iowait time in CPU trigger"
+	depends LEDS_TRIGGER_CPU
+	default n
+	help
+	  This option makes iowait CPU time cause the CPU trigger to activate.
+
 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-05 22:42:38.000000000 -0400
@@ -0,0 +1,87 @@
+/*
+ * LED CPU Activity Trigger
+ *
+ * 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/config.h>
+#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 */
+
+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) {
+#ifdef CONFIG_LEDS_TRIGGER_CPU_INCLUDE_USER
+		time = cputime64_add(time, kstat_cpu(i).cpustat.user);
+#endif
+#ifdef CONFIG_LEDS_TRIGGER_CPU_INCLUDE_NICE
+		time = cputime64_add(time, kstat_cpu(i).cpustat.nice);
+#endif
+#ifdef CONFIG_LEDS_TRIGGER_CPU_INCLUDE_SYSTEM
+		time = cputime64_add(time, kstat_cpu(i).cpustat.system);
+#endif
+#ifdef CONFIG_LEDS_TRIGGER_CPU_INCLUDE_IOWAIT
+		time = cputime64_add(time, kstat_cpu(i).cpustat.iowait);
+#endif
+	}
+
+	return time;
+}
+
+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;
+	led_trigger_event(ledtrig_cpu, 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);
+	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(&ledtrig_cpu_timer);
+	led_trigger_unregister_simple(ledtrig_cpu);
+}
+
+module_init(ledtrig_cpu_init);
+module_exit(ledtrig_cpu_exit);
+
+MODULE_AUTHOR("Thomas Tuttle <thinkinginbinary@gmail.com>");
+MODULE_DESCRIPTION("LED CPU Activity Trigger");
+MODULE_LICENSE("GPL");
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 --]

PATCH: Create new LED trigger for CPU activity (ledtrig-cpu)

[Thomas Tuttle]

[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]

Here is a patch I wrote that creates a new LED trigger for CPU
activity.  It creates a new config option, CONFIG_LEDS_TRIGGER_CPU, as
well as four suboptions to select the combination of user, nice,
system, and iowait that should turn the LED on.

It's loosely based on the ledtrig-ide-disk plugin, but only for
general guidance.  It doesn't modify anything in the CPU scheduling
code, for better or for worse; it is less efficient, because it checks
the CPU time every 5ms instead of "knowing" when the CPU is idle or
active, but it doesn't require changes to any other code, and is much
less hairy for not digging into the scheduling stuff.  (It's also more
flexible, because it can deal with user/nice/system/iowait time
without much effort.)

The same disclaimers from my last email (the asus_acpi LED support
one) apply--I haven't written much kernel code; this is diffed against
2.6.17.1, not .3, but I don't think anything has changed; I've tested
it, but no guarantees it's perfect; and I apologize for the MIME type
of text/x-patch, but it should work.

Constructive comments would be greatly appreciated.

-- 
Thomas Tuttle
http://thinkinginbinary.webhop.net/
thinkinginbinary@gmail.com
AIM: thinkinginbinary

[-- Attachment #2: ledtrig-cpu.patch --]
[-- Type: text/x-patch, Size: 4752 bytes --]

diff -udrN linux-2.6.17.1/drivers/leds/Kconfig linux-2.6.17.1-mine/drivers/leds/Kconfig
--- linux-2.6.17.1/drivers/leds/Kconfig	2006-07-05 19:39:25.000000000 -0400
+++ linux-2.6.17.1-mine/drivers/leds/Kconfig	2006-07-05 21:04:37.000000000 -0400
@@ -80,6 +80,37 @@
 	  This allows LEDs to be controlled by a programmable timer
 	  via sysfs. 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_CPU_INCLUDE_USER
+	bool "Include user time in CPU trigger"
+	depends LEDS_TRIGGER_CPU
+	help
+	  This option makes user CPU time cause the CPU trigger to activate.
+
+config LEDS_TRIGGER_CPU_INCLUDE_NICE
+	bool "Include nice time in CPU trigger"
+	depends LEDS_TRIGGER_CPU
+	help
+	  This option makes nice CPU time cause the CPU trigger to activate.
+
+config LEDS_TRIGGER_CPU_INCLUDE_SYSTEM
+	bool "Include system time in CPU trigger"
+	depends LEDS_TRIGGER_CPU
+	help
+	  This option makes system CPU time cause the CPU trigger to activate.
+
+config LEDS_TRIGGER_CPU_INCLUDE_IOWAIT
+	bool "Include iowait time in CPU trigger"
+	depends LEDS_TRIGGER_CPU
+	help
+	  This option makes iowait CPU time cause the CPU trigger to activate.
+
 config LEDS_TRIGGER_IDE_DISK
 	bool "LED IDE Disk Trigger"
 	depends LEDS_TRIGGERS && BLK_DEV_IDEDISK
diff -udrN linux-2.6.17.1/drivers/leds/ledtrig-cpu.c linux-2.6.17.1-mine/drivers/leds/ledtrig-cpu.c
--- linux-2.6.17.1/drivers/leds/ledtrig-cpu.c	1969-12-31 19:00:00.000000000 -0500
+++ linux-2.6.17.1-mine/drivers/leds/ledtrig-cpu.c	2006-07-05 21:01:54.000000000 -0400
@@ -0,0 +1,90 @@
+/*
+ * LED CPU Activity Trigger
+ *
+ * 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/config.h>
+#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) // ms
+
+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 user, nice, system, idle, iowait, total;
+	user = nice = system = idle = iowait = total = cputime64_zero;
+
+	for_each_possible_cpu(i) {
+		user = cputime64_add(user, kstat_cpu(i).cpustat.user);
+		nice = cputime64_add(nice, kstat_cpu(i).cpustat.nice);
+		system = cputime64_add(system, kstat_cpu(i).cpustat.system);
+		idle = cputime64_add(idle, kstat_cpu(i).cpustat.idle);
+		iowait = cputime64_add(iowait, kstat_cpu(i).cpustat.iowait);
+	}
+
+#ifdef CONFIG_LEDS_TRIGGER_CPU_INCLUDE_USER
+	total = cputime64_add(total, user);
+#endif
+#ifdef CONFIG_LEDS_TRIGGER_CPU_INCLUDE_NICE
+	total = cputime64_add(total, nice);
+#endif
+#ifdef CONFIG_LEDS_TRIGGER_CPU_INCLUDE_SYSTEM
+	total = cputime64_add(total, system);
+#endif
+#ifdef CONFIG_LEDS_TRIGGER_CPU_INCLUDE_IOWAIT
+	total = cputime64_add(total, iowait);
+#endif
+
+	return total;
+}
+
+cputime64_t last_cputime;
+
+static void ledtrig_cpu_timerfunc(unsigned long data) {
+	cputime64_t this_cputime = cpu_usage();
+	cputime64_t used_cputime = this_cputime - last_cputime;
+	if (used_cputime > 0) {
+		led_trigger_event(ledtrig_cpu, LED_FULL);
+	} else {
+		led_trigger_event(ledtrig_cpu, LED_OFF);
+	}
+	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);
+	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(&ledtrig_cpu_timer);
+	led_trigger_unregister_simple(ledtrig_cpu);
+}
+
+module_init(ledtrig_cpu_init);
+module_exit(ledtrig_cpu_exit);
+
+MODULE_AUTHOR("Thomas Tuttle <thinkinginbinary@gmail.com>");
+MODULE_DESCRIPTION("LED CPU Activity Trigger");
+MODULE_LICENSE("GPL");
diff -udrN linux-2.6.17.1/drivers/leds/Makefile linux-2.6.17.1-mine/drivers/leds/Makefile
--- linux-2.6.17.1/drivers/leds/Makefile	2006-07-05 19:39:25.000000000 -0400
+++ linux-2.6.17.1-mine/drivers/leds/Makefile	2006-07-05 21:07:27.000000000 -0400
@@ -15,3 +15,4 @@
 # 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

Re: PATCH: Create new LED trigger for CPU activity (ledtrig-cpu)

[Randy.Dunlap]

On Wed, 5 Jul 2006 21:24:17 -0400 Thomas Tuttle wrote:

> Here is a patch I wrote that creates a new LED trigger for CPU
> activity.  It creates a new config option, CONFIG_LEDS_TRIGGER_CPU, as
> well as four suboptions to select the combination of user, nice,
> system, and iowait that should turn the LED on.

What "the LED" does it turn on/off?
I'm curious how I can use it.

> It's loosely based on the ledtrig-ide-disk plugin, but only for
> general guidance.  It doesn't modify anything in the CPU scheduling
> code, for better or for worse; it is less efficient, because it checks
> the CPU time every 5ms instead of "knowing" when the CPU is idle or
> active, but it doesn't require changes to any other code, and is much
> less hairy for not digging into the scheduling stuff.  (It's also more
> flexible, because it can deal with user/nice/system/iowait time
> without much effort.)
> 
> The same disclaimers from my last email (the asus_acpi LED support
> one) apply--I haven't written much kernel code; this is diffed against
> 2.6.17.1, not .3, but I don't think anything has changed; I've tested
> it, but no guarantees it's perfect; and I apologize for the MIME type
> of text/x-patch, but it should work.

It may not matter, but generally you should diff against the latest
linus-mainline kernel, e.g., 2.6.7-git25 or Linus's git tree.

> Constructive comments would be greatly appreciated.

The attachment makes it difficult to review/comment.  Apparently
gmail munges inline patches so that's not an option (from what I
have read here on lkml).

Oh, where is the LED IDE patch?

Patch comments:  all seem to be for style etc.

+#define UPDATE_INTERVAL (5) // ms

Don't use // style comments, use /* ... */.

+static cputime64_t cpu_usage(void) {

Function { and } should be on separate lines.

+	for_each_possible_cpu(i) {
+		user = cputime64_add(user, kstat_cpu(i).cpustat.user);
+		nice = cputime64_add(nice, kstat_cpu(i).cpustat.nice);
+		system = cputime64_add(system, kstat_cpu(i).cpustat.system);
+		idle = cputime64_add(idle, kstat_cpu(i).cpustat.idle);
+		iowait = cputime64_add(iowait, kstat_cpu(i).cpustat.iowait);
+	}

We don't like ifdefs in C code very much, but since there are some
below here, why not here also?

+	if (used_cputime > 0) {
+		led_trigger_event(ledtrig_cpu, LED_FULL);
+	} else {
+		led_trigger_event(ledtrig_cpu, LED_OFF);
+	}

No braces for 1-statement "blocks".

---
~Randy