2006-09-19 19:54:40

by Peter Zijlstra

Subject: [PATCH] rtc: lockdep fix/workaround

BUG: warning at kernel/lockdep.c:1816/trace_hardirqs_on() (Not tainted)
[<c04051ee>] show_trace_log_lvl+0x58/0x171
[<c0405802>] show_trace+0xd/0x10
[<c040591b>] dump_stack+0x19/0x1b
[<c043abee>] trace_hardirqs_on+0xa2/0x11e
[<c06143c3>] _spin_unlock_irq+0x22/0x26
[<c0541540>] rtc_get_rtc_time+0x32/0x176
[<c0419ba4>] hpet_rtc_interrupt+0x92/0x14d
[<c0450f94>] handle_IRQ_event+0x20/0x4d
[<c0451055>] __do_IRQ+0x94/0xef
[<c040678d>] do_IRQ+0x9e/0xbd
[<c0404a49>] common_interrupt+0x25/0x2c
DWARF2 unwinder stuck at common_interrupt+0x25/0x2c

Signed-off-by: Peter Zijlstra <[email protected]>
---
drivers/char/rtc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

Index: linux-2.6.17.noarch/drivers/char/rtc.c
===================================================================
--- linux-2.6.17.noarch.orig/drivers/char/rtc.c 2006-09-18 21:34:11.000000000 +0200
+++ linux-2.6.17.noarch/drivers/char/rtc.c 2006-09-18 21:35:03.000000000 +0200
@@ -209,11 +209,12 @@ static const unsigned char days_in_mo[]
*/
static inline unsigned char rtc_is_updating(void)
{
+ unsigned long flags;
unsigned char uip;

- spin_lock_irq(&rtc_lock);
+ spin_lock_irqsave(&rtc_lock, flags);
uip = (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP);
- spin_unlock_irq(&rtc_lock);
+ spin_unlock_irqrestore(&rtc_lock, flags);
return uip;
}

2006-09-20 08:29:24

by Ingo Molnar

Subject: Re: [PATCH] rtc: lockdep fix

* Peter Zijlstra <[email protected]> wrote:

> BUG: warning at kernel/lockdep.c:1816/trace_hardirqs_on() (Not tainted)
> [<c04051ee>] show_trace_log_lvl+0x58/0x171
> [<c0405802>] show_trace+0xd/0x10
> [<c040591b>] dump_stack+0x19/0x1b
> [<c043abee>] trace_hardirqs_on+0xa2/0x11e
> [<c06143c3>] _spin_unlock_irq+0x22/0x26
> [<c0541540>] rtc_get_rtc_time+0x32/0x176
> [<c0419ba4>] hpet_rtc_interrupt+0x92/0x14d
> [<c0450f94>] handle_IRQ_event+0x20/0x4d
> [<c0451055>] __do_IRQ+0x94/0xef
> [<c040678d>] do_IRQ+0x9e/0xbd
> [<c0404a49>] common_interrupt+0x25/0x2c

ouch! That is a scenario that could lead to real lockups. Fix looks good
and necessary for v2.6.18 to me.

> Signed-off-by: Peter Zijlstra <[email protected]>

Acked-by: Ingo Molnar <[email protected]>

Ingo

2006-09-20 08:49:28

by Arjan van de Ven

Subject: Re: [PATCH] rtc: lockdep fix

Ingo Molnar wrote:
> * Peter Zijlstra <[email protected]> wrote:
>
>> BUG: warning at kernel/lockdep.c:1816/trace_hardirqs_on() (Not tainted)
>> [<c04051ee>] show_trace_log_lvl+0x58/0x171
>> [<c0405802>] show_trace+0xd/0x10
>> [<c040591b>] dump_stack+0x19/0x1b
>> [<c043abee>] trace_hardirqs_on+0xa2/0x11e
>> [<c06143c3>] _spin_unlock_irq+0x22/0x26
>> [<c0541540>] rtc_get_rtc_time+0x32/0x176
>> [<c0419ba4>] hpet_rtc_interrupt+0x92/0x14d
>> [<c0450f94>] handle_IRQ_event+0x20/0x4d
>> [<c0451055>] __do_IRQ+0x94/0xef
>> [<c040678d>] do_IRQ+0x9e/0xbd
>> [<c0404a49>] common_interrupt+0x25/0x2c
>
> ouch! That is a scenario that could lead to real lockups. Fix looks good
> and necessary for v2.6.18 to me.
>

btw this entire code path is evil; the rtc_get_rtc_time() function can do really long delays
which is unsuitable for being called in interrupt context!

2006-09-20 08:55:58

by Ingo Molnar

Subject: Re: [PATCH] rtc: lockdep fix

* Arjan van de Ven <[email protected]> wrote:

> >ouch! That is a scenario that could lead to real lockups. Fix looks
> >good and necessary for v2.6.18 to me.
>
> btw this entire code path is evil; the rtc_get_rtc_time() function can
> do really long delays which is unsuitable for being called in
> interrupt context!

yeah - but i dont think it triggers that often, and a fix for that
probably isnt for v2.6.18.

Ingo