[PATCH] 2.4.21-pre7 ide request races

[PATCH] 2.4.21-pre7 ide request races

[Jens Axboe]

Hi,

We've had some problems with request corruption on IDE in the past, IBM
traced these to stack corruption. In various places, the IDE code does
something ala:

submission:
	struct request rq;

	...
	ide_do_drive_cmd(drive, &rq, ide_wait);

ide_end_request:
	...
	blkdev_release_request()

which works fine, as long as the stack persists for the
blkdev_release_request() call, but it may not if the task has already
exited (CPU0 may be waiting in ide_do_drive_cmd(), CPU1 gets the
completion interrupt, task is woken, and exits, CPU0 now calls
blkdev_release_request()). The result is random stack corruption (or
request list corruption, rq->q may have been scrippled!), not good.

The solution is to allocate the requests from the ordinary block pool,
which not alone fixes the problem but also is a lot cleaner IMHO. I've
added blk_get_request() which is identical to what I added in 2.5, so we
can easily carry this fix forward to the 2.5 IDE base as well.

diff -ur -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.4.21-pre7/drivers/block/ll_rw_blk.c linux-2.4.21-pre7/drivers/block/ll_rw_blk.c
--- /opt/kernel/linux-2.4.21-pre7/drivers/block/ll_rw_blk.c	2003-04-14 10:48:21.000000000 +0200
+++ linux-2.4.21-pre7/drivers/block/ll_rw_blk.c	2003-04-14 11:28:51.000000000 +0200
@@ -427,7 +427,9 @@
 	INIT_LIST_HEAD(&q->rq[READ].free);
 	INIT_LIST_HEAD(&q->rq[WRITE].free);
 	q->rq[READ].count = 0;
+	q->rq[READ].wait = &q->wait_for_requests[READ];
 	q->rq[WRITE].count = 0;
+	q->rq[WRITE].wait = &q->wait_for_requests[WRITE];
 	q->nr_requests = 0;
 
 	si_meminfo(&si);
@@ -521,6 +523,7 @@
 		rq->cmd = rw;
 		rq->special = NULL;
 		rq->q = q;
+		rq->rl = rl;
 	}
 
 	return rq;
@@ -605,6 +608,30 @@
 	return rq;
 }
 
+struct request *blk_get_request(request_queue_t *q, int rw, int gfp_mask)
+{
+	struct request *rq;
+	unsigned long flags;
+
+	BUG_ON(rw != READ && rw != WRITE);
+
+	spin_lock_irqsave(&io_request_lock, flags);
+	rq = get_request(q, rw);
+	spin_unlock_irqrestore(&io_request_lock, flags);
+
+	if (!rq && (gfp_mask & __GFP_WAIT))
+		rq = __get_request_wait(q, rw);
+
+	if (rq) {
+		rq->buffer = NULL;
+		rq->bh = rq->bhtail = NULL;
+		rq->waiting = NULL;
+		rq->special = NULL;
+	}
+
+	return rq;
+}
+
 /* RO fail safe mechanism */
 
 static long ro_bits[MAX_BLKDEV][8];
@@ -818,20 +845,21 @@
 void blkdev_release_request(struct request *req)
 {
 	request_queue_t *q = req->q;
-	int rw = req->cmd;
+	struct request_list *rl = req->rl;
 
 	req->rq_status = RQ_INACTIVE;
 	req->q = NULL;
+	req->rl = NULL;
 
 	/*
 	 * Request may not have originated from ll_rw_blk. if not,
 	 * assume it has free buffers and check waiters
 	 */
-	if (q) {
-		list_add(&req->queue, &q->rq[rw].free);
-		if (++q->rq[rw].count >= q->batch_requests &&
-				waitqueue_active(&q->wait_for_requests[rw]))
-			wake_up(&q->wait_for_requests[rw]);
+	if (rl) {
+		list_add(&req->queue, &rl->free);
+		if (++rl->count >= q->batch_requests &&
+		    waitqueue_active(rl->wait))
+			wake_up(rl->wait);
 	}
 }
 
@@ -1518,3 +1546,4 @@
 EXPORT_SYMBOL(blk_max_pfn);
 EXPORT_SYMBOL(blk_seg_merge_ok);
 EXPORT_SYMBOL(blk_nohighio);
+EXPORT_SYMBOL(blk_get_request);
diff -ur -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.4.21-pre7/drivers/ide/ide-cd.c linux-2.4.21-pre7/drivers/ide/ide-cd.c
--- /opt/kernel/linux-2.4.21-pre7/drivers/ide/ide-cd.c	2003-04-14 10:48:22.000000000 +0200
+++ linux-2.4.21-pre7/drivers/ide/ide-cd.c	2003-04-14 11:13:38.000000000 +0200
@@ -1601,7 +1601,7 @@
 int cdrom_queue_packet_command(ide_drive_t *drive, struct packet_command *pc)
 {
 	struct request_sense sense;
-	struct request req;
+	struct request *req;
 	int retries = 10;
 
 	if (pc->sense == NULL)
@@ -1609,10 +1609,11 @@
 
 	/* Start of retry loop. */
 	do {
-		ide_init_drive_cmd (&req);
-		req.cmd = PACKET_COMMAND;
-		req.buffer = (char *)pc;
-		ide_do_drive_cmd(drive, &req, ide_wait);
+		req = blk_get_request(&drive->queue, WRITE, GFP_KERNEL);
+		ide_init_drive_cmd (req);
+		req->cmd = PACKET_COMMAND;
+		req->buffer = (char *)pc;
+		ide_do_drive_cmd(drive, req, ide_wait);
 		/* FIXME: we should probably abort/retry or something 
 		 * in case of failure */
 		if (pc->stat != 0) {
@@ -2487,12 +2488,12 @@
 {
 	ide_drive_t *drive = (ide_drive_t*) cdi->handle;
 	struct request_sense sense;
-	struct request req;
+	struct request *req = blk_get_request(&drive->queue, WRITE, GFP_KERNEL);
 	int ret;
 
-	ide_init_drive_cmd (&req);
-	req.cmd = RESET_DRIVE_COMMAND;
-	ret = ide_do_drive_cmd(drive, &req, ide_wait);
+	ide_init_drive_cmd (req);
+	req->cmd = RESET_DRIVE_COMMAND;
+	ret = ide_do_drive_cmd(drive, req, ide_wait);
 
 	/*
 	 * A reset will unlock the door. If it was previously locked,
diff -ur -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.4.21-pre7/drivers/ide/ide-disk.c linux-2.4.21-pre7/drivers/ide/ide-disk.c
--- /opt/kernel/linux-2.4.21-pre7/drivers/ide/ide-disk.c	2003-04-14 10:48:22.000000000 +0200
+++ linux-2.4.21-pre7/drivers/ide/ide-disk.c	2003-04-14 11:07:24.000000000 +0200
@@ -1414,15 +1414,16 @@
  */
 static int set_multcount(ide_drive_t *drive, int arg)
 {
-	struct request rq;
+	struct request *rq;
 
 	if (drive->special.b.set_multmode)
 		return -EBUSY;
-	ide_init_drive_cmd (&rq);
-	rq.cmd = IDE_DRIVE_CMD;
+	rq = blk_get_request(&drive->queue, WRITE, GFP_KERNEL);
+	ide_init_drive_cmd (rq);
+	rq->cmd = IDE_DRIVE_CMD;
 	drive->mult_req = arg;
 	drive->special.b.set_multmode = 1;
-	(void) ide_do_drive_cmd (drive, &rq, ide_wait);
+	(void) ide_do_drive_cmd (drive, rq, ide_wait);
 	return (drive->mult_count == arg) ? 0 : -EIO;
 }
 
diff -ur -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.4.21-pre7/drivers/ide/ide-io.c linux-2.4.21-pre7/drivers/ide/ide-io.c
--- /opt/kernel/linux-2.4.21-pre7/drivers/ide/ide-io.c	2003-04-14 10:48:22.000000000 +0200
+++ linux-2.4.21-pre7/drivers/ide/ide-io.c	2003-04-14 10:53:01.000000000 +0200
@@ -1281,8 +1281,10 @@
 
 void ide_init_drive_cmd (struct request *rq)
 {
-	memset(rq, 0, sizeof(*rq));
+	INIT_LIST_HEAD(&rq->queue);
 	rq->cmd = IDE_DRIVE_CMD;
+	rq->special = rq->buffer = NULL;
+	rq->bh = rq->bhtail = NULL;
 }
 
 EXPORT_SYMBOL(ide_init_drive_cmd);
diff -ur -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.4.21-pre7/drivers/ide/ide-taskfile.c linux-2.4.21-pre7/drivers/ide/ide-taskfile.c
--- /opt/kernel/linux-2.4.21-pre7/drivers/ide/ide-taskfile.c	2003-04-14 10:48:22.000000000 +0200
+++ linux-2.4.21-pre7/drivers/ide/ide-taskfile.c	2003-04-14 11:16:15.000000000 +0200
@@ -1306,7 +1306,7 @@
  */
 void ide_init_drive_taskfile (struct request *rq)
 {
-	memset(rq, 0, sizeof(*rq));
+	ide_init_drive_cmd(rq);
 	rq->cmd = IDE_DRIVE_TASK_NO_DATA;
 }
 
@@ -1316,11 +1316,11 @@
 
 int ide_diag_taskfile (ide_drive_t *drive, ide_task_t *args, unsigned long data_size, u8 *buf)
 {
-	struct request rq;
+	struct request *rq = blk_get_request(&drive->queue, WRITE, GFP_KERNEL);
 
-	ide_init_drive_taskfile(&rq);
-	rq.cmd = IDE_DRIVE_TASKFILE;
-	rq.buffer = buf;
+	ide_init_drive_taskfile(rq);
+	rq->cmd = IDE_DRIVE_TASKFILE;
+	rq->buffer = buf;
 
 	/*
 	 * (ks) We transfer currently only whole sectors.
@@ -1330,11 +1330,11 @@
 	 */
 	if (args->command_type != IDE_DRIVE_TASK_NO_DATA) {
 		if (data_size == 0)
-			rq.current_nr_sectors = rq.nr_sectors = (args->hobRegister[IDE_NSECTOR_OFFSET_HOB] << 8) | args->tfRegister[IDE_NSECTOR_OFFSET];
-		/*	rq.hard_cur_sectors	*/
+			rq->current_nr_sectors = rq->nr_sectors = (args->hobRegister[IDE_NSECTOR_OFFSET_HOB] << 8) | args->tfRegister[IDE_NSECTOR_OFFSET];
+		/*	rq->hard_cur_sectors	*/
 		else
-			rq.current_nr_sectors = rq.nr_sectors = data_size / SECTOR_SIZE;
-		/*	rq.hard_cur_sectors	*/
+			rq->current_nr_sectors = rq->nr_sectors = data_size / SECTOR_SIZE;
+		/*	rq->hard_cur_sectors	*/
 	}
 
 	if (args->tf_out_flags.all == 0) {
@@ -1347,8 +1347,8 @@
 				(struct hd_drive_hob_hdr *) args->hobRegister);
 
 	}
-	rq.special = args;
-	return ide_do_drive_cmd(drive, &rq, ide_wait);
+	rq->special = args;
+	return ide_do_drive_cmd(drive, rq, ide_wait);
 }
 
 #else
@@ -1649,19 +1649,19 @@
 
 int ide_wait_cmd (ide_drive_t *drive, u8 cmd, u8 nsect, u8 feature, u8 sectors, u8 *buf)
 {
-	struct request rq;
+	struct request *rq = blk_get_request(&drive->queue, WRITE, GFP_KERNEL);
 	u8 buffer[4];
 
 	if (!buf)
 		buf = buffer;
 	memset(buf, 0, 4 + SECTOR_WORDS * 4 * sectors);
-	ide_init_drive_cmd(&rq);
-	rq.buffer = buf;
+	ide_init_drive_cmd(rq);
+	rq->buffer = buf;
 	*buf++ = cmd;
 	*buf++ = nsect;
 	*buf++ = feature;
 	*buf++ = sectors;
-	return ide_do_drive_cmd(drive, &rq, ide_wait);
+	return ide_do_drive_cmd(drive, rq, ide_wait);
 }
 
 EXPORT_SYMBOL(ide_wait_cmd);
@@ -1679,9 +1679,9 @@
 	ide_task_t tfargs;
 
 	if (NULL == (void *) arg) {
-		struct request rq;
-		ide_init_drive_cmd(&rq);
-		return ide_do_drive_cmd(drive, &rq, ide_wait);
+		struct request *rq = blk_get_request(&drive->queue, WRITE, GFP_KERNEL);
+		ide_init_drive_cmd(rq);
+		return ide_do_drive_cmd(drive, rq, ide_wait);
 	}
 
 	if (copy_from_user(args, (void *)arg, 4))
@@ -1732,9 +1732,9 @@
 	ide_task_t tfargs;
 
 	if (NULL == (void *) arg) {
-		struct request rq;
-		ide_init_drive_cmd(&rq);
-		return ide_do_drive_cmd(drive, &rq, ide_wait);
+		struct request *rq = blk_get_request(&drive->queue, WRITE, GFP_KERNEL);
+		ide_init_drive_cmd(rq);
+		return ide_do_drive_cmd(drive, rq, ide_wait);
 	}
 
 	if (copy_from_user(args, (void *)arg, 4))
@@ -1792,12 +1792,12 @@
 
 int ide_wait_cmd_task (ide_drive_t *drive, u8 *buf)
 {
-	struct request rq;
+	struct request *rq = blk_get_request(&drive->queue, WRITE, GFP_KERNEL);
 
-	ide_init_drive_cmd(&rq);
-	rq.cmd = IDE_DRIVE_TASK;
-	rq.buffer = buf;
-	return ide_do_drive_cmd(drive, &rq, ide_wait);
+	ide_init_drive_cmd(rq);
+	rq->cmd = IDE_DRIVE_TASK;
+	rq->buffer = buf;
+	return ide_do_drive_cmd(drive, rq, ide_wait);
 }
 
 EXPORT_SYMBOL(ide_wait_cmd_task);
diff -ur -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.4.21-pre7/drivers/ide/ide.c linux-2.4.21-pre7/drivers/ide/ide.c
--- /opt/kernel/linux-2.4.21-pre7/drivers/ide/ide.c	2003-04-14 10:48:22.000000000 +0200
+++ linux-2.4.21-pre7/drivers/ide/ide.c	2003-04-14 11:08:25.000000000 +0200
@@ -1359,16 +1359,17 @@
 
 static int set_pio_mode (ide_drive_t *drive, int arg)
 {
-	struct request rq;
+	struct request *rq;
 
 	if (!HWIF(drive)->tuneproc)
 		return -ENOSYS;
 	if (drive->special.b.set_tune)
 		return -EBUSY;
-	ide_init_drive_cmd(&rq);
+	rq = blk_get_request(&drive->queue, WRITE, GFP_KERNEL);
+	ide_init_drive_cmd(rq);
 	drive->tune_req = (u8) arg;
 	drive->special.b.set_tune = 1;
-	(void) ide_do_drive_cmd(drive, &rq, ide_wait);
+	(void) ide_do_drive_cmd(drive, rq, ide_wait);
 	return 0;
 }
 
@@ -1533,7 +1534,6 @@
 {
 	int err = 0, major, minor;
 	ide_drive_t *drive;
-	struct request rq;
 	kdev_t dev;
 	ide_settings_t *setting;
 
@@ -1560,7 +1560,6 @@
 	}
 	up(&ide_setting_sem);
 
-	ide_init_drive_cmd (&rq);
 	switch (cmd) {
 		case HDIO_GETGEO:
 		{
diff -ur -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.4.21-pre7/include/linux/blkdev.h linux-2.4.21-pre7/include/linux/blkdev.h
--- /opt/kernel/linux-2.4.21-pre7/include/linux/blkdev.h	2003-04-14 10:48:24.000000000 +0200
+++ linux-2.4.21-pre7/include/linux/blkdev.h	2003-04-14 11:26:31.000000000 +0200
@@ -15,6 +15,12 @@
 struct elevator_s;
 typedef struct elevator_s elevator_t;
 
+struct request_list {
+	unsigned int count;
+	wait_queue_head_t *wait;
+	struct list_head free;
+};
+
 /*
  * Ok, this is an expanded form so that we can use the same
  * request for paging requests.
@@ -46,6 +52,7 @@
 	struct buffer_head * bh;
 	struct buffer_head * bhtail;
 	request_queue_t *q;
+	struct request_list *rl;
 };
 
 #include <linux/elevator.h>
@@ -70,11 +77,6 @@
  */
 #define QUEUE_NR_REQUESTS	8192
 
-struct request_list {
-	unsigned int count;
-	struct list_head free;
-};
-
 struct request_queue
 {
 	/*
@@ -217,6 +219,7 @@
 extern void generic_make_request(int rw, struct buffer_head * bh);
 extern inline request_queue_t *blk_get_queue(kdev_t dev);
 extern void blkdev_release_request(struct request *);
+extern struct request *blk_get_request(request_queue_t *, int, int);
 
 /*
  * Access functions for manipulating queue properties

-- 
Jens Axboe

Re: [PATCH] 2.4.21-pre7 ide request races

[Andrew Morton]

Jens Axboe <axboe@suse.de> wrote:
>
> We've had some problems with request corruption on IDE in the past, IBM
> traced these to stack corruption. In various places, the IDE code does
> something ala:
> 
> submission:
> 	struct request rq;
> 
> 	...
> 	ide_do_drive_cmd(drive, &rq, ide_wait);
> 
> ide_end_request:
> 	...
> 	blkdev_release_request()
> 
> which works fine, as long as the stack persists for the
> blkdev_release_request() call, but it may not if the task has already
> exited (CPU0 may be waiting in ide_do_drive_cmd(), CPU1 gets the
> completion interrupt, task is woken, and exits, CPU0 now calls
> blkdev_release_request()). The result is random stack corruption (or
> request list corruption, rq->q may have been scrippled!), not good.
> 

Those locally allocated requests are foul, and the patch is a good cleanup,
but is a simpler fix more appropriate?

The bug is in end_that_request_last(), yes?

void end_that_request_last(struct request *req)
{
	if (req->waiting != NULL)
		complete(req->waiting);
	req_finished_io(req);

	blkdev_release_request(req);
}

Wouldn't it be simpler to just do:

void end_that_request_last(struct request *req)
{
	struct completion *c = req->waiting;

	req_finished_io(req);
	blkdev_release_request(req);
	if (c)
		complete(c);
}


I vaguely seem to remember being told months ago why this wasn't right ;)

Re: [PATCH] 2.4.21-pre7 ide request races

[Jens Axboe]

On Mon, Apr 14 2003, Andrew Morton wrote:
> Jens Axboe <axboe@suse.de> wrote:
> >
> > We've had some problems with request corruption on IDE in the past, IBM
> > traced these to stack corruption. In various places, the IDE code does
> > something ala:
> > 
> > submission:
> > 	struct request rq;
> > 
> > 	...
> > 	ide_do_drive_cmd(drive, &rq, ide_wait);
> > 
> > ide_end_request:
> > 	...
> > 	blkdev_release_request()
> > 
> > which works fine, as long as the stack persists for the
> > blkdev_release_request() call, but it may not if the task has already
> > exited (CPU0 may be waiting in ide_do_drive_cmd(), CPU1 gets the
> > completion interrupt, task is woken, and exits, CPU0 now calls
> > blkdev_release_request()). The result is random stack corruption (or
> > request list corruption, rq->q may have been scrippled!), not good.
> > 
> 
> Those locally allocated requests are foul, and the patch is a good cleanup,
> but is a simpler fix more appropriate?
> 
> The bug is in end_that_request_last(), yes?
> 
> void end_that_request_last(struct request *req)
> {
> 	if (req->waiting != NULL)
> 		complete(req->waiting);
> 	req_finished_io(req);
> 
> 	blkdev_release_request(req);
> }
> 
> Wouldn't it be simpler to just do:
> 
> void end_that_request_last(struct request *req)
> {
> 	struct completion *c = req->waiting;
> 
> 	req_finished_io(req);
> 	blkdev_release_request(req);
> 	if (c)
> 		complete(c);
> }
> 
> 
> I vaguely seem to remember being told months ago why this wasn't right ;)

How would that solve the problem? The request could be gone even before
end_that_request_last() is run, that is the issue.

-- 
Jens Axboe

Re: [PATCH] 2.4.21-pre7 ide request races

[Andrew Morton]

Jens Axboe <axboe@suse.de> wrote:
>
> How would that solve the problem? The request could be gone even before
> end_that_request_last() is run, that is the issue.

In that case I didn't understand your description of the bug even the tiniest
little bit.

That request is sitting in the kernel stack of some process which is sleeping
in wait_for_completion().  Hence it is safe memory until someone runs
complete() against the completion struct.

Re: [PATCH] 2.4.21-pre7 ide request races

[Jens Axboe]

On Mon, Apr 14 2003, Andrew Morton wrote:
> Jens Axboe <axboe@suse.de> wrote:
> >
> > How would that solve the problem? The request could be gone even before
> > end_that_request_last() is run, that is the issue.
> 
> In that case I didn't understand your description of the bug even the tiniest
> little bit.
> 
> That request is sitting in the kernel stack of some process which is sleeping
> in wait_for_completion().  Hence it is safe memory until someone runs
> complete() against the completion struct.

Sorry you are right, that should fix the problem as well! Your fix is
probably the better one for 2.4, less intrusive. I'll kill the stack
requests in 2.5 then.

-- 
Jens Axboe

Re: [PATCH] 2.4.21-pre7 ide request races

[Jens Axboe]

On Mon, Apr 14 2003, Jens Axboe wrote:
> On Mon, Apr 14 2003, Andrew Morton wrote:
> > Jens Axboe <axboe@suse.de> wrote:
> > >
> > > How would that solve the problem? The request could be gone even before
> > > end_that_request_last() is run, that is the issue.
> > 
> > In that case I didn't understand your description of the bug even the tiniest
> > little bit.
> > 
> > That request is sitting in the kernel stack of some process which is sleeping
> > in wait_for_completion().  Hence it is safe memory until someone runs
> > complete() against the completion struct.
> 
> Sorry you are right, that should fix the problem as well! Your fix is
> probably the better one for 2.4, less intrusive. I'll kill the stack
> requests in 2.5 then.

Marcelo,

Please apply this simpler variant. Thanks!

--- /opt/kernel/linux-2.4.21-pre7/drivers/block/ll_rw_blk.c	2003-04-14 10:48:21.000000000 +0200
+++ drivers/block/ll_rw_blk.c	2003-04-14 12:53:03.000000000 +0200
@@ -1375,11 +1403,12 @@
 
 void end_that_request_last(struct request *req)
 {
-	if (req->waiting != NULL)
-		complete(req->waiting);
-	req_finished_io(req);
+	struct completion *waiting = req->waiting;
 
+	req_finished_io(req);
 	blkdev_release_request(req);
+	if (waiting)
+		complete(waiting);
 }
 
 int __init blk_dev_init(void)

-- 
Jens Axboe

Re: [PATCH] 2.4.21-pre7 ide request races

[Mike Anderson]

Jens Axboe [axboe@suse.de] wrote:
> On Mon, Apr 14 2003, Andrew Morton wrote:
> > Jens Axboe <axboe@suse.de> wrote:
> > >
> > > How would that solve the problem? The request could be gone even before
> > > end_that_request_last() is run, that is the issue.
> > 
> > In that case I didn't understand your description of the bug even the tiniest
> > little bit.
> > 
> > That request is sitting in the kernel stack of some process which is sleeping
> > in wait_for_completion().  Hence it is safe memory until someone runs
> > complete() against the completion struct.
> 
> Sorry you are right, that should fix the problem as well! Your fix is
> probably the better one for 2.4, less intrusive. I'll kill the stack
> requests in 2.5 then.

In 2.5 will you include the 2.4 end_that_request_last fix proposed in
this thread along with removal of requests on the stack?

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [PATCH] 2.4.21-pre7 ide request races

[Jens Axboe]

On Mon, Apr 14 2003, Mike Anderson wrote:
> Jens Axboe [axboe@suse.de] wrote:
> > On Mon, Apr 14 2003, Andrew Morton wrote:
> > > Jens Axboe <axboe@suse.de> wrote:
> > > >
> > > > How would that solve the problem? The request could be gone even before
> > > > end_that_request_last() is run, that is the issue.
> > > 
> > > In that case I didn't understand your description of the bug even the tiniest
> > > little bit.
> > > 
> > > That request is sitting in the kernel stack of some process which is sleeping
> > > in wait_for_completion().  Hence it is safe memory until someone runs
> > > complete() against the completion struct.
> > 
> > Sorry you are right, that should fix the problem as well! Your fix is
> > probably the better one for 2.4, less intrusive. I'll kill the stack
> > requests in 2.5 then.
> 
> In 2.5 will you include the 2.4 end_that_request_last fix proposed in
> this thread along with removal of requests on the stack?

Yes of course. One is a good cleanup, the other prevents similar
problems from other drivers.

-- 
Jens Axboe