From: Nick Piggin <piggin@cyberone.com.au>
This goes on top of previous. It fixes a leak (search for "no need") - I'm
not sure what was going on there, I think the semantics for
elv_completed_request function might have changed. I surely put it there
for a reason :P
Anyway, it also changes a lot of stuff from:
if (x)
blah
to:
if (!x) {
WARN
return;
}
blah
And it changes put_as_io_context so it no longer NULLs the target and
catches double frees (of which there don't appear to be any).
Adds a check to be sure nr_as_io_contexts is > 0. Don't have a check for
the upper bound unfortunately. A decent invariant would be something like:
nr_as_io_contexts <= total-reqs-for-all-queues + nr_threads. Which could
be done quite easily.
But anyway, it seems to not leak anymore. Please apply. I may have been a
bit eager in my checks, but they are only WARNs...
drivers/block/as-iosched.c | 69 +++++++++++++++++++++++++++++----------------
1 files changed, 45 insertions(+), 24 deletions(-)
diff -puN drivers/block/as-iosched.c~as-double-free-and-debug drivers/block/as-iosched.c
--- 25/drivers/block/as-iosched.c~as-double-free-and-debug 2003-06-11 19:49:20.000000000 -0700
+++ 25-akpm/drivers/block/as-iosched.c 2003-06-11 19:49:20.000000000 -0700
@@ -214,8 +214,9 @@ static void put_as_io_context(struct as_
return;
BUG_ON(atomic_read(&aic->refcount) == 0);
- *paic = NULL;
+
if (atomic_dec_and_test(&aic->refcount)) {
+ WARN_ON(atomic_read(&nr_as_io_requests) == 0);
atomic_dec(&nr_as_io_requests);
kfree(aic);
}
@@ -354,7 +355,12 @@ static inline void as_hot_arq_hash(struc
struct request *rq = arq->request;
struct list_head *head = &ad->hash[AS_HASH_FN(rq_hash_key(rq))];
- if (ON_HASH(arq) && arq->hash.prev != head) {
+ if (!ON_HASH(arq)) {
+ WARN_ON(1);
+ return;
+ }
+
+ if (arq->hash.prev != head) {
list_del(&arq->hash);
list_add(&arq->hash, head);
}
@@ -462,10 +468,13 @@ static void as_add_arq_rb(struct as_data
static inline void as_del_arq_rb(struct as_data *ad, struct as_rq *arq)
{
- if (ON_RB(&arq->rb_node)) {
- rb_erase(&arq->rb_node, ARQ_RB_ROOT(ad, arq));
- RB_CLEAR(&arq->rb_node);
+ if (!ON_RB(&arq->rb_node)) {
+ WARN_ON(1);
+ return;
}
+
+ rb_erase(&arq->rb_node, ARQ_RB_ROOT(ad, arq));
+ RB_CLEAR(&arq->rb_node);
}
static struct request *
@@ -1061,17 +1070,19 @@ static void as_remove_queued_request(req
static void as_remove_dispatched_request(request_queue_t *q, struct request *rq)
{
struct as_rq *arq = RQ_DATA(rq);
+ struct as_io_context *aic;
- if (arq) {
- struct as_io_context *aic;
+ if (!arq) {
+ WARN_ON(1);
+ return;
+ }
- WARN_ON(arq->state != AS_RQ_DISPATCHED);
- WARN_ON(ON_RB(&arq->rb_node));
- aic = arq->as_io_context;
- if (aic) {
- WARN_ON(!atomic_read(&aic->nr_dispatched));
- atomic_dec(&aic->nr_dispatched);
- }
+ WARN_ON(arq->state != AS_RQ_DISPATCHED);
+ WARN_ON(ON_RB(&arq->rb_node));
+ aic = arq->as_io_context;
+ if (aic) {
+ WARN_ON(!atomic_read(&aic->nr_dispatched));
+ atomic_dec(&aic->nr_dispatched);
}
}
/*
@@ -1086,12 +1097,15 @@ static void as_remove_request(request_qu
if (unlikely(!blk_fs_request(rq)))
return;
- if (arq) {
- if (ON_RB(&arq->rb_node))
- as_remove_queued_request(q, rq);
- else
- as_remove_dispatched_request(q, rq);
+ if (!arq) {
+ WARN_ON(1);
+ return;
}
+
+ if (ON_RB(&arq->rb_node))
+ as_remove_queued_request(q, rq);
+ else
+ as_remove_dispatched_request(q, rq);
}
/*
@@ -1165,7 +1179,11 @@ static void as_move_to_dispatch(struct a
/* In case we have to anticipate after this */
copy_as_io_context(&ad->as_io_context, &arq->as_io_context);
} else {
- put_as_io_context(&ad->as_io_context);
+ if (ad->as_io_context) {
+ put_as_io_context(&ad->as_io_context);
+ ad->as_io_context = NULL;
+ }
+
if (ad->current_write_count != 0)
ad->current_write_count--;
}
@@ -1563,7 +1581,7 @@ as_merged_requests(request_queue_t *q, s
* kill knowledge of next, this one is a goner
*/
as_remove_queued_request(q, next);
-/* put_as_io_context(&anext->as_io_context); no need to do this because as_completed_request takes care of it */
+ put_as_io_context(&anext->as_io_context);
}
/*
@@ -1591,10 +1609,13 @@ static void as_put_request(request_queue
struct as_data *ad = q->elevator.elevator_data;
struct as_rq *arq = RQ_DATA(rq);
- if (arq) {
- mempool_free(arq, ad->arq_pool);
- rq->elevator_private = NULL;
+ if (!arq) {
+ WARN_ON(1);
+ return;
}
+
+ mempool_free(arq, ad->arq_pool);
+ rq->elevator_private = NULL;
}
static int as_set_request(request_queue_t *q, struct request *rq, int gfp_mask)
_