lkml.org 
[lkml]   [2006]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
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/
\
 
 \ /
  Last update: 2006-07-11 09:53    [W:0.115 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site