2003-03-03 04:11:31

by Zwane Mwaikambo

Subject: [PATCH][2.5][CHECKER] rtc locking

Index: linux-2.5.62-numaq/drivers/char/rtc.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.62/drivers/char/rtc.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 rtc.c
--- linux-2.5.62-numaq/drivers/char/rtc.c 18 Feb 2003 00:15:30 -0000 1.1.1.1
+++ linux-2.5.62-numaq/drivers/char/rtc.c 3 Mar 2003 03:57:09 -0000
@@ -746,13 +746,15 @@
#else
unsigned char tmp;

- spin_lock_irq(&rtc_task_lock);
+ spin_lock_irq(&rtc_lock);
+ spin_lock(&rtc_task_lock);
if (rtc_callback != task) {
- spin_unlock_irq(&rtc_task_lock);
+ spin_unlock(&rtc_task_lock);
+ spin_unlock_irq(rtc_lock);
return -ENXIO;
}
rtc_callback = NULL;
- spin_lock(&rtc_lock);
+
/* disable controls */
tmp = CMOS_READ(RTC_CONTROL);
tmp &= ~RTC_PIE;
@@ -765,8 +767,8 @@
del_timer(&rtc_irq_timer);
}
rtc_status &= ~RTC_IS_OPEN;
- spin_unlock(&rtc_lock);
- spin_unlock_irq(&rtc_task_lock);
+ spin_unlock(&rtc_task_lock);
+ spin_unlock_irq(&rtc_lock);
return 0;
#endif
}

2003-03-03 04:17:19

by William Lee Irwin III

Subject: Re: [PATCH][2.5][CHECKER] rtc locking

On Sun, Mar 02, 2003 at 11:19:57PM -0500, Zwane Mwaikambo wrote:
> Index: linux-2.5.62-numaq/drivers/char/rtc.c
[... good patch]

Do you think some kind of API safety (e.g. removing *_lock_irq() in
favor of *_lock_irqsave()) would be worthwhile for catching these kinds
of errors?



-- wli

2003-03-03 04:14:11

by Zwane Mwaikambo

Subject: Re: [PATCH][2.5][CHECKER] rtc locking

oops missing ampersand

Index: linux-2.5.62-numaq/drivers/char/rtc.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.62/drivers/char/rtc.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 rtc.c
--- linux-2.5.62-numaq/drivers/char/rtc.c 18 Feb 2003 00:15:30 -0000 1.1.1.1
+++ linux-2.5.62-numaq/drivers/char/rtc.c 3 Mar 2003 04:21:09 -0000
@@ -746,13 +746,15 @@
#else
unsigned char tmp;

- spin_lock_irq(&rtc_task_lock);
+ spin_lock_irq(&rtc_lock);
+ spin_lock(&rtc_task_lock);
if (rtc_callback != task) {
- spin_unlock_irq(&rtc_task_lock);
+ spin_unlock(&rtc_task_lock);
+ spin_unlock_irq(&rtc_lock);
return -ENXIO;
}
rtc_callback = NULL;
- spin_lock(&rtc_lock);
+
/* disable controls */
tmp = CMOS_READ(RTC_CONTROL);
tmp &= ~RTC_PIE;
@@ -765,8 +767,8 @@
del_timer(&rtc_irq_timer);
}
rtc_status &= ~RTC_IS_OPEN;
- spin_unlock(&rtc_lock);
- spin_unlock_irq(&rtc_task_lock);
+ spin_unlock(&rtc_task_lock);
+ spin_unlock_irq(&rtc_lock);
return 0;
#endif
}

--
function.linuxpower.ca

2003-03-03 04:27:01

by Zwane Mwaikambo

Subject: Re: [PATCH][2.5][CHECKER] rtc locking

On Sun, 2 Mar 2003, William Lee Irwin III wrote:

> On Sun, Mar 02, 2003 at 11:19:57PM -0500, Zwane Mwaikambo wrote:
> > Index: linux-2.5.62-numaq/drivers/char/rtc.c
> [... good patch]
>
> Do you think some kind of API safety (e.g. removing *_lock_irq() in
> favor of *_lock_irqsave()) would be worthwhile for catching these kinds
> of errors?

I think it may be worth checking *_lock_irq users period, however iirc
there is a speed argument wrt using _lock_irq since we don't save flags.

Zwane
--
function.linuxpower.ca