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 ;)
-
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/
This archive was generated by hypermail 2b29 : Tue Apr 15 2003 - 22:00:30 EST