2003-02-16 00:02:07

by Andrew Morton

[permalink] [raw]
Subject: [patch] elv_former_request reversion


This morning's fix for elv_former_request() is causing oopses all over the
place in the IO scheduler.

Jens, remember that I did try that fix a while ago, and the same happened.

I believe it has exposed a new problem at the __make_request/attempt_front_merge
level: if attempt_front_merge() actually succeeds, the wrong request gets freed
up in elv_merged_request().

It may be best to back this change out until it can be fixed up for real.


diff -puN drivers/block/elevator.c~deadline-hack drivers/block/elevator.c
--- 25/drivers/block/elevator.c~deadline-hack 2003-02-15 15:56:56.000000000 -0800
+++ 25-akpm/drivers/block/elevator.c 2003-02-15 15:57:09.000000000 -0800
@@ -399,7 +399,7 @@ struct request *elv_former_request(reque
elevator_t *e = &q->elevator;

if (e->elevator_former_req_fn)
- return e->elevator_former_req_fn(q, rq);
+ return e->elevator_latter_req_fn(q, rq);

prev = rq->queuelist.prev;
if (prev != &q->queue_head && prev != &rq->queuelist)

_


2003-02-16 09:23:50

by Jens Axboe

[permalink] [raw]
Subject: Re: [patch] elv_former_request reversion

On Sat, Feb 15 2003, Andrew Morton wrote:
>
> This morning's fix for elv_former_request() is causing oopses all over the
> place in the IO scheduler.
>
> Jens, remember that I did try that fix a while ago, and the same happened.
>
> I believe it has exposed a new problem at the
> __make_request/attempt_front_merge level: if attempt_front_merge()
> actually succeeds, the wrong request gets freed up in
> elv_merged_request().
>
> It may be best to back this change out until it can be fixed up for
> real.

Yes agreed, I had forgotten about that point... Will fix.

--
Jens Axboe

2003-02-16 11:49:52

by Jens Axboe

[permalink] [raw]
Subject: Re: [patch] elv_former_request reversion

On Sun, Feb 16 2003, Jens Axboe wrote:
> On Sat, Feb 15 2003, Andrew Morton wrote:
> >
> > This morning's fix for elv_former_request() is causing oopses all over the
> > place in the IO scheduler.
> >
> > Jens, remember that I did try that fix a while ago, and the same happened.
> >
> > I believe it has exposed a new problem at the
> > __make_request/attempt_front_merge level: if attempt_front_merge()
> > actually succeeds, the wrong request gets freed up in
> > elv_merged_request().
> >
> > It may be best to back this change out until it can be fixed up for
> > real.
>
> Yes agreed, I had forgotten about that point... Will fix.

Andrew, does this work for you?

===== drivers/block/deadline-iosched.c 1.14 vs edited =====
--- 1.14/drivers/block/deadline-iosched.c Fri Feb 14 13:57:15 2003
+++ edited/drivers/block/deadline-iosched.c Sun Feb 16 12:57:35 2003
@@ -297,6 +297,9 @@
deadline_del_drq_rb(dd, drq);
}

+ if (q->last_merge == &rq->queuelist)
+ q->last_merge = NULL;
+
list_del_init(&rq->queuelist);
}

@@ -424,12 +427,7 @@
{
request_queue_t *q = drq->request->q;

- if (q->last_merge == &drq->request->queuelist)
- q->last_merge = NULL;
-
- deadline_del_drq_hash(drq);
- deadline_del_drq_rb(dd, drq);
- list_del_init(&drq->fifo);
+ deadline_remove_request(q, drq->request);
list_add_tail(&drq->request->queuelist, dd->dispatch);
}

===== drivers/block/elevator.c 1.39 vs edited =====
--- 1.39/drivers/block/elevator.c Sun Feb 16 00:57:09 2003
+++ edited/drivers/block/elevator.c Sun Feb 16 11:32:35 2003
@@ -399,7 +399,7 @@
elevator_t *e = &q->elevator;

if (e->elevator_former_req_fn)
- return e->elevator_latter_req_fn(q, rq);
+ return e->elevator_former_req_fn(q, rq);

prev = rq->queuelist.prev;
if (prev != &q->queue_head && prev != &rq->queuelist)

--
Jens Axboe

2003-02-16 12:18:50

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] elv_former_request reversion

Jens Axboe <[email protected]> wrote:
>
> Andrew, does this work for you?

Yes. I threw a bunch of nastiness at that on IDE and SCSI, 2-way and 4-way.
No problems. Thanks.

2003-02-16 12:44:17

by Anton Blanchard

[permalink] [raw]
Subject: Re: [patch] elv_former_request reversion


> Andrew, does this work for you?

Looks good on my ppc64 box too, it was hitting the BUG() when running
SDET before.

Anton