Commit 6f04a530 authored by Jens Axboe's avatar Jens Axboe

[PATCH] request references and list deletion/insertion checking

o Always use list_del_init() on request queuelist, this allows us to
  sanity check the integrity of the request on insertion and removal.
  So we can complain loudly instead of silently corrupting memory.

o Add references to requests. This is cheap, since we dont have to use
  an atomic variable for it (all puts are inside queue lock). We've had
  a bug in IDE for years where we want to inspect request state after
  io completion, but this is not possible to do race free right now.
  REQ_BLOCK_PC and sgio will need this too, for checking io residual
  etc. This is not just a theoretical race, I've seen it happen.
parent c5868eb7
...@@ -565,7 +565,7 @@ void blk_queue_end_tag(request_queue_t *q, struct request *rq) ...@@ -565,7 +565,7 @@ void blk_queue_end_tag(request_queue_t *q, struct request *rq)
return; return;
} }
list_del(&rq->queuelist); list_del_init(&rq->queuelist);
rq->flags &= ~REQ_QUEUED; rq->flags &= ~REQ_QUEUED;
rq->tag = -1; rq->tag = -1;
...@@ -649,7 +649,7 @@ void blk_queue_invalidate_tags(request_queue_t *q) ...@@ -649,7 +649,7 @@ void blk_queue_invalidate_tags(request_queue_t *q)
if (rq->tag == -1) { if (rq->tag == -1) {
printk("bad tag found on list\n"); printk("bad tag found on list\n");
list_del(&rq->queuelist); list_del_init(&rq->queuelist);
rq->flags &= ~REQ_QUEUED; rq->flags &= ~REQ_QUEUED;
} else } else
blk_queue_end_tag(q, rq); blk_queue_end_tag(q, rq);
...@@ -1132,7 +1132,7 @@ static int __blk_cleanup_queue(struct request_list *list) ...@@ -1132,7 +1132,7 @@ static int __blk_cleanup_queue(struct request_list *list)
while (!list_empty(head)) { while (!list_empty(head)) {
rq = list_entry(head->next, struct request, queuelist); rq = list_entry(head->next, struct request, queuelist);
list_del(&rq->queuelist); list_del_init(&rq->queuelist);
kmem_cache_free(request_cachep, rq); kmem_cache_free(request_cachep, rq);
i++; i++;
} }
...@@ -1292,13 +1292,20 @@ static struct request *get_request(request_queue_t *q, int rw) ...@@ -1292,13 +1292,20 @@ static struct request *get_request(request_queue_t *q, int rw)
if (!list_empty(&rl->free)) { if (!list_empty(&rl->free)) {
rq = blkdev_free_rq(&rl->free); rq = blkdev_free_rq(&rl->free);
list_del(&rq->queuelist); list_del_init(&rq->queuelist);
rq->ref_count = 1;
rl->count--; rl->count--;
if (rl->count < queue_congestion_on_threshold()) if (rl->count < queue_congestion_on_threshold())
set_queue_congested(q, rw); set_queue_congested(q, rw);
rq->flags = 0; rq->flags = 0;
rq->rq_status = RQ_ACTIVE; rq->rq_status = RQ_ACTIVE;
rq->errors = 0;
rq->special = NULL; rq->special = NULL;
rq->buffer = NULL;
rq->data = NULL;
rq->sense = NULL;
rq->waiting = NULL;
rq->bio = rq->biotail = NULL;
rq->q = q; rq->q = q;
rq->rl = rl; rq->rl = rl;
} }
...@@ -1497,13 +1504,14 @@ static inline void add_request(request_queue_t * q, struct request * req, ...@@ -1497,13 +1504,14 @@ static inline void add_request(request_queue_t * q, struct request * req,
__elv_add_request_pos(q, req, insert_here); __elv_add_request_pos(q, req, insert_here);
} }
/* void __blk_put_request(request_queue_t *q, struct request *req)
* Must be called with queue lock held and interrupts disabled
*/
void blk_put_request(struct request *req)
{ {
struct request_list *rl = req->rl; struct request_list *rl = req->rl;
request_queue_t *q = req->q;
if (unlikely(--req->ref_count))
return;
if (unlikely(!q))
return;
req->rq_status = RQ_INACTIVE; req->rq_status = RQ_INACTIVE;
req->q = NULL; req->q = NULL;
...@@ -1516,6 +1524,8 @@ void blk_put_request(struct request *req) ...@@ -1516,6 +1524,8 @@ void blk_put_request(struct request *req)
if (rl) { if (rl) {
int rw = 0; int rw = 0;
BUG_ON(!list_empty(&req->queuelist));
list_add(&req->queuelist, &rl->free); list_add(&req->queuelist, &rl->free);
if (rl == &q->rq[WRITE]) if (rl == &q->rq[WRITE])
...@@ -1533,6 +1543,23 @@ void blk_put_request(struct request *req) ...@@ -1533,6 +1543,23 @@ void blk_put_request(struct request *req)
} }
} }
void blk_put_request(struct request *req)
{
request_queue_t *q = req->q;
/*
* if req->q isn't set, this request didnt originate from the
* block layer, so it's safe to just disregard it
*/
if (q) {
unsigned long flags;
spin_lock_irqsave(q->queue_lock, flags);
__blk_put_request(q, req);
spin_unlock_irqrestore(q->queue_lock, flags);
}
}
/** /**
* blk_congestion_wait - wait for a queue to become uncongested * blk_congestion_wait - wait for a queue to become uncongested
* @rw: READ or WRITE * @rw: READ or WRITE
...@@ -1591,7 +1618,7 @@ static void attempt_merge(request_queue_t *q, struct request *req, ...@@ -1591,7 +1618,7 @@ static void attempt_merge(request_queue_t *q, struct request *req,
elv_merge_requests(q, req, next); elv_merge_requests(q, req, next);
blkdev_dequeue_request(next); blkdev_dequeue_request(next);
blk_put_request(next); __blk_put_request(q, next);
} }
} }
...@@ -1784,7 +1811,7 @@ static int __make_request(request_queue_t *q, struct bio *bio) ...@@ -1784,7 +1811,7 @@ static int __make_request(request_queue_t *q, struct bio *bio)
add_request(q, req, insert_here); add_request(q, req, insert_here);
out: out:
if (freereq) if (freereq)
blk_put_request(freereq); __blk_put_request(q, freereq);
spin_unlock_irq(q->queue_lock); spin_unlock_irq(q->queue_lock);
return 0; return 0;
...@@ -2069,7 +2096,7 @@ void end_that_request_last(struct request *req) ...@@ -2069,7 +2096,7 @@ void end_that_request_last(struct request *req)
if (req->waiting) if (req->waiting)
complete(req->waiting); complete(req->waiting);
blk_put_request(req); __blk_put_request(req);
} }
int __init blk_dev_init(void) int __init blk_dev_init(void)
......
...@@ -44,7 +44,9 @@ struct request *elv_next_request(request_queue_t *q); ...@@ -44,7 +44,9 @@ struct request *elv_next_request(request_queue_t *q);
static inline void blkdev_dequeue_request(struct request *req) static inline void blkdev_dequeue_request(struct request *req)
{ {
list_del(&req->queuelist); BUG_ON(list_empty(&req->queuelist));
list_del_init(&req->queuelist);
if (req->q) if (req->q)
elv_remove_request(req->q, req); elv_remove_request(req->q, req);
......
...@@ -26,6 +26,8 @@ struct request { ...@@ -26,6 +26,8 @@ struct request {
struct list_head queuelist; /* looking for ->queue? you must _not_ struct list_head queuelist; /* looking for ->queue? you must _not_
* access it directly, use * access it directly, use
* blkdev_dequeue_request! */ * blkdev_dequeue_request! */
int ref_count;
void *elevator_private; void *elevator_private;
unsigned char cmd[16]; unsigned char cmd[16];
......
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