[patch] lockdep: HPET/RTC fix

[Ingo Molnar]

* Joseph Fannin <jfannin@xxxxxxxxx> wrote:

>     2.6.18-rc1-mm1, which includes this change, is printing this at 
> the same point I used to get the lockdep message:
> 
> [   25.628000] BUG: warning at kernel/lockdep.c:1803/trace_hardirqs_on()
> [   25.628000]  [<c0104a18>] show_trace_log_lvl+0x148/0x170
> [   25.628000]  [<c0105cab>] show_trace+0x1b/0x20
> [   25.628000]  [<c0105cd4>] dump_stack+0x24/0x30
> [   25.628000]  [<c014af4e>] trace_hardirqs_on+0xce/0x200
> [   25.628000]  [<c036cf21>] _spin_unlock_irq+0x31/0x70
> [   25.628000]  [<c0296584>] rtc_get_rtc_time+0x44/0x1a0
> [   25.628000]  [<c01198bb>] hpet_rtc_interrupt+0x21b/0x280
> [   25.628000]  [<c0161141>] handle_IRQ_event+0x31/0x70
> [   25.628000]  [<c0162d37>] handle_edge_irq+0xe7/0x210
> [   25.628000]  [<c0106192>] do_IRQ+0x92/0x120
> [   25.628000]  [<c0104121>] common_interrupt+0x25/0x2c
> [   25.628000]  [<b7f15410>] 0xb7f15410

ouch! That's another HPET bug i believe. AFAICS rtc_get_rtc_time() is 
really not meant to be called from any sort of timer interrupt! In 
particular this looping code:

        while (rtc_is_updating() != 0 && jiffies - uip_watchdog < 2*HZ/100) {
                barrier();
                cpu_relax();
        }

it utterly bad in any hardirq context. Also, the locking isnt 
hardirq-safe either:

        spin_lock_irq(&rtc_lock);
	...
        spin_unlock_irq(&rtc_lock);

as it will enable interrupts unconditionally - even if we are in a 
irqs-off hardirq.

John, how is this supposed to work?

	Ingo

---------------->
Subject: lockdep: HPET/RTC fix
From: Ingo Molnar <mingo@xxxxxxx>

Joseph Fannin reported that hpet_rtc_interrupt() enables hardirqs
in irq context:

[   25.628000]  [<c014af4e>] trace_hardirqs_on+0xce/0x200
[   25.628000]  [<c036cf21>] _spin_unlock_irq+0x31/0x70
[   25.628000]  [<c0296584>] rtc_get_rtc_time+0x44/0x1a0
[   25.628000]  [<c01198bb>] hpet_rtc_interrupt+0x21b/0x280
[   25.628000]  [<c0161141>] handle_IRQ_event+0x31/0x70
[   25.628000]  [<c0162d37>] handle_edge_irq+0xe7/0x210
[   25.628000]  [<c0106192>] do_IRQ+0x92/0x120
[   25.628000]  [<c0104121>] common_interrupt+0x25/0x2c

the call of rtc_get_rtc_time() is highly suspect. At a minimum we
need the patch below to save/restore hardirq state.

Signed-off-by: Ingo Molnar <mingo@xxxxxxx>
---
 drivers/char/rtc.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux/drivers/char/rtc.c
===================================================================
--- linux.orig/drivers/char/rtc.c
+++ linux/drivers/char/rtc.c
@@ -1222,7 +1222,7 @@ static int rtc_proc_open(struct inode *i
 
 void rtc_get_rtc_time(struct rtc_time *rtc_tm)
 {
-	unsigned long uip_watchdog = jiffies;
+	unsigned long uip_watchdog = jiffies, flags;
 	unsigned char ctrl;
 #ifdef CONFIG_MACH_DECSTATION
 	unsigned int real_year;
@@ -1249,7 +1249,7 @@ void rtc_get_rtc_time(struct rtc_time *r
 	 * RTC has RTC_DAY_OF_WEEK, we should usually ignore it, as it is
 	 * only updated by the RTC when initially set to a non-zero value.
 	 */
-	spin_lock_irq(&rtc_lock);
+	spin_lock_irqsave(&rtc_lock, flags);
 	rtc_tm->tm_sec = CMOS_READ(RTC_SECONDS);
 	rtc_tm->tm_min = CMOS_READ(RTC_MINUTES);
 	rtc_tm->tm_hour = CMOS_READ(RTC_HOURS);
@@ -1263,7 +1263,7 @@ void rtc_get_rtc_time(struct rtc_time *r
 	real_year = CMOS_READ(RTC_DEC_YEAR);
 #endif
 	ctrl = CMOS_READ(RTC_CONTROL);
-	spin_unlock_irq(&rtc_lock);
+	spin_unlock_irqrestore(&rtc_lock, flags);
 
 	if (!(ctrl & RTC_DM_BINARY) || RTC_ALWAYS_BCD)
 	{
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/