lkml.org 
[lkml]   [2002]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subjectblock 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/
\
 
 \ /
  Last update: 2005-03-22 13:23    [W:3.030 / U:1.336 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site