Messages in this thread Patch in this message | | | Date | Tue, 11 Jul 2006 09:45:41 +0200 | From | Ingo Molnar <> | Subject | [patch] lockdep: HPET/RTC fix |
| |
* Joseph Fannin <jfannin@gmail.com> 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@elte.hu>
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@elte.hu> --- 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |