lkml.org 
[lkml]   [2007]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] Correctly prevent IDE timer expiry function to run if request was already handled
It is possible for the timer expiry function to run even though the
request has already been handled: ide_timer_expiry() only checks that
the handler is not NULL, but it is possible that we have handled a
request (thus clearing the handler) and then started a new request
(thus starting the timer again, and setting a handler).

A simple way to exhibit this is to set the DMA timeout to 1 jiffy and
run dd: The kernel will panic after a few minutes because
ide_timer_expiry() tries to add a timer when it's already active.

To fix this, we simply add a request generation count that gets
incremented at every interrupt, and check in ide_timer_expiry() that
we have not already handled a new interrupt before running the expiry
function.

Signed-off-by: Suleiman Souhlal <suleiman@google.com>

---
drivers/ide/ide-io.c | 6 +++++-
drivers/ide/ide-iops.c | 2 ++
include/linux/ide.h | 2 ++
3 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 0e02800..8670112 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -1226,6 +1226,7 @@ #if 1
#endif
/* so that ide_timer_expiry knows what to do */
hwgroup->sleeping = 1;
+ hwgroup->req_gen_timer = hwgroup->req_gen;
mod_timer(&hwgroup->timer, sleep);
/* we purposely leave hwgroup->busy==1
* while sleeping */
@@ -1411,7 +1412,8 @@ void ide_timer_expiry (unsigned long dat

spin_lock_irqsave(&ide_lock, flags);

- if ((handler = hwgroup->handler) == NULL) {
+ if (((handler = hwgroup->handler) == NULL) ||
+ (hwgroup->req_gen != hwgroup->req_gen_timer)) {
/*
* Either a marginal timeout occurred
* (got the interrupt just as timer expired),
@@ -1439,6 +1441,7 @@ void ide_timer_expiry (unsigned long dat
if ((wait = expiry(drive)) > 0) {
/* reset timer */
hwgroup->timer.expires = jiffies + wait;
+ hwgroup->req_gen_timer = hwgroup->req_gen;
add_timer(&hwgroup->timer);
spin_unlock_irqrestore(&ide_lock, flags);
return;
@@ -1653,6 +1656,7 @@ #endif /* CONFIG_BLK_DEV_IDEPCI */
printk(KERN_ERR "%s: ide_intr: hwgroup->busy was 0 ??\n", drive->name);
}
hwgroup->handler = NULL;
+ hwgroup->req_gen++;
del_timer(&hwgroup->timer);
spin_unlock(&ide_lock);

diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index 1ee53a5..3caa176 100644
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -889,6 +889,7 @@ static void __ide_set_handler (ide_drive
hwgroup->handler = handler;
hwgroup->expiry = expiry;
hwgroup->timer.expires = jiffies + timeout;
+ hwgroup->req_gen_timer = hwgroup->req_gen;
add_timer(&hwgroup->timer);
}

@@ -929,6 +930,7 @@ void ide_execute_command(ide_drive_t *dr
hwgroup->handler = handler;
hwgroup->expiry = expiry;
hwgroup->timer.expires = jiffies + timeout;
+ hwgroup->req_gen_timer = hwgroup->req_gen;
add_timer(&hwgroup->timer);
hwif->OUTBSYNC(drive, cmd, IDE_COMMAND_REG);
/* Drive takes 400nS to respond, we must avoid the IRQ being
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 58564a1..d3bbc71 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -861,6 +861,8 @@ typedef struct hwgroup_s {
int (*expiry)(ide_drive_t *);
/* ide_system_bus_speed */
int pio_clock;
+ int req_gen;
+ int req_gen_timer;

unsigned char cmd_buf[4];
} ide_hwgroup_t;
-
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: 2007-04-06 20:01    [W:0.073 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site