Commit 8c86608b authored by Jens Axboe's avatar Jens Axboe Committed by Linus Torvalds

[PATCH] cfq-iosched: fix allocation increment race #3

There is a stupid error in cfq-iosched that spews a warning on
(typically) SMP systems because cfqq->allocated[rw] goes below zero. The
error is that the increment on alloc happens outside of the queue lock.
Signed-off-by: default avatarJens Axboe <axboe@suse.de>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 5bc58b21
...@@ -1398,10 +1398,7 @@ static void cfq_put_request(request_queue_t *q, struct request *rq) ...@@ -1398,10 +1398,7 @@ static void cfq_put_request(request_queue_t *q, struct request *rq)
if (crq->io_context) if (crq->io_context)
put_io_context(crq->io_context->ioc); put_io_context(crq->io_context->ioc);
if (!cfqq->allocated[crq->is_write]) { BUG_ON(!cfqq->allocated[crq->is_write]);
WARN_ON(1);
cfqq->allocated[crq->is_write] = 1;
}
cfqq->allocated[crq->is_write]--; cfqq->allocated[crq->is_write]--;
mempool_free(crq, cfqd->crq_pool); mempool_free(crq, cfqd->crq_pool);
...@@ -1421,7 +1418,7 @@ static int cfq_set_request(request_queue_t *q, struct request *rq, int gfp_mask) ...@@ -1421,7 +1418,7 @@ static int cfq_set_request(request_queue_t *q, struct request *rq, int gfp_mask)
struct cfq_data *cfqd = q->elevator->elevator_data; struct cfq_data *cfqd = q->elevator->elevator_data;
struct cfq_io_context *cic; struct cfq_io_context *cic;
const int rw = rq_data_dir(rq); const int rw = rq_data_dir(rq);
struct cfq_queue *cfqq; struct cfq_queue *cfqq, *saved_cfqq;
struct cfq_rq *crq; struct cfq_rq *crq;
unsigned long flags; unsigned long flags;
...@@ -1439,20 +1436,30 @@ static int cfq_set_request(request_queue_t *q, struct request *rq, int gfp_mask) ...@@ -1439,20 +1436,30 @@ static int cfq_set_request(request_queue_t *q, struct request *rq, int gfp_mask)
#endif #endif
} }
repeat:
if (cfqq->allocated[rw] >= cfqd->max_queued) if (cfqq->allocated[rw] >= cfqd->max_queued)
goto out_lock; goto out_lock;
cfqq->allocated[rw]++;
spin_unlock_irqrestore(q->queue_lock, flags); spin_unlock_irqrestore(q->queue_lock, flags);
/* /*
* if hashing type has changed, the cfq_queue might change here. we * if hashing type has changed, the cfq_queue might change here.
* don't bother rechecking ->allocated since it should be a rare
* event
*/ */
saved_cfqq = cfqq;
cic = cfq_get_io_context(&cfqq, gfp_mask); cic = cfq_get_io_context(&cfqq, gfp_mask);
if (!cic) if (!cic)
goto err; goto err;
/*
* repeat allocation checks on queue change
*/
if (unlikely(saved_cfqq != cfqq)) {
spin_lock_irqsave(q->queue_lock, flags);
saved_cfqq->allocated[rw]--;
goto repeat;
}
crq = mempool_alloc(cfqd->crq_pool, gfp_mask); crq = mempool_alloc(cfqd->crq_pool, gfp_mask);
if (crq) { if (crq) {
RB_CLEAR(&crq->rb_node); RB_CLEAR(&crq->rb_node);
...@@ -1465,7 +1472,6 @@ static int cfq_set_request(request_queue_t *q, struct request *rq, int gfp_mask) ...@@ -1465,7 +1472,6 @@ static int cfq_set_request(request_queue_t *q, struct request *rq, int gfp_mask)
crq->in_flight = crq->accounted = crq->is_sync = 0; crq->in_flight = crq->accounted = crq->is_sync = 0;
crq->is_write = rw; crq->is_write = rw;
rq->elevator_private = crq; rq->elevator_private = crq;
cfqq->allocated[rw]++;
cfqq->alloc_limit[rw] = 0; cfqq->alloc_limit[rw] = 0;
return 0; return 0;
} }
...@@ -1473,6 +1479,7 @@ static int cfq_set_request(request_queue_t *q, struct request *rq, int gfp_mask) ...@@ -1473,6 +1479,7 @@ static int cfq_set_request(request_queue_t *q, struct request *rq, int gfp_mask)
put_io_context(cic->ioc); put_io_context(cic->ioc);
err: err:
spin_lock_irqsave(q->queue_lock, flags); spin_lock_irqsave(q->queue_lock, flags);
cfqq->allocated[rw]--;
cfq_put_queue(cfqq); cfq_put_queue(cfqq);
out_lock: out_lock:
spin_unlock_irqrestore(q->queue_lock, flags); spin_unlock_irqrestore(q->queue_lock, flags);
......
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