Commit bc7e5508 authored by Paul Clements's avatar Paul Clements Committed by Linus Torvalds

[PATCH] nbd: fix struct request race condition

Here's a patch to fix a race condition in nbd that was causing struct
request corruption (requests were being freed while still in use).  This
patch improves on the previous one, which admittedly was a bit dodgy, using
struct request's ref_count field (I should have listened to Jens in the
first place :).  This should fix all the corner cases related to struct
request usage/freeing in nbd.  My stress tests do a lot better with this
patch applied.
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 5a3d730c
...@@ -128,23 +128,11 @@ static void nbd_end_request(struct request *req) ...@@ -128,23 +128,11 @@ static void nbd_end_request(struct request *req)
{ {
int uptodate = (req->errors == 0) ? 1 : 0; int uptodate = (req->errors == 0) ? 1 : 0;
request_queue_t *q = req->q; request_queue_t *q = req->q;
struct nbd_device *lo = req->rq_disk->private_data;
unsigned long flags; unsigned long flags;
dprintk(DBG_BLKDEV, "%s: request %p: %s\n", req->rq_disk->disk_name, dprintk(DBG_BLKDEV, "%s: request %p: %s\n", req->rq_disk->disk_name,
req, uptodate? "done": "failed"); req, uptodate? "done": "failed");
spin_lock(&lo->queue_lock);
while (req->ref_count > 1) { /* still in send */
spin_unlock(&lo->queue_lock);
printk(KERN_DEBUG "%s: request %p still in use (%d), waiting\n",
lo->disk->disk_name, req, req->ref_count);
set_current_state(TASK_UNINTERRUPTIBLE);
schedule_timeout(HZ); /* wait a second */
spin_lock(&lo->queue_lock);
}
spin_unlock(&lo->queue_lock);
spin_lock_irqsave(q->queue_lock, flags); spin_lock_irqsave(q->queue_lock, flags);
if (!end_that_request_first(req, uptodate, req->nr_sectors)) { if (!end_that_request_first(req, uptodate, req->nr_sectors)) {
end_that_request_last(req); end_that_request_last(req);
...@@ -228,7 +216,7 @@ static inline int sock_send_bvec(struct socket *sock, struct bio_vec *bvec, ...@@ -228,7 +216,7 @@ static inline int sock_send_bvec(struct socket *sock, struct bio_vec *bvec,
return result; return result;
} }
void nbd_send_req(struct nbd_device *lo, struct request *req) static int nbd_send_req(struct nbd_device *lo, struct request *req)
{ {
int result, i, flags; int result, i, flags;
struct nbd_request request; struct nbd_request request;
...@@ -288,11 +276,11 @@ void nbd_send_req(struct nbd_device *lo, struct request *req) ...@@ -288,11 +276,11 @@ void nbd_send_req(struct nbd_device *lo, struct request *req)
} }
} }
up(&lo->tx_lock); up(&lo->tx_lock);
return; return 0;
error_out: error_out:
up(&lo->tx_lock); up(&lo->tx_lock);
req->errors++; return 1;
} }
static struct request *nbd_find_request(struct nbd_device *lo, char *handle) static struct request *nbd_find_request(struct nbd_device *lo, char *handle)
...@@ -477,26 +465,19 @@ static void do_nbd_request(request_queue_t * q) ...@@ -477,26 +465,19 @@ static void do_nbd_request(request_queue_t * q)
} }
list_add(&req->queuelist, &lo->queue_head); list_add(&req->queuelist, &lo->queue_head);
req->ref_count++; /* make sure req does not get freed */
spin_unlock(&lo->queue_lock); spin_unlock(&lo->queue_lock);
nbd_send_req(lo, req); if (nbd_send_req(lo, req) != 0) {
if (req->errors) {
printk(KERN_ERR "%s: Request send failed\n", printk(KERN_ERR "%s: Request send failed\n",
lo->disk->disk_name); lo->disk->disk_name);
spin_lock(&lo->queue_lock); if (nbd_find_request(lo, (char *)&req) != NULL) {
list_del_init(&req->queuelist); /* we still own req */
req->ref_count--; req->errors++;
spin_unlock(&lo->queue_lock); nbd_end_request(req);
nbd_end_request(req); } else /* we're racing with nbd_clear_que */
spin_lock_irq(q->queue_lock); printk(KERN_DEBUG "nbd: can't find req\n");
continue;
} }
spin_lock(&lo->queue_lock);
req->ref_count--;
spin_unlock(&lo->queue_lock);
spin_lock_irq(q->queue_lock); spin_lock_irq(q->queue_lock);
continue; continue;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment