Re: [PATCH] 2.4.21-pre7 ide request races

From: Jens Axboe (axboe@suse.de)
Date: Mon Apr 14 2003 - 05:17:47 EST


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/



This archive was generated by hypermail 2b29 : Tue Apr 15 2003 - 22:00:30 EST