[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20080103170643.2fb89f1a@lxorguk.ukuu.org.uk>
Date: Thu, 3 Jan 2008 17:06:43 +0000
From: Alan Cox <alan@...rguk.ukuu.org.uk>
To: linux-kernel@...r.kernel.org, akpm@...l.org, wim@...ana.be
Subject: [PATCH] wdt: Fix locking
The audit of _p usage shows various drivers assume inb_p is somehow
atomic. Of course it isn't and the delay can be split from the I/O cycle
causing a timing violation on chips that matter (eg this one)
With the proposed use of udelay() for some _p delays this will cease to
be a mostly theoretical bug (as the delay stall is unsplittable) and
wants fixing.
Lots of other drivers need fixing this way too.
Signed-off-by: Alan Cox <alan@...hat.com>
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.24-rc6-mm1/drivers/watchdog/wdt.c linux-2.6.24-rc6-mm1/drivers/watchdog/wdt.c
--- linux.vanilla-2.6.24-rc6-mm1/drivers/watchdog/wdt.c 2008-01-02 16:04:06.000000000 +0000
+++ linux-2.6.24-rc6-mm1/drivers/watchdog/wdt.c 2008-01-02 20:28:26.000000000 +0000
@@ -70,6 +70,8 @@
static int io=0x240;
static int irq=11;
+static DEFINE_SPINLOCK(wdt_lock);
+
module_param(io, int, 0);
MODULE_PARM_DESC(io, "WDT io port (default=0x240)");
module_param(irq, int, 0);
@@ -109,6 +111,8 @@
static int wdt_start(void)
{
+ unsigned long flags;
+ spin_lock_irqsave(&wdt_lock, flags);
inb_p(WDT_DC); /* Disable watchdog */
wdt_ctr_mode(0,3); /* Program CTR0 for Mode 3: Square Wave Generator */
wdt_ctr_mode(1,2); /* Program CTR1 for Mode 2: Rate Generator */
@@ -117,6 +121,7 @@
wdt_ctr_load(1,wd_heartbeat); /* Heartbeat */
wdt_ctr_load(2,65535); /* Length of reset pulse */
outb_p(0, WDT_DC); /* Enable watchdog */
+ spin_unlock_irqrestore(&wdt_lock, flags);
return 0;
}
@@ -128,9 +133,12 @@
static int wdt_stop (void)
{
+ unsigned long flags;
+ spin_lock_irqsave(&wdt_lock, flags);
/* Turn the card off */
inb_p(WDT_DC); /* Disable watchdog */
wdt_ctr_load(2,0); /* 0 length reset pulses now */
+ spin_unlock_irqrestore(&wdt_lock, flags);
return 0;
}
@@ -143,11 +151,14 @@
static int wdt_ping(void)
{
+ unsigned long flags;
+ spin_lock_irqsave(&wdt_lock, flags);
/* Write a watchdog value */
inb_p(WDT_DC); /* Disable watchdog */
wdt_ctr_mode(1,2); /* Re-Program CTR1 for Mode 2: Rate Generator */
wdt_ctr_load(1,wd_heartbeat); /* Heartbeat */
outb_p(0, WDT_DC); /* Enable watchdog */
+ spin_unlock_irqrestore(&wdt_lock, flags);
return 0;
}
@@ -182,7 +193,12 @@
static int wdt_get_status(int *status)
{
- unsigned char new_status=inb_p(WDT_SR);
+ unsigned char new_status;
+ unsigned long flags;
+
+ spin_lock_irqsave(&wdt_lock, flags);
+ new_status = inb_p(WDT_SR);
+ spin_unlock_irqrestore(&wdt_lock, flags);
*status=0;
if (new_status & WDC_SR_ISOI0)
@@ -214,8 +230,12 @@
static int wdt_get_temperature(int *temperature)
{
- unsigned short c=inb_p(WDT_RT);
+ unsigned short c;
+ unsigned long flags;
+ spin_lock_irqsave(&wdt_lock, flags);
+ c=inb_p(WDT_RT);
+ spin_unlock_irqrestore(&wdt_lock, flags);
*temperature = (c * 11 / 15) + 7;
return 0;
}
@@ -237,7 +257,10 @@
* Read the status register see what is up and
* then printk it.
*/
- unsigned char status=inb_p(WDT_SR);
+ unsigned char status;
+
+ spin_lock(&wdt_lock);
+ status = inb_p(WDT_SR);
printk(KERN_CRIT "WDT status %d\n", status);
@@ -265,6 +288,7 @@
printk(KERN_CRIT "Reset in 5ms.\n");
#endif
}
+ spin_unlock(&wdt_lock);
return IRQ_HANDLED;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists