Messages in this thread Patch in this message | | | Date | Tue, 15 Jan 2002 17:50:51 -0800 | From | Andrew Morton <> | Subject | block completion races |
| |
void end_that_request_last(struct request *req) { if (req->waiting != NULL) complete(req->waiting);
blkdev_release_request(req); }
I think a bug. Sometimes (eg, cdrom_queue_packet_command()) the request is allocated on a task's kernel stack. As soon as we call complete(), that task can wake and release the request while blkdev_release_request() is diddling it on this CPU.
Do you see any problem with releasing the request before running complete()?. Also I think it's best to uninline blkdev_release_request(). It's 104 bytes long, and we have four copies of it in ll_rw_blk.c. A patch is here.
Also, there is this code in ide_do_drive_cmd():
if (action == ide_wait) { wait_for_completion(&wait); /* wait for it to be serviced */ return rq->errors ? -EIO : 0; /* return -EIO if errors */ }
Is it safe to use `rq' here? It has just been recycled in end_that_request_last() and we don't own it any more.
I think the simplest approach to this one is to make the error code a part of the completion structure, so:
struct blkdev_completion { struct completion completion; int errcode; };
If you agree, I'll do the patch.
--- linux-2.4.18-pre4/drivers/block/ll_rw_blk.c Tue Jan 15 15:08:24 2002 +++ linux-akpm/drivers/block/ll_rw_blk.c Tue Jan 15 17:39:22 2002 @@ -546,7 +546,7 @@ static inline void add_request(request_q /* * Must be called with io_request_lock held and interrupts disabled */ -inline void blkdev_release_request(struct request *req) +void blkdev_release_request(struct request *req) { request_queue_t *q = req->q; int rw = req->cmd; @@ -1084,10 +1084,11 @@ int end_that_request_first (struct reque void end_that_request_last(struct request *req) { - if (req->waiting != NULL) - complete(req->waiting); + struct completion *waiting = req->waiting; blkdev_release_request(req); + if (waiting != NULL) + complete(waiting); } #define MB(kb) ((kb) << 10) - 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/
| |