Commit d6de8be7 authored by Jens Axboe's avatar Jens Axboe

cfq-iosched: fix RCU problem in cfq_cic_lookup()

cfq_cic_lookup() needs to properly protect ioc->ioc_data before
dereferencing it and also exclude updaters of ioc->ioc_data as well.

Also add a number of comments documenting why the existing RCU usage
is OK.

Thanks a lot to "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> for
review and comments!
Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
parent 64565911
...@@ -1142,6 +1142,9 @@ static void cfq_put_queue(struct cfq_queue *cfqq) ...@@ -1142,6 +1142,9 @@ static void cfq_put_queue(struct cfq_queue *cfqq)
kmem_cache_free(cfq_pool, cfqq); kmem_cache_free(cfq_pool, cfqq);
} }
/*
* Must always be called with the rcu_read_lock() held
*/
static void static void
__call_for_each_cic(struct io_context *ioc, __call_for_each_cic(struct io_context *ioc,
void (*func)(struct io_context *, struct cfq_io_context *)) void (*func)(struct io_context *, struct cfq_io_context *))
...@@ -1197,6 +1200,11 @@ static void cic_free_func(struct io_context *ioc, struct cfq_io_context *cic) ...@@ -1197,6 +1200,11 @@ static void cic_free_func(struct io_context *ioc, struct cfq_io_context *cic)
cfq_cic_free(cic); cfq_cic_free(cic);
} }
/*
* Must be called with rcu_read_lock() held or preemption otherwise disabled.
* Only two callers of this - ->dtor() which is called with the rcu_read_lock(),
* and ->trim() which is called with the task lock held
*/
static void cfq_free_io_context(struct io_context *ioc) static void cfq_free_io_context(struct io_context *ioc)
{ {
/* /*
...@@ -1502,20 +1510,24 @@ static struct cfq_io_context * ...@@ -1502,20 +1510,24 @@ static struct cfq_io_context *
cfq_cic_lookup(struct cfq_data *cfqd, struct io_context *ioc) cfq_cic_lookup(struct cfq_data *cfqd, struct io_context *ioc)
{ {
struct cfq_io_context *cic; struct cfq_io_context *cic;
unsigned long flags;
void *k; void *k;
if (unlikely(!ioc)) if (unlikely(!ioc))
return NULL; return NULL;
rcu_read_lock();
/* /*
* we maintain a last-hit cache, to avoid browsing over the tree * we maintain a last-hit cache, to avoid browsing over the tree
*/ */
cic = rcu_dereference(ioc->ioc_data); cic = rcu_dereference(ioc->ioc_data);
if (cic && cic->key == cfqd) if (cic && cic->key == cfqd) {
rcu_read_unlock();
return cic; return cic;
}
do { do {
rcu_read_lock();
cic = radix_tree_lookup(&ioc->radix_root, (unsigned long) cfqd); cic = radix_tree_lookup(&ioc->radix_root, (unsigned long) cfqd);
rcu_read_unlock(); rcu_read_unlock();
if (!cic) if (!cic)
...@@ -1524,10 +1536,13 @@ cfq_cic_lookup(struct cfq_data *cfqd, struct io_context *ioc) ...@@ -1524,10 +1536,13 @@ cfq_cic_lookup(struct cfq_data *cfqd, struct io_context *ioc)
k = cic->key; k = cic->key;
if (unlikely(!k)) { if (unlikely(!k)) {
cfq_drop_dead_cic(cfqd, ioc, cic); cfq_drop_dead_cic(cfqd, ioc, cic);
rcu_read_lock();
continue; continue;
} }
spin_lock_irqsave(&ioc->lock, flags);
rcu_assign_pointer(ioc->ioc_data, cic); rcu_assign_pointer(ioc->ioc_data, cic);
spin_unlock_irqrestore(&ioc->lock, flags);
break; break;
} while (1); } while (1);
...@@ -2134,6 +2149,10 @@ static void *cfq_init_queue(struct request_queue *q) ...@@ -2134,6 +2149,10 @@ static void *cfq_init_queue(struct request_queue *q)
static void cfq_slab_kill(void) static void cfq_slab_kill(void)
{ {
/*
* Caller already ensured that pending RCU callbacks are completed,
* so we should have no busy allocations at this point.
*/
if (cfq_pool) if (cfq_pool)
kmem_cache_destroy(cfq_pool); kmem_cache_destroy(cfq_pool);
if (cfq_ioc_pool) if (cfq_ioc_pool)
...@@ -2292,6 +2311,11 @@ static void __exit cfq_exit(void) ...@@ -2292,6 +2311,11 @@ static void __exit cfq_exit(void)
ioc_gone = &all_gone; ioc_gone = &all_gone;
/* ioc_gone's update must be visible before reading ioc_count */ /* ioc_gone's update must be visible before reading ioc_count */
smp_wmb(); smp_wmb();
/*
* this also protects us from entering cfq_slab_kill() with
* pending RCU callbacks
*/
if (elv_ioc_count_read(ioc_count)) if (elv_ioc_count_read(ioc_count))
wait_for_completion(ioc_gone); wait_for_completion(ioc_gone);
cfq_slab_kill(); cfq_slab_kill();
......
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