Messages in this thread | | | Date | Wed, 5 Jul 2006 18:51:25 -0700 | From | "Randy.Dunlap" <> | Subject | Re: PATCH: Create new LED trigger for CPU activity (ledtrig-cpu) |
| |
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |