Messages in this thread | | | Date | Mon, 14 Apr 2003 12:17:47 +0200 | From | Jens Axboe <> | Subject | Re: [PATCH] 2.4.21-pre7 ide request races |
| |
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
- 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/
| |