parisc: blink loadavg LEDs on Oops

- blink loadavg LEDs only (not all LEDs) twice a second on Oops
- cancel_rearming_delayed_workqueue() is obsolete, use cancel_delayed_work_sync() instead

index f9b1266..0cb4862 100644
@@ -3,7 +3,7 @@
  *
  *      (c) Copyright 2000 Red Hat Software
  *      (c) Copyright 2000 Helge Deller <[email protected]>
- *      (c) Copyright 2001-2005 Helge Deller <[email protected]>
+ *      (c) Copyright 2001-2009 Helge Deller <[email protected]>
  *      (c) Copyright 2001 Randolph Chung <[email protected]>
  *
  *      This program is free software; you can redistribute it and/or modify

@@ -463,9 +463,13 @@ static void led_work_func (struct work_struct *unused)
 	if (likely(led_lanrxtx))  currentleds |= led_get_net_activity();
 	if (likely(led_diskio))   currentleds |= led_get_diskio_activity();
 
-	/* blink all LEDs twice a second if we got an Oops (HPMC) */
-	if (unlikely(oops_in_progress)) 
-		currentleds = (count_HZ<=(HZ/2)) ? 0 : 0xff;
+	/* blink loadavg LEDs twice per second if we got an Oops (HPMC) */
+	if (unlikely(oops_in_progress)) {
+		if (count_HZ <= (HZ/2))
+			currentleds &= ~(LED4|LED5|LED6|LED7);
+		else
+			currentleds |= (LED4|LED5|LED6|LED7);
+	}
 
 	if (currentleds != lastleds)
 	{

@@ -511,7 +515,7 @@ static int led_halt(struct notifier_block *nb, unsigned long event, void *buf)
 	
 	/* Cancel the work item and delete the queue */
 	if (led_wq) {
-		cancel_rearming_delayed_workqueue(led_wq, &led_task);
+		cancel_delayed_work_sync(&led_task);
 		destroy_workqueue(led_wq);
 		led_wq = NULL;
 	}

@@ -630,7 +634,7 @@ int lcd_print( const char *str )
 	
 	/* temporarily disable the led work task */
 	if (led_wq)
-		cancel_rearming_delayed_workqueue(led_wq, &led_task);
+		cancel_delayed_work_sync(&led_task);
 
 	/* copy display string to buffer for procfs */
 	strlcpy(lcd_text, str, sizeof(lcd_text));

Just out of curiosity, why wouldn't you want to have all leds blinking on something as serious as an Oops? I kind of liked to clearly see when something went wrong...

Cheers

Thibaut VARENE
http://www.parisc-linux.org/~varenet/

Yeah, valid question.

While doing some bugfixing on the HIL drivers I noticed that suddenly all LEDs were lit on the 715/64. It was strange, as I still was using the machine and it was running fine.

The reason why the LEDs were lit was, that oops_in_progress was set due a bug in my code. In dmesg I saw:

BUG: rwlock cpu recursion on CPU#0, modprobe/1098, 00a1599c
Backtrace:
 [<102a9c78>] _raw_write_lock+0x54/0x8c
 ....

So, whenever the kernel hits such a WARNING(), all LEDs will blink. Now after my patch, the 4 LEDs on the left side blink (they are currently not used anyway), while Heartbeat, disk-IO and LAN in/out still work as usual. So, you will notice the 4 LEDs at once as well, but in addition you still have the possibility to see if LAN goes in/out (e.g. if you ping the machine) or if the heartbeat still works (and so your machine should still be reachable).

Helge

I meant: The LEDs were blinking twice per second, not full-time on.

....

The thing is that you have a machine with more than 4 LEDs. I have B180/B132s which have only 4 leds. Thus I won't notice anything when an Oops occurs. If you really don't want to have the old behaviour (which, again, I really found nice), how about having the heartbeat LED fixed when an oops occurs.

Even better, a way to "reset" the state of the leds through /proc would solve both problems: an oops occurs, all/some leds flash/remain lit, you (try to) login, figure out the oops wasn't so serious, echo something > /proc/pdc/led and the LEDs start blinking normally again.

Anyway, the key point here is that not all machines without LCD have more than 4 LEDs ;)

Thoughts?

Cheers,
T-Bone

Thibaut VARENE
http://www.parisc-linux.org/~varenet/

I think you are right. My b160L has 4 LEDs only as well.
So, just changing the "upper" 4 LEDs is the wrong way.
Then I was thinking to just check for the machine type or processor type and make it dependend on that. No good idea either.

In the end, I think it's the fault of CONFIG_DEBUG_SPINLOCK. If this is enabled, it just turns sets oops_in_progress=1 as soon as a spinlock was wrong. Either way, if this happens something is wrong, and the LEDs should flash.
That said, I think we should just drop my patch. If they all flash as soon as an error happened this is just right.
In a normal productive system a bad spinlock shouldn't happen either, so my patch just adds some unnecessary noise.

Helge