From 42ec57a8f68311bbbf4ff96a5d33c8a2e90b9d05 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 14 Dec 2011 00:33:37 +0100 Subject: block: misc ioc cleanups * int return from put_io_context() wasn't used by anybody. Make it return void like other put functions and docbook-fy the function comment. * Reorder dummy declarations for !CONFIG_BLOCK case a bit. * Make alloc_ioc_context() use __GFP_ZERO allocation, take init out of if block and drop 0'ing. * Docbook-fy current_io_context() comment. This patch doesn't introduce any functional change. Signed-off-by: Tejun Heo Signed-off-by: Jens Axboe --- include/linux/iocontext.h | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'include/linux/iocontext.h') diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h index 5037a0ad2312..8a6ecb66346f 100644 --- a/include/linux/iocontext.h +++ b/include/linux/iocontext.h @@ -76,20 +76,14 @@ static inline struct io_context *ioc_task_link(struct io_context *ioc) struct task_struct; #ifdef CONFIG_BLOCK -int put_io_context(struct io_context *ioc); +void put_io_context(struct io_context *ioc); void exit_io_context(struct task_struct *task); struct io_context *get_io_context(gfp_t gfp_flags, int node); struct io_context *alloc_io_context(gfp_t gfp_flags, int node); #else -static inline void exit_io_context(struct task_struct *task) -{ -} - struct io_context; -static inline int put_io_context(struct io_context *ioc) -{ - return 1; -} +static inline void put_io_context(struct io_context *ioc) { } +static inline void exit_io_context(struct task_struct *task) { } #endif #endif -- cgit v1.2.3 From 6e736be7f282fff705db7c34a15313281b372a76 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 14 Dec 2011 00:33:38 +0100 Subject: block: make ioc get/put interface more conventional and fix race on alloction Ignoring copy_io() during fork, io_context can be allocated from two places - current_io_context() and set_task_ioprio(). The former is always called from local task while the latter can be called from different task. The synchornization between them are peculiar and dubious. * current_io_context() doesn't grab task_lock() and assumes that if it saw %NULL ->io_context, it would stay that way until allocation and assignment is complete. It has smp_wmb() between alloc/init and assignment. * set_task_ioprio() grabs task_lock() for assignment and does smp_read_barrier_depends() between "ioc = task->io_context" and "if (ioc)". Unfortunately, this doesn't achieve anything - the latter is not a dependent load of the former. ie, if ioc itself were being dereferenced "ioc->xxx", it would mean something (not sure what tho) but as the code currently stands, the dependent read barrier is noop. As only one of the the two test-assignment sequences is task_lock() protected, the task_lock() can't do much about race between the two. Nothing prevents current_io_context() and set_task_ioprio() allocating its own ioc for the same task and overwriting the other's. Also, set_task_ioprio() can race with exiting task and create a new ioc after exit_io_context() is finished. ioc get/put doesn't have any reason to be complex. The only hot path is accessing the existing ioc of %current, which is simple to achieve given that ->io_context is never destroyed as long as the task is alive. All other paths can happily go through task_lock() like all other task sub structures without impacting anything. This patch updates ioc get/put so that it becomes more conventional. * alloc_io_context() is replaced with get_task_io_context(). This is the only interface which can acquire access to ioc of another task. On return, the caller has an explicit reference to the object which should be put using put_io_context() afterwards. * The functionality of current_io_context() remains the same but when creating a new ioc, it shares the code path with get_task_io_context() and always goes through task_lock(). * get_io_context() now means incrementing ref on an ioc which the caller already has access to (be that an explicit refcnt or implicit %current one). * PF_EXITING inhibits creation of new io_context and once exit_io_context() is finished, it's guaranteed that both ioc acquisition functions return %NULL. * All users are updated. Most are trivial but smp_read_barrier_depends() removal from cfq_get_io_context() needs a bit of explanation. I suppose the original intention was to ensure ioc->ioprio is visible when set_task_ioprio() allocates new io_context and installs it; however, this wouldn't have worked because set_task_ioprio() doesn't have wmb between init and install. There are other problems with this which will be fixed in another patch. * While at it, use NUMA_NO_NODE instead of -1 for wildcard node specification. -v2: Vivek spotted contamination from debug patch. Removed. Signed-off-by: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 9 +++-- block/blk-ioc.c | 99 +++++++++++++++++++++++++++++++---------------- block/blk.h | 1 + block/cfq-iosched.c | 18 ++++----- fs/ioprio.c | 21 ++-------- include/linux/iocontext.h | 4 +- kernel/fork.c | 8 ++-- 7 files changed, 91 insertions(+), 69 deletions(-) (limited to 'include/linux/iocontext.h') diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 8f630cec906e..4b001dcd85b0 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -1645,11 +1645,12 @@ static void blkiocg_attach_task(struct cgroup *cgrp, struct task_struct *tsk) { struct io_context *ioc; - task_lock(tsk); - ioc = tsk->io_context; - if (ioc) + /* we don't lose anything even if ioc allocation fails */ + ioc = get_task_io_context(tsk, GFP_ATOMIC, NUMA_NO_NODE); + if (ioc) { ioc->cgroup_changed = 1; - task_unlock(tsk); + put_io_context(ioc); + } } void blkio_policy_register(struct blkio_policy_type *blkiop) diff --git a/block/blk-ioc.c b/block/blk-ioc.c index 8bebf06bac76..b13ed96776c2 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -16,6 +16,19 @@ */ static struct kmem_cache *iocontext_cachep; +/** + * get_io_context - increment reference count to io_context + * @ioc: io_context to get + * + * Increment reference count to @ioc. + */ +void get_io_context(struct io_context *ioc) +{ + BUG_ON(atomic_long_read(&ioc->refcount) <= 0); + atomic_long_inc(&ioc->refcount); +} +EXPORT_SYMBOL(get_io_context); + static void cfq_dtor(struct io_context *ioc) { if (!hlist_empty(&ioc->cic_list)) { @@ -71,6 +84,9 @@ void exit_io_context(struct task_struct *task) { struct io_context *ioc; + /* PF_EXITING prevents new io_context from being attached to @task */ + WARN_ON_ONCE(!(current->flags & PF_EXITING)); + task_lock(task); ioc = task->io_context; task->io_context = NULL; @@ -82,7 +98,9 @@ void exit_io_context(struct task_struct *task) put_io_context(ioc); } -struct io_context *alloc_io_context(gfp_t gfp_flags, int node) +static struct io_context *create_task_io_context(struct task_struct *task, + gfp_t gfp_flags, int node, + bool take_ref) { struct io_context *ioc; @@ -98,6 +116,20 @@ struct io_context *alloc_io_context(gfp_t gfp_flags, int node) INIT_RADIX_TREE(&ioc->radix_root, GFP_ATOMIC | __GFP_HIGH); INIT_HLIST_HEAD(&ioc->cic_list); + /* try to install, somebody might already have beaten us to it */ + task_lock(task); + + if (!task->io_context && !(task->flags & PF_EXITING)) { + task->io_context = ioc; + } else { + kmem_cache_free(iocontext_cachep, ioc); + ioc = task->io_context; + } + + if (ioc && take_ref) + get_io_context(ioc); + + task_unlock(task); return ioc; } @@ -114,46 +146,47 @@ struct io_context *alloc_io_context(gfp_t gfp_flags, int node) */ struct io_context *current_io_context(gfp_t gfp_flags, int node) { - struct task_struct *tsk = current; - struct io_context *ret; - - ret = tsk->io_context; - if (likely(ret)) - return ret; - - ret = alloc_io_context(gfp_flags, node); - if (ret) { - /* make sure set_task_ioprio() sees the settings above */ - smp_wmb(); - tsk->io_context = ret; - } + might_sleep_if(gfp_flags & __GFP_WAIT); - return ret; + if (current->io_context) + return current->io_context; + + return create_task_io_context(current, gfp_flags, node, false); } +EXPORT_SYMBOL(current_io_context); -/* - * If the current task has no IO context then create one and initialise it. - * If it does have a context, take a ref on it. +/** + * get_task_io_context - get io_context of a task + * @task: task of interest + * @gfp_flags: allocation flags, used if allocation is necessary + * @node: allocation node, used if allocation is necessary + * + * Return io_context of @task. If it doesn't exist, it is created with + * @gfp_flags and @node. The returned io_context has its reference count + * incremented. * - * This is always called in the context of the task which submitted the I/O. + * This function always goes through task_lock() and it's better to use + * current_io_context() + get_io_context() for %current. */ -struct io_context *get_io_context(gfp_t gfp_flags, int node) +struct io_context *get_task_io_context(struct task_struct *task, + gfp_t gfp_flags, int node) { - struct io_context *ioc = NULL; - - /* - * Check for unlikely race with exiting task. ioc ref count is - * zero when ioc is being detached. - */ - do { - ioc = current_io_context(gfp_flags, node); - if (unlikely(!ioc)) - break; - } while (!atomic_long_inc_not_zero(&ioc->refcount)); + struct io_context *ioc; - return ioc; + might_sleep_if(gfp_flags & __GFP_WAIT); + + task_lock(task); + ioc = task->io_context; + if (likely(ioc)) { + get_io_context(ioc); + task_unlock(task); + return ioc; + } + task_unlock(task); + + return create_task_io_context(task, gfp_flags, node, true); } -EXPORT_SYMBOL(get_io_context); +EXPORT_SYMBOL(get_task_io_context); static int __init blk_ioc_init(void) { diff --git a/block/blk.h b/block/blk.h index aae4d88fc523..fc3c41b2fd24 100644 --- a/block/blk.h +++ b/block/blk.h @@ -122,6 +122,7 @@ static inline int blk_should_fake_timeout(struct request_queue *q) } #endif +void get_io_context(struct io_context *ioc); struct io_context *current_io_context(gfp_t gfp_flags, int node); int ll_back_merge_fn(struct request_queue *q, struct request *req, diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index ec3f5e8ba564..d42d89ccce1b 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -14,6 +14,7 @@ #include #include #include +#include "blk.h" #include "cfq.h" /* @@ -3194,13 +3195,13 @@ static struct cfq_io_context * cfq_get_io_context(struct cfq_data *cfqd, gfp_t gfp_mask) { struct io_context *ioc = NULL; - struct cfq_io_context *cic; + struct cfq_io_context *cic = NULL; might_sleep_if(gfp_mask & __GFP_WAIT); - ioc = get_io_context(gfp_mask, cfqd->queue->node); + ioc = current_io_context(gfp_mask, cfqd->queue->node); if (!ioc) - return NULL; + goto err; cic = cfq_cic_lookup(cfqd, ioc); if (cic) @@ -3211,10 +3212,10 @@ cfq_get_io_context(struct cfq_data *cfqd, gfp_t gfp_mask) goto err; if (cfq_cic_link(cfqd, ioc, cic, gfp_mask)) - goto err_free; - + goto err; out: - smp_read_barrier_depends(); + get_io_context(ioc); + if (unlikely(ioc->ioprio_changed)) cfq_ioc_set_ioprio(ioc); @@ -3223,10 +3224,9 @@ out: cfq_ioc_set_cgroup(ioc); #endif return cic; -err_free: - cfq_cic_free(cic); err: - put_io_context(ioc); + if (cic) + cfq_cic_free(cic); return NULL; } diff --git a/fs/ioprio.c b/fs/ioprio.c index f79dab83e17b..998ec239d1ea 100644 --- a/fs/ioprio.c +++ b/fs/ioprio.c @@ -48,28 +48,13 @@ int set_task_ioprio(struct task_struct *task, int ioprio) if (err) return err; - task_lock(task); - do { - ioc = task->io_context; - /* see wmb() in current_io_context() */ - smp_read_barrier_depends(); - if (ioc) - break; - - ioc = alloc_io_context(GFP_ATOMIC, -1); - if (!ioc) { - err = -ENOMEM; - break; - } - task->io_context = ioc; - } while (1); - - if (!err) { + ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE); + if (ioc) { ioc->ioprio = ioprio; ioc->ioprio_changed = 1; + put_io_context(ioc); } - task_unlock(task); return err; } EXPORT_SYMBOL_GPL(set_task_ioprio); diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h index 8a6ecb66346f..28bb621ef5a2 100644 --- a/include/linux/iocontext.h +++ b/include/linux/iocontext.h @@ -78,8 +78,8 @@ struct task_struct; #ifdef CONFIG_BLOCK void put_io_context(struct io_context *ioc); void exit_io_context(struct task_struct *task); -struct io_context *get_io_context(gfp_t gfp_flags, int node); -struct io_context *alloc_io_context(gfp_t gfp_flags, int node); +struct io_context *get_task_io_context(struct task_struct *task, + gfp_t gfp_flags, int node); #else struct io_context; static inline void put_io_context(struct io_context *ioc) { } diff --git a/kernel/fork.c b/kernel/fork.c index da4a6a10d088..5bcfc739bb7c 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -870,6 +870,7 @@ static int copy_io(unsigned long clone_flags, struct task_struct *tsk) { #ifdef CONFIG_BLOCK struct io_context *ioc = current->io_context; + struct io_context *new_ioc; if (!ioc) return 0; @@ -881,11 +882,12 @@ static int copy_io(unsigned long clone_flags, struct task_struct *tsk) if (unlikely(!tsk->io_context)) return -ENOMEM; } else if (ioprio_valid(ioc->ioprio)) { - tsk->io_context = alloc_io_context(GFP_KERNEL, -1); - if (unlikely(!tsk->io_context)) + new_ioc = get_task_io_context(tsk, GFP_KERNEL, NUMA_NO_NODE); + if (unlikely(!new_ioc)) return -ENOMEM; - tsk->io_context->ioprio = ioc->ioprio; + new_ioc->ioprio = ioc->ioprio; + put_io_context(new_ioc); } #endif return 0; -- cgit v1.2.3 From 283287a52e3c3f7f8f9da747f4b8c5202740d776 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 14 Dec 2011 00:33:38 +0100 Subject: block, cfq: misc updates to cfq_io_context Make the following changes to prepare for ioc/cic management cleanup. * Add cic->q so that ioc can determine the associated queue without querying cfq. This will eventually replace ->key. * Factor out cfq_release_cic() from cic_free_func(). This function assumes that the caller handled locking. * Rename __cfq_exit_single_io_context() to cfq_exit_cic() and make it take only @cic. * Restructure cfq_cic_link() for future updates. This patch doesn't introduce any functional changes. Signed-off-by: Tejun Heo Signed-off-by: Jens Axboe --- block/cfq-iosched.c | 58 ++++++++++++++++++++++++++--------------------- include/linux/iocontext.h | 1 + 2 files changed, 33 insertions(+), 26 deletions(-) (limited to 'include/linux/iocontext.h') diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index d42d89ccce1b..a612ca65f371 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -2709,21 +2709,26 @@ static void cfq_cic_free(struct cfq_io_context *cic) call_rcu(&cic->rcu_head, cfq_cic_free_rcu); } -static void cic_free_func(struct io_context *ioc, struct cfq_io_context *cic) +static void cfq_release_cic(struct cfq_io_context *cic) { - unsigned long flags; + struct io_context *ioc = cic->ioc; unsigned long dead_key = (unsigned long) cic->key; BUG_ON(!(dead_key & CIC_DEAD_KEY)); - - spin_lock_irqsave(&ioc->lock, flags); radix_tree_delete(&ioc->radix_root, dead_key >> CIC_DEAD_INDEX_SHIFT); hlist_del_rcu(&cic->cic_list); - spin_unlock_irqrestore(&ioc->lock, flags); - cfq_cic_free(cic); } +static void cic_free_func(struct io_context *ioc, struct cfq_io_context *cic) +{ + unsigned long flags; + + spin_lock_irqsave(&ioc->lock, flags); + cfq_release_cic(cic); + spin_unlock_irqrestore(&ioc->lock, flags); +} + /* * 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(), @@ -2773,9 +2778,9 @@ static void cfq_exit_cfqq(struct cfq_data *cfqd, struct cfq_queue *cfqq) cfq_put_queue(cfqq); } -static void __cfq_exit_single_io_context(struct cfq_data *cfqd, - struct cfq_io_context *cic) +static void cfq_exit_cic(struct cfq_io_context *cic) { + struct cfq_data *cfqd = cic_to_cfqd(cic); struct io_context *ioc = cic->ioc; list_del_init(&cic->queue_list); @@ -2823,7 +2828,7 @@ static void cfq_exit_single_io_context(struct io_context *ioc, */ smp_read_barrier_depends(); if (cic->key == cfqd) - __cfq_exit_single_io_context(cfqd, cic); + cfq_exit_cic(cic); spin_unlock_irqrestore(q->queue_lock, flags); } @@ -3161,28 +3166,29 @@ static int cfq_cic_link(struct cfq_data *cfqd, struct io_context *ioc, int ret; ret = radix_tree_preload(gfp_mask); - if (!ret) { - cic->ioc = ioc; - cic->key = cfqd; + if (ret) + goto out; - spin_lock_irqsave(&ioc->lock, flags); - ret = radix_tree_insert(&ioc->radix_root, cfqd->queue->id, cic); - if (!ret) - hlist_add_head_rcu(&cic->cic_list, &ioc->cic_list); - spin_unlock_irqrestore(&ioc->lock, flags); + cic->ioc = ioc; + cic->key = cfqd; + cic->q = cfqd->queue; + + spin_lock_irqsave(&ioc->lock, flags); + ret = radix_tree_insert(&ioc->radix_root, cfqd->queue->id, cic); + if (!ret) + hlist_add_head_rcu(&cic->cic_list, &ioc->cic_list); + spin_unlock_irqrestore(&ioc->lock, flags); - radix_tree_preload_end(); + radix_tree_preload_end(); - if (!ret) { - spin_lock_irqsave(cfqd->queue->queue_lock, flags); - list_add(&cic->queue_list, &cfqd->cic_list); - spin_unlock_irqrestore(cfqd->queue->queue_lock, flags); - } + if (!ret) { + spin_lock_irqsave(cfqd->queue->queue_lock, flags); + list_add(&cic->queue_list, &cfqd->cic_list); + spin_unlock_irqrestore(cfqd->queue->queue_lock, flags); } - +out: if (ret) printk(KERN_ERR "cfq: cic link failed!\n"); - return ret; } @@ -3922,7 +3928,7 @@ static void cfq_exit_queue(struct elevator_queue *e) struct cfq_io_context, queue_list); - __cfq_exit_single_io_context(cfqd, cic); + cfq_exit_cic(cic); } cfq_put_async_queues(cfqd); diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h index 28bb621ef5a2..079aea8fd8a8 100644 --- a/include/linux/iocontext.h +++ b/include/linux/iocontext.h @@ -15,6 +15,7 @@ struct cfq_ttime { struct cfq_io_context { void *key; + struct request_queue *q; struct cfq_queue *cfqq[2]; -- cgit v1.2.3 From dc86900e0a8f665122de6faadd27fb4c6d2b3e4d Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 14 Dec 2011 00:33:38 +0100 Subject: block, cfq: move ioc ioprio/cgroup changed handling to cic ioprio/cgroup change was handled by marking the changed state in ioc and, on the following access to the ioc, performing RCU-protected iteration through all cic's grabbing the matching queue_lock. This patch moves the changed state to each cic. When ioprio or cgroup changes, the respective bit is set on all cic's of the ioc and when each of those cic (not ioc) is accessed, change is applied for that specific ioc-queue pair. This also fixes the following two race conditions between setting and clearing of changed states. * Missing barrier between assign/load of ioprio and ioprio_changed allowed applying old ioprio. * Change requests could happen between application of change and clearing of changed variables. Signed-off-by: Tejun Heo Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 2 +- block/blk-ioc.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ block/cfq-iosched.c | 28 +++++++++------------------- fs/ioprio.c | 3 +-- include/linux/iocontext.h | 14 +++++++++----- 5 files changed, 65 insertions(+), 27 deletions(-) (limited to 'include/linux/iocontext.h') diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 4b001dcd85b0..dc00835aab6a 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -1648,7 +1648,7 @@ static void blkiocg_attach_task(struct cgroup *cgrp, struct task_struct *tsk) /* we don't lose anything even if ioc allocation fails */ ioc = get_task_io_context(tsk, GFP_ATOMIC, NUMA_NO_NODE); if (ioc) { - ioc->cgroup_changed = 1; + ioc_cgroup_changed(ioc); put_io_context(ioc); } } diff --git a/block/blk-ioc.c b/block/blk-ioc.c index b13ed96776c2..6f59fbad93d9 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -188,6 +188,51 @@ struct io_context *get_task_io_context(struct task_struct *task, } EXPORT_SYMBOL(get_task_io_context); +void ioc_set_changed(struct io_context *ioc, int which) +{ + struct cfq_io_context *cic; + struct hlist_node *n; + + hlist_for_each_entry(cic, n, &ioc->cic_list, cic_list) + set_bit(which, &cic->changed); +} + +/** + * ioc_ioprio_changed - notify ioprio change + * @ioc: io_context of interest + * @ioprio: new ioprio + * + * @ioc's ioprio has changed to @ioprio. Set %CIC_IOPRIO_CHANGED for all + * cic's. iosched is responsible for checking the bit and applying it on + * request issue path. + */ +void ioc_ioprio_changed(struct io_context *ioc, int ioprio) +{ + unsigned long flags; + + spin_lock_irqsave(&ioc->lock, flags); + ioc->ioprio = ioprio; + ioc_set_changed(ioc, CIC_IOPRIO_CHANGED); + spin_unlock_irqrestore(&ioc->lock, flags); +} + +/** + * ioc_cgroup_changed - notify cgroup change + * @ioc: io_context of interest + * + * @ioc's cgroup has changed. Set %CIC_CGROUP_CHANGED for all cic's. + * iosched is responsible for checking the bit and applying it on request + * issue path. + */ +void ioc_cgroup_changed(struct io_context *ioc) +{ + unsigned long flags; + + spin_lock_irqsave(&ioc->lock, flags); + ioc_set_changed(ioc, CIC_CGROUP_CHANGED); + spin_unlock_irqrestore(&ioc->lock, flags); +} + static int __init blk_ioc_init(void) { iocontext_cachep = kmem_cache_create("blkdev_ioc", diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index a612ca65f371..51aece2eea7c 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -2904,7 +2904,7 @@ static void cfq_init_prio_data(struct cfq_queue *cfqq, struct io_context *ioc) cfq_clear_cfqq_prio_changed(cfqq); } -static void changed_ioprio(struct io_context *ioc, struct cfq_io_context *cic) +static void changed_ioprio(struct cfq_io_context *cic) { struct cfq_data *cfqd = cic_to_cfqd(cic); struct cfq_queue *cfqq; @@ -2933,12 +2933,6 @@ static void changed_ioprio(struct io_context *ioc, struct cfq_io_context *cic) spin_unlock_irqrestore(cfqd->queue->queue_lock, flags); } -static void cfq_ioc_set_ioprio(struct io_context *ioc) -{ - call_for_each_cic(ioc, changed_ioprio); - ioc->ioprio_changed = 0; -} - static void cfq_init_cfqq(struct cfq_data *cfqd, struct cfq_queue *cfqq, pid_t pid, bool is_sync) { @@ -2960,7 +2954,7 @@ static void cfq_init_cfqq(struct cfq_data *cfqd, struct cfq_queue *cfqq, } #ifdef CONFIG_CFQ_GROUP_IOSCHED -static void changed_cgroup(struct io_context *ioc, struct cfq_io_context *cic) +static void changed_cgroup(struct cfq_io_context *cic) { struct cfq_queue *sync_cfqq = cic_to_cfqq(cic, 1); struct cfq_data *cfqd = cic_to_cfqd(cic); @@ -2986,12 +2980,6 @@ static void changed_cgroup(struct io_context *ioc, struct cfq_io_context *cic) spin_unlock_irqrestore(q->queue_lock, flags); } - -static void cfq_ioc_set_cgroup(struct io_context *ioc) -{ - call_for_each_cic(ioc, changed_cgroup); - ioc->cgroup_changed = 0; -} #endif /* CONFIG_CFQ_GROUP_IOSCHED */ static struct cfq_queue * @@ -3222,13 +3210,15 @@ cfq_get_io_context(struct cfq_data *cfqd, gfp_t gfp_mask) out: get_io_context(ioc); - if (unlikely(ioc->ioprio_changed)) - cfq_ioc_set_ioprio(ioc); - + if (unlikely(cic->changed)) { + if (test_and_clear_bit(CIC_IOPRIO_CHANGED, &cic->changed)) + changed_ioprio(cic); #ifdef CONFIG_CFQ_GROUP_IOSCHED - if (unlikely(ioc->cgroup_changed)) - cfq_ioc_set_cgroup(ioc); + if (test_and_clear_bit(CIC_CGROUP_CHANGED, &cic->changed)) + changed_cgroup(cic); #endif + } + return cic; err: if (cic) diff --git a/fs/ioprio.c b/fs/ioprio.c index 998ec239d1ea..0f1b9515213b 100644 --- a/fs/ioprio.c +++ b/fs/ioprio.c @@ -50,8 +50,7 @@ int set_task_ioprio(struct task_struct *task, int ioprio) ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE); if (ioc) { - ioc->ioprio = ioprio; - ioc->ioprio_changed = 1; + ioc_ioprio_changed(ioc, ioprio); put_io_context(ioc); } diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h index 079aea8fd8a8..2c2b6da96b3c 100644 --- a/include/linux/iocontext.h +++ b/include/linux/iocontext.h @@ -13,6 +13,11 @@ struct cfq_ttime { unsigned long ttime_mean; }; +enum { + CIC_IOPRIO_CHANGED, + CIC_CGROUP_CHANGED, +}; + struct cfq_io_context { void *key; struct request_queue *q; @@ -26,6 +31,8 @@ struct cfq_io_context { struct list_head queue_list; struct hlist_node cic_list; + unsigned long changed; + void (*dtor)(struct io_context *); /* destructor */ void (*exit)(struct io_context *); /* called on task exit */ @@ -44,11 +51,6 @@ struct io_context { spinlock_t lock; unsigned short ioprio; - unsigned short ioprio_changed; - -#if defined(CONFIG_BLK_CGROUP) || defined(CONFIG_BLK_CGROUP_MODULE) - unsigned short cgroup_changed; -#endif /* * For request batching @@ -81,6 +83,8 @@ void put_io_context(struct io_context *ioc); void exit_io_context(struct task_struct *task); struct io_context *get_task_io_context(struct task_struct *task, gfp_t gfp_flags, int node); +void ioc_ioprio_changed(struct io_context *ioc, int ioprio); +void ioc_cgroup_changed(struct io_context *ioc); #else struct io_context; static inline void put_io_context(struct io_context *ioc) { } -- cgit v1.2.3 From b2efa05265d62bc29f3a64400fad4b44340eedb8 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 14 Dec 2011 00:33:39 +0100 Subject: block, cfq: unlink cfq_io_context's immediately cic is association between io_context and request_queue. A cic is linked from both ioc and q and should be destroyed when either one goes away. As ioc and q both have their own locks, locking becomes a bit complex - both orders work for removal from one but not from the other. Currently, cfq tries to circumvent this locking order issue with RCU. ioc->lock nests inside queue_lock but the radix tree and cic's are also protected by RCU allowing either side to walk their lists without grabbing lock. This rather unconventional use of RCU quickly devolves into extremely fragile convolution. e.g. The following is from cfqd going away too soon after ioc and q exits raced. general protection fault: 0000 [#1] PREEMPT SMP CPU 2 Modules linked in: [ 88.503444] Pid: 599, comm: hexdump Not tainted 3.1.0-rc10-work+ #158 Bochs Bochs RIP: 0010:[] [] cfq_exit_single_io_context+0x58/0xf0 ... Call Trace: [] call_for_each_cic+0x5a/0x90 [] cfq_exit_io_context+0x15/0x20 [] exit_io_context+0x100/0x140 [] do_exit+0x579/0x850 [] do_group_exit+0x5b/0xd0 [] sys_exit_group+0x17/0x20 [] system_call_fastpath+0x16/0x1b The only real hot path here is cic lookup during request initialization and avoiding extra locking requires very confined use of RCU. This patch makes cic removal from both ioc and request_queue perform double-locking and unlink immediately. * From q side, the change is almost trivial as ioc->lock nests inside queue_lock. It just needs to grab each ioc->lock as it walks cic_list and unlink it. * From ioc side, it's a bit more difficult because of inversed lock order. ioc needs its lock to walk its cic_list but can't grab the matching queue_lock and needs to perform unlock-relock dancing. Unlinking is now wholly done from put_io_context() and fast path is optimized by using the queue_lock the caller already holds, which is by far the most common case. If the ioc accessed multiple devices, it tries with trylock. In unlikely cases of fast path failure, it falls back to full double-locking dance from workqueue. Double-locking isn't the prettiest thing in the world but it's *far* simpler and more understandable than RCU trick without adding any meaningful overhead. This still leaves a lot of now unnecessary RCU logics. Future patches will trim them. -v2: Vivek pointed out that cic->q was being dereferenced after cic->release() was called. Updated to use local variable @this_q instead. Signed-off-by: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 2 +- block/blk-ioc.c | 166 ++++++++++++++++++++++++++++++++++++++-------- block/cfq-iosched.c | 44 +++--------- fs/ioprio.c | 2 +- include/linux/blkdev.h | 3 + include/linux/iocontext.h | 12 ++-- kernel/fork.c | 2 +- 7 files changed, 159 insertions(+), 72 deletions(-) (limited to 'include/linux/iocontext.h') diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index dc00835aab6a..278869358049 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -1649,7 +1649,7 @@ static void blkiocg_attach_task(struct cgroup *cgrp, struct task_struct *tsk) ioc = get_task_io_context(tsk, GFP_ATOMIC, NUMA_NO_NODE); if (ioc) { ioc_cgroup_changed(ioc); - put_io_context(ioc); + put_io_context(ioc, NULL); } } diff --git a/block/blk-ioc.c b/block/blk-ioc.c index 6f59fbad93d9..fb23965595da 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -29,55 +29,164 @@ void get_io_context(struct io_context *ioc) } EXPORT_SYMBOL(get_io_context); -static void cfq_dtor(struct io_context *ioc) +/* + * Releasing ioc may nest into another put_io_context() leading to nested + * fast path release. As the ioc's can't be the same, this is okay but + * makes lockdep whine. Keep track of nesting and use it as subclass. + */ +#ifdef CONFIG_LOCKDEP +#define ioc_release_depth(q) ((q) ? (q)->ioc_release_depth : 0) +#define ioc_release_depth_inc(q) (q)->ioc_release_depth++ +#define ioc_release_depth_dec(q) (q)->ioc_release_depth-- +#else +#define ioc_release_depth(q) 0 +#define ioc_release_depth_inc(q) do { } while (0) +#define ioc_release_depth_dec(q) do { } while (0) +#endif + +/* + * Slow path for ioc release in put_io_context(). Performs double-lock + * dancing to unlink all cic's and then frees ioc. + */ +static void ioc_release_fn(struct work_struct *work) { - if (!hlist_empty(&ioc->cic_list)) { - struct cfq_io_context *cic; + struct io_context *ioc = container_of(work, struct io_context, + release_work); + struct request_queue *last_q = NULL; + + spin_lock_irq(&ioc->lock); + + while (!hlist_empty(&ioc->cic_list)) { + struct cfq_io_context *cic = hlist_entry(ioc->cic_list.first, + struct cfq_io_context, + cic_list); + struct request_queue *this_q = cic->q; + + if (this_q != last_q) { + /* + * Need to switch to @this_q. Once we release + * @ioc->lock, it can go away along with @cic. + * Hold on to it. + */ + __blk_get_queue(this_q); + + /* + * blk_put_queue() might sleep thanks to kobject + * idiocy. Always release both locks, put and + * restart. + */ + if (last_q) { + spin_unlock(last_q->queue_lock); + spin_unlock_irq(&ioc->lock); + blk_put_queue(last_q); + } else { + spin_unlock_irq(&ioc->lock); + } + + last_q = this_q; + spin_lock_irq(this_q->queue_lock); + spin_lock(&ioc->lock); + continue; + } + ioc_release_depth_inc(this_q); + cic->exit(cic); + cic->release(cic); + ioc_release_depth_dec(this_q); + } - cic = hlist_entry(ioc->cic_list.first, struct cfq_io_context, - cic_list); - cic->dtor(ioc); + if (last_q) { + spin_unlock(last_q->queue_lock); + spin_unlock_irq(&ioc->lock); + blk_put_queue(last_q); + } else { + spin_unlock_irq(&ioc->lock); } + + kmem_cache_free(iocontext_cachep, ioc); } /** * put_io_context - put a reference of io_context * @ioc: io_context to put + * @locked_q: request_queue the caller is holding queue_lock of (hint) * * Decrement reference count of @ioc and release it if the count reaches - * zero. + * zero. If the caller is holding queue_lock of a queue, it can indicate + * that with @locked_q. This is an optimization hint and the caller is + * allowed to pass in %NULL even when it's holding a queue_lock. */ -void put_io_context(struct io_context *ioc) +void put_io_context(struct io_context *ioc, struct request_queue *locked_q) { + struct request_queue *last_q = locked_q; + unsigned long flags; + if (ioc == NULL) return; BUG_ON(atomic_long_read(&ioc->refcount) <= 0); + if (locked_q) + lockdep_assert_held(locked_q->queue_lock); if (!atomic_long_dec_and_test(&ioc->refcount)) return; - rcu_read_lock(); - cfq_dtor(ioc); - rcu_read_unlock(); - - kmem_cache_free(iocontext_cachep, ioc); -} -EXPORT_SYMBOL(put_io_context); + /* + * Destroy @ioc. This is a bit messy because cic's are chained + * from both ioc and queue, and ioc->lock nests inside queue_lock. + * The inner ioc->lock should be held to walk our cic_list and then + * for each cic the outer matching queue_lock should be grabbed. + * ie. We need to do reverse-order double lock dancing. + * + * Another twist is that we are often called with one of the + * matching queue_locks held as indicated by @locked_q, which + * prevents performing double-lock dance for other queues. + * + * So, we do it in two stages. The fast path uses the queue_lock + * the caller is holding and, if other queues need to be accessed, + * uses trylock to avoid introducing locking dependency. This can + * handle most cases, especially if @ioc was performing IO on only + * single device. + * + * If trylock doesn't cut it, we defer to @ioc->release_work which + * can do all the double-locking dancing. + */ + spin_lock_irqsave_nested(&ioc->lock, flags, + ioc_release_depth(locked_q)); + + while (!hlist_empty(&ioc->cic_list)) { + struct cfq_io_context *cic = hlist_entry(ioc->cic_list.first, + struct cfq_io_context, + cic_list); + struct request_queue *this_q = cic->q; + + if (this_q != last_q) { + if (last_q && last_q != locked_q) + spin_unlock(last_q->queue_lock); + last_q = NULL; + + if (!spin_trylock(this_q->queue_lock)) + break; + last_q = this_q; + continue; + } + ioc_release_depth_inc(this_q); + cic->exit(cic); + cic->release(cic); + ioc_release_depth_dec(this_q); + } -static void cfq_exit(struct io_context *ioc) -{ - rcu_read_lock(); + if (last_q && last_q != locked_q) + spin_unlock(last_q->queue_lock); - if (!hlist_empty(&ioc->cic_list)) { - struct cfq_io_context *cic; + spin_unlock_irqrestore(&ioc->lock, flags); - cic = hlist_entry(ioc->cic_list.first, struct cfq_io_context, - cic_list); - cic->exit(ioc); - } - rcu_read_unlock(); + /* if no cic's left, we're done; otherwise, kick release_work */ + if (hlist_empty(&ioc->cic_list)) + kmem_cache_free(iocontext_cachep, ioc); + else + schedule_work(&ioc->release_work); } +EXPORT_SYMBOL(put_io_context); /* Called by the exiting task */ void exit_io_context(struct task_struct *task) @@ -92,10 +201,8 @@ void exit_io_context(struct task_struct *task) task->io_context = NULL; task_unlock(task); - if (atomic_dec_and_test(&ioc->nr_tasks)) - cfq_exit(ioc); - - put_io_context(ioc); + atomic_dec(&ioc->nr_tasks); + put_io_context(ioc, NULL); } static struct io_context *create_task_io_context(struct task_struct *task, @@ -115,6 +222,7 @@ static struct io_context *create_task_io_context(struct task_struct *task, spin_lock_init(&ioc->lock); INIT_RADIX_TREE(&ioc->radix_root, GFP_ATOMIC | __GFP_HIGH); INIT_HLIST_HEAD(&ioc->cic_list); + INIT_WORK(&ioc->release_work, ioc_release_fn); /* try to install, somebody might already have beaten us to it */ task_lock(task); diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index e617b088c59b..6cc606560402 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -1778,7 +1778,7 @@ __cfq_slice_expired(struct cfq_data *cfqd, struct cfq_queue *cfqq, cfqd->active_queue = NULL; if (cfqd->active_cic) { - put_io_context(cfqd->active_cic->ioc); + put_io_context(cfqd->active_cic->ioc, cfqd->queue); cfqd->active_cic = NULL; } } @@ -2812,38 +2812,6 @@ static void cfq_exit_cic(struct cfq_io_context *cic) } } -static void cfq_exit_single_io_context(struct io_context *ioc, - struct cfq_io_context *cic) -{ - struct cfq_data *cfqd = cic_to_cfqd(cic); - - if (cfqd) { - struct request_queue *q = cfqd->queue; - unsigned long flags; - - spin_lock_irqsave(q->queue_lock, flags); - - /* - * Ensure we get a fresh copy of the ->key to prevent - * race between exiting task and queue - */ - smp_read_barrier_depends(); - if (cic->key == cfqd) - cfq_exit_cic(cic); - - spin_unlock_irqrestore(q->queue_lock, flags); - } -} - -/* - * The process that ioc belongs to has exited, we need to clean up - * and put the internal structures we have that belongs to that process. - */ -static void cfq_exit_io_context(struct io_context *ioc) -{ - call_for_each_cic(ioc, cfq_exit_single_io_context); -} - static struct cfq_io_context * cfq_alloc_io_context(struct cfq_data *cfqd, gfp_t gfp_mask) { @@ -2855,8 +2823,8 @@ cfq_alloc_io_context(struct cfq_data *cfqd, gfp_t gfp_mask) cic->ttime.last_end_request = jiffies; INIT_LIST_HEAD(&cic->queue_list); INIT_HLIST_NODE(&cic->cic_list); - cic->dtor = cfq_free_io_context; - cic->exit = cfq_exit_io_context; + cic->exit = cfq_exit_cic; + cic->release = cfq_release_cic; elv_ioc_count_inc(cfq_ioc_count); } @@ -3726,7 +3694,7 @@ static void cfq_put_request(struct request *rq) BUG_ON(!cfqq->allocated[rw]); cfqq->allocated[rw]--; - put_io_context(RQ_CIC(rq)->ioc); + put_io_context(RQ_CIC(rq)->ioc, cfqq->cfqd->queue); rq->elevator_private[0] = NULL; rq->elevator_private[1] = NULL; @@ -3937,8 +3905,12 @@ static void cfq_exit_queue(struct elevator_queue *e) struct cfq_io_context *cic = list_entry(cfqd->cic_list.next, struct cfq_io_context, queue_list); + struct io_context *ioc = cic->ioc; + spin_lock(&ioc->lock); cfq_exit_cic(cic); + cfq_release_cic(cic); + spin_unlock(&ioc->lock); } cfq_put_async_queues(cfqd); diff --git a/fs/ioprio.c b/fs/ioprio.c index 0f1b9515213b..f84b380d65e5 100644 --- a/fs/ioprio.c +++ b/fs/ioprio.c @@ -51,7 +51,7 @@ int set_task_ioprio(struct task_struct *task, int ioprio) ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE); if (ioc) { ioc_ioprio_changed(ioc, ioprio); - put_io_context(ioc); + put_io_context(ioc, NULL); } return err; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index d1b6f4ed1f96..65c2f8c70089 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -393,6 +393,9 @@ struct request_queue { /* Throttle data */ struct throtl_data *td; #endif +#ifdef CONFIG_LOCKDEP + int ioc_release_depth; +#endif }; #define QUEUE_FLAG_QUEUED 1 /* uses generic tag queueing */ diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h index 2c2b6da96b3c..01e863128780 100644 --- a/include/linux/iocontext.h +++ b/include/linux/iocontext.h @@ -3,6 +3,7 @@ #include #include +#include struct cfq_queue; struct cfq_ttime { @@ -33,8 +34,8 @@ struct cfq_io_context { unsigned long changed; - void (*dtor)(struct io_context *); /* destructor */ - void (*exit)(struct io_context *); /* called on task exit */ + void (*exit)(struct cfq_io_context *); + void (*release)(struct cfq_io_context *); struct rcu_head rcu_head; }; @@ -61,6 +62,8 @@ struct io_context { struct radix_tree_root radix_root; struct hlist_head cic_list; void __rcu *ioc_data; + + struct work_struct release_work; }; static inline struct io_context *ioc_task_link(struct io_context *ioc) @@ -79,7 +82,7 @@ static inline struct io_context *ioc_task_link(struct io_context *ioc) struct task_struct; #ifdef CONFIG_BLOCK -void put_io_context(struct io_context *ioc); +void put_io_context(struct io_context *ioc, struct request_queue *locked_q); void exit_io_context(struct task_struct *task); struct io_context *get_task_io_context(struct task_struct *task, gfp_t gfp_flags, int node); @@ -87,7 +90,8 @@ void ioc_ioprio_changed(struct io_context *ioc, int ioprio); void ioc_cgroup_changed(struct io_context *ioc); #else struct io_context; -static inline void put_io_context(struct io_context *ioc) { } +static inline void put_io_context(struct io_context *ioc, + struct request_queue *locked_q) { } static inline void exit_io_context(struct task_struct *task) { } #endif diff --git a/kernel/fork.c b/kernel/fork.c index 5bcfc739bb7c..2753449f2038 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -887,7 +887,7 @@ static int copy_io(unsigned long clone_flags, struct task_struct *tsk) return -ENOMEM; new_ioc->ioprio = ioc->ioprio; - put_io_context(new_ioc); + put_io_context(new_ioc, NULL); } #endif return 0; -- cgit v1.2.3 From 1238033c79e92e5c315af12e45396f1a78c73dec Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 14 Dec 2011 00:33:40 +0100 Subject: block, cfq: kill cic->key Now that lazy paths are removed, cfqd_dead_key() is meaningless and cic->q can be used whereever cic->key is used. Kill cic->key. Signed-off-by: Tejun Heo Signed-off-by: Jens Axboe --- block/cfq-iosched.c | 26 +++++--------------------- include/linux/iocontext.h | 1 - 2 files changed, 5 insertions(+), 22 deletions(-) (limited to 'include/linux/iocontext.h') diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index ae7791a8ded9..3b07ce168780 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -472,22 +472,9 @@ static inline void cic_set_cfqq(struct cfq_io_context *cic, cic->cfqq[is_sync] = cfqq; } -#define CIC_DEAD_KEY 1ul -#define CIC_DEAD_INDEX_SHIFT 1 - -static inline void *cfqd_dead_key(struct cfq_data *cfqd) -{ - return (void *)(cfqd->queue->id << CIC_DEAD_INDEX_SHIFT | CIC_DEAD_KEY); -} - static inline struct cfq_data *cic_to_cfqd(struct cfq_io_context *cic) { - struct cfq_data *cfqd = cic->key; - - if (unlikely((unsigned long) cfqd & CIC_DEAD_KEY)) - return NULL; - - return cfqd; + return cic->q->elevator->elevator_data; } /* @@ -2679,10 +2666,8 @@ static void cfq_cic_free(struct cfq_io_context *cic) static void cfq_release_cic(struct cfq_io_context *cic) { struct io_context *ioc = cic->ioc; - unsigned long dead_key = (unsigned long) cic->key; - BUG_ON(!(dead_key & CIC_DEAD_KEY)); - radix_tree_delete(&ioc->radix_root, dead_key >> CIC_DEAD_INDEX_SHIFT); + radix_tree_delete(&ioc->radix_root, cic->q->id); hlist_del(&cic->cic_list); cfq_cic_free(cic); } @@ -2726,7 +2711,6 @@ static void cfq_exit_cic(struct cfq_io_context *cic) struct io_context *ioc = cic->ioc; list_del_init(&cic->queue_list); - cic->key = cfqd_dead_key(cfqd); /* * Both setting lookup hint to and clearing it from @cic are done @@ -2982,6 +2966,7 @@ cfq_get_queue(struct cfq_data *cfqd, bool is_sync, struct io_context *ioc, static struct cfq_io_context * cfq_cic_lookup(struct cfq_data *cfqd, struct io_context *ioc) { + struct request_queue *q = cfqd->queue; struct cfq_io_context *cic; lockdep_assert_held(cfqd->queue->queue_lock); @@ -2996,11 +2981,11 @@ cfq_cic_lookup(struct cfq_data *cfqd, struct io_context *ioc) */ rcu_read_lock(); cic = rcu_dereference(ioc->ioc_data); - if (cic && cic->key == cfqd) + if (cic && cic->q == q) goto out; cic = radix_tree_lookup(&ioc->radix_root, cfqd->queue->id); - if (cic && cic->key == cfqd) + if (cic && cic->q == q) rcu_assign_pointer(ioc->ioc_data, cic); /* allowed to race */ else cic = NULL; @@ -3040,7 +3025,6 @@ static int cfq_create_cic(struct cfq_data *cfqd, gfp_t gfp_mask) goto out; cic->ioc = ioc; - cic->key = cfqd; cic->q = cfqd->queue; /* lock both q and ioc and try to link @cic */ diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h index 01e863128780..b2b75a54f252 100644 --- a/include/linux/iocontext.h +++ b/include/linux/iocontext.h @@ -20,7 +20,6 @@ enum { }; struct cfq_io_context { - void *key; struct request_queue *q; struct cfq_queue *cfqq[2]; -- cgit v1.2.3 From c58698073218f2c8f2fc5982fa3938c2d3803b9f Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 14 Dec 2011 00:33:41 +0100 Subject: block, cfq: reorganize cfq_io_context into generic and cfq specific parts Currently io_context and cfq logics are mixed without clear boundary. Most of io_context is independent from cfq but cfq_io_context handling logic is dispersed between generic ioc code and cfq. cfq_io_context represents association between an io_context and a request_queue, which is a concept useful outside of cfq, but it also contains fields which are useful only to cfq. This patch takes out generic part and put it into io_cq (io context-queue) and the rest into cfq_io_cq (cic moniker remains the same) which contains io_cq. The following changes are made together. * cfq_ttime and cfq_io_cq now live in cfq-iosched.c. * All related fields, functions and constants are renamed accordingly. * ioc->ioc_data is now "struct io_cq *" instead of "void *" and renamed to icq_hint. This prepares for io_context API cleanup. Documentation is currently sparse. It will be added later. Changes in this patch are mechanical and don't cause functional change. Signed-off-by: Tejun Heo Signed-off-by: Jens Axboe --- block/blk-ioc.c | 58 ++++++----- block/cfq-iosched.c | 248 +++++++++++++++++++++++++--------------------- include/linux/iocontext.h | 43 +++----- 3 files changed, 175 insertions(+), 174 deletions(-) (limited to 'include/linux/iocontext.h') diff --git a/block/blk-ioc.c b/block/blk-ioc.c index e23c797b4685..dc5e69d335a0 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -46,7 +46,7 @@ EXPORT_SYMBOL(get_io_context); /* * Slow path for ioc release in put_io_context(). Performs double-lock - * dancing to unlink all cic's and then frees ioc. + * dancing to unlink all icq's and then frees ioc. */ static void ioc_release_fn(struct work_struct *work) { @@ -56,11 +56,10 @@ static void ioc_release_fn(struct work_struct *work) spin_lock_irq(&ioc->lock); - while (!hlist_empty(&ioc->cic_list)) { - struct cfq_io_context *cic = hlist_entry(ioc->cic_list.first, - struct cfq_io_context, - cic_list); - struct request_queue *this_q = cic->q; + while (!hlist_empty(&ioc->icq_list)) { + struct io_cq *icq = hlist_entry(ioc->icq_list.first, + struct io_cq, ioc_node); + struct request_queue *this_q = icq->q; if (this_q != last_q) { /* @@ -89,8 +88,8 @@ static void ioc_release_fn(struct work_struct *work) continue; } ioc_release_depth_inc(this_q); - cic->exit(cic); - cic->release(cic); + icq->exit(icq); + icq->release(icq); ioc_release_depth_dec(this_q); } @@ -131,10 +130,10 @@ void put_io_context(struct io_context *ioc, struct request_queue *locked_q) return; /* - * Destroy @ioc. This is a bit messy because cic's are chained + * Destroy @ioc. This is a bit messy because icq's are chained * from both ioc and queue, and ioc->lock nests inside queue_lock. - * The inner ioc->lock should be held to walk our cic_list and then - * for each cic the outer matching queue_lock should be grabbed. + * The inner ioc->lock should be held to walk our icq_list and then + * for each icq the outer matching queue_lock should be grabbed. * ie. We need to do reverse-order double lock dancing. * * Another twist is that we are often called with one of the @@ -153,11 +152,10 @@ void put_io_context(struct io_context *ioc, struct request_queue *locked_q) spin_lock_irqsave_nested(&ioc->lock, flags, ioc_release_depth(locked_q)); - while (!hlist_empty(&ioc->cic_list)) { - struct cfq_io_context *cic = hlist_entry(ioc->cic_list.first, - struct cfq_io_context, - cic_list); - struct request_queue *this_q = cic->q; + while (!hlist_empty(&ioc->icq_list)) { + struct io_cq *icq = hlist_entry(ioc->icq_list.first, + struct io_cq, ioc_node); + struct request_queue *this_q = icq->q; if (this_q != last_q) { if (last_q && last_q != locked_q) @@ -170,8 +168,8 @@ void put_io_context(struct io_context *ioc, struct request_queue *locked_q) continue; } ioc_release_depth_inc(this_q); - cic->exit(cic); - cic->release(cic); + icq->exit(icq); + icq->release(icq); ioc_release_depth_dec(this_q); } @@ -180,8 +178,8 @@ void put_io_context(struct io_context *ioc, struct request_queue *locked_q) spin_unlock_irqrestore(&ioc->lock, flags); - /* if no cic's left, we're done; otherwise, kick release_work */ - if (hlist_empty(&ioc->cic_list)) + /* if no icq is left, we're done; otherwise, kick release_work */ + if (hlist_empty(&ioc->icq_list)) kmem_cache_free(iocontext_cachep, ioc); else schedule_work(&ioc->release_work); @@ -219,8 +217,8 @@ void create_io_context_slowpath(struct task_struct *task, gfp_t gfp_flags, atomic_long_set(&ioc->refcount, 1); atomic_set(&ioc->nr_tasks, 1); spin_lock_init(&ioc->lock); - INIT_RADIX_TREE(&ioc->radix_root, GFP_ATOMIC | __GFP_HIGH); - INIT_HLIST_HEAD(&ioc->cic_list); + INIT_RADIX_TREE(&ioc->icq_tree, GFP_ATOMIC | __GFP_HIGH); + INIT_HLIST_HEAD(&ioc->icq_list); INIT_WORK(&ioc->release_work, ioc_release_fn); /* try to install, somebody might already have beaten us to it */ @@ -270,11 +268,11 @@ EXPORT_SYMBOL(get_task_io_context); void ioc_set_changed(struct io_context *ioc, int which) { - struct cfq_io_context *cic; + struct io_cq *icq; struct hlist_node *n; - hlist_for_each_entry(cic, n, &ioc->cic_list, cic_list) - set_bit(which, &cic->changed); + hlist_for_each_entry(icq, n, &ioc->icq_list, ioc_node) + set_bit(which, &icq->changed); } /** @@ -282,8 +280,8 @@ void ioc_set_changed(struct io_context *ioc, int which) * @ioc: io_context of interest * @ioprio: new ioprio * - * @ioc's ioprio has changed to @ioprio. Set %CIC_IOPRIO_CHANGED for all - * cic's. iosched is responsible for checking the bit and applying it on + * @ioc's ioprio has changed to @ioprio. Set %ICQ_IOPRIO_CHANGED for all + * icq's. iosched is responsible for checking the bit and applying it on * request issue path. */ void ioc_ioprio_changed(struct io_context *ioc, int ioprio) @@ -292,7 +290,7 @@ void ioc_ioprio_changed(struct io_context *ioc, int ioprio) spin_lock_irqsave(&ioc->lock, flags); ioc->ioprio = ioprio; - ioc_set_changed(ioc, CIC_IOPRIO_CHANGED); + ioc_set_changed(ioc, ICQ_IOPRIO_CHANGED); spin_unlock_irqrestore(&ioc->lock, flags); } @@ -300,7 +298,7 @@ void ioc_ioprio_changed(struct io_context *ioc, int ioprio) * ioc_cgroup_changed - notify cgroup change * @ioc: io_context of interest * - * @ioc's cgroup has changed. Set %CIC_CGROUP_CHANGED for all cic's. + * @ioc's cgroup has changed. Set %ICQ_CGROUP_CHANGED for all icq's. * iosched is responsible for checking the bit and applying it on request * issue path. */ @@ -309,7 +307,7 @@ void ioc_cgroup_changed(struct io_context *ioc) unsigned long flags; spin_lock_irqsave(&ioc->lock, flags); - ioc_set_changed(ioc, CIC_CGROUP_CHANGED); + ioc_set_changed(ioc, ICQ_CGROUP_CHANGED); spin_unlock_irqrestore(&ioc->lock, flags); } diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 5f7e4d161404..d2f16fcdec7f 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -54,13 +54,12 @@ static const int cfq_hist_divisor = 4; #define CFQQ_SECT_THR_NONROT (sector_t)(2 * 32) #define CFQQ_SEEKY(cfqq) (hweight32(cfqq->seek_history) > 32/8) -#define RQ_CIC(rq) \ - ((struct cfq_io_context *) (rq)->elevator_private[0]) +#define RQ_CIC(rq) icq_to_cic((rq)->elevator_private[0]) #define RQ_CFQQ(rq) (struct cfq_queue *) ((rq)->elevator_private[1]) #define RQ_CFQG(rq) (struct cfq_group *) ((rq)->elevator_private[2]) static struct kmem_cache *cfq_pool; -static struct kmem_cache *cfq_ioc_pool; +static struct kmem_cache *cfq_icq_pool; #define CFQ_PRIO_LISTS IOPRIO_BE_NR #define cfq_class_idle(cfqq) ((cfqq)->ioprio_class == IOPRIO_CLASS_IDLE) @@ -69,6 +68,14 @@ static struct kmem_cache *cfq_ioc_pool; #define sample_valid(samples) ((samples) > 80) #define rb_entry_cfqg(node) rb_entry((node), struct cfq_group, rb_node) +struct cfq_ttime { + unsigned long last_end_request; + + unsigned long ttime_total; + unsigned long ttime_samples; + unsigned long ttime_mean; +}; + /* * Most of our rbtree usage is for sorting with min extraction, so * if we cache the leftmost node we don't have to walk down the tree @@ -210,6 +217,12 @@ struct cfq_group { struct cfq_ttime ttime; }; +struct cfq_io_cq { + struct io_cq icq; /* must be the first member */ + struct cfq_queue *cfqq[2]; + struct cfq_ttime ttime; +}; + /* * Per block device queue structure */ @@ -261,7 +274,7 @@ struct cfq_data { struct work_struct unplug_work; struct cfq_queue *active_queue; - struct cfq_io_context *active_cic; + struct cfq_io_cq *active_cic; /* * async queue for each priority case @@ -284,7 +297,7 @@ struct cfq_data { unsigned int cfq_group_idle; unsigned int cfq_latency; - struct list_head cic_list; + struct list_head icq_list; /* * Fallback dummy cfqq for extreme OOM conditions @@ -457,24 +470,28 @@ static inline int cfqg_busy_async_queues(struct cfq_data *cfqd, static void cfq_dispatch_insert(struct request_queue *, struct request *); static struct cfq_queue *cfq_get_queue(struct cfq_data *, bool, struct io_context *, gfp_t); -static struct cfq_io_context *cfq_cic_lookup(struct cfq_data *, - struct io_context *); +static struct cfq_io_cq *cfq_cic_lookup(struct cfq_data *, struct io_context *); -static inline struct cfq_queue *cic_to_cfqq(struct cfq_io_context *cic, - bool is_sync) +static inline struct cfq_io_cq *icq_to_cic(struct io_cq *icq) +{ + /* cic->icq is the first member, %NULL will convert to %NULL */ + return container_of(icq, struct cfq_io_cq, icq); +} + +static inline struct cfq_queue *cic_to_cfqq(struct cfq_io_cq *cic, bool is_sync) { return cic->cfqq[is_sync]; } -static inline void cic_set_cfqq(struct cfq_io_context *cic, - struct cfq_queue *cfqq, bool is_sync) +static inline void cic_set_cfqq(struct cfq_io_cq *cic, struct cfq_queue *cfqq, + bool is_sync) { cic->cfqq[is_sync] = cfqq; } -static inline struct cfq_data *cic_to_cfqd(struct cfq_io_context *cic) +static inline struct cfq_data *cic_to_cfqd(struct cfq_io_cq *cic) { - return cic->q->elevator->elevator_data; + return cic->icq.q->elevator->elevator_data; } /* @@ -1541,7 +1558,7 @@ static struct request * cfq_find_rq_fmerge(struct cfq_data *cfqd, struct bio *bio) { struct task_struct *tsk = current; - struct cfq_io_context *cic; + struct cfq_io_cq *cic; struct cfq_queue *cfqq; cic = cfq_cic_lookup(cfqd, tsk->io_context); @@ -1655,7 +1672,7 @@ static int cfq_allow_merge(struct request_queue *q, struct request *rq, struct bio *bio) { struct cfq_data *cfqd = q->elevator->elevator_data; - struct cfq_io_context *cic; + struct cfq_io_cq *cic; struct cfq_queue *cfqq; /* @@ -1671,7 +1688,7 @@ static int cfq_allow_merge(struct request_queue *q, struct request *rq, * and %current are guaranteed to be equal. Avoid lookup which * requires queue_lock by using @rq's cic. */ - if (current->io_context == RQ_CIC(rq)->ioc) { + if (current->io_context == RQ_CIC(rq)->icq.ioc) { cic = RQ_CIC(rq); } else { cic = cfq_cic_lookup(cfqd, current->io_context); @@ -1761,7 +1778,7 @@ __cfq_slice_expired(struct cfq_data *cfqd, struct cfq_queue *cfqq, cfqd->active_queue = NULL; if (cfqd->active_cic) { - put_io_context(cfqd->active_cic->ioc, cfqd->queue); + put_io_context(cfqd->active_cic->icq.ioc, cfqd->queue); cfqd->active_cic = NULL; } } @@ -1981,7 +1998,7 @@ static bool cfq_should_idle(struct cfq_data *cfqd, struct cfq_queue *cfqq) static void cfq_arm_slice_timer(struct cfq_data *cfqd) { struct cfq_queue *cfqq = cfqd->active_queue; - struct cfq_io_context *cic; + struct cfq_io_cq *cic; unsigned long sl, group_idle = 0; /* @@ -2016,7 +2033,7 @@ static void cfq_arm_slice_timer(struct cfq_data *cfqd) * task has exited, don't wait */ cic = cfqd->active_cic; - if (!cic || !atomic_read(&cic->ioc->nr_tasks)) + if (!cic || !atomic_read(&cic->icq.ioc->nr_tasks)) return; /* @@ -2567,9 +2584,9 @@ static bool cfq_dispatch_request(struct cfq_data *cfqd, struct cfq_queue *cfqq) cfq_dispatch_insert(cfqd->queue, rq); if (!cfqd->active_cic) { - struct cfq_io_context *cic = RQ_CIC(rq); + struct cfq_io_cq *cic = RQ_CIC(rq); - atomic_long_inc(&cic->ioc->refcount); + atomic_long_inc(&cic->icq.ioc->refcount); cfqd->active_cic = cic; } @@ -2652,24 +2669,24 @@ static void cfq_put_queue(struct cfq_queue *cfqq) cfq_put_cfqg(cfqg); } -static void cfq_cic_free_rcu(struct rcu_head *head) +static void cfq_icq_free_rcu(struct rcu_head *head) { - kmem_cache_free(cfq_ioc_pool, - container_of(head, struct cfq_io_context, rcu_head)); + kmem_cache_free(cfq_icq_pool, + icq_to_cic(container_of(head, struct io_cq, rcu_head))); } -static void cfq_cic_free(struct cfq_io_context *cic) +static void cfq_icq_free(struct io_cq *icq) { - call_rcu(&cic->rcu_head, cfq_cic_free_rcu); + call_rcu(&icq->rcu_head, cfq_icq_free_rcu); } -static void cfq_release_cic(struct cfq_io_context *cic) +static void cfq_release_icq(struct io_cq *icq) { - struct io_context *ioc = cic->ioc; + struct io_context *ioc = icq->ioc; - radix_tree_delete(&ioc->radix_root, cic->q->id); - hlist_del(&cic->cic_list); - cfq_cic_free(cic); + radix_tree_delete(&ioc->icq_tree, icq->q->id); + hlist_del(&icq->ioc_node); + cfq_icq_free(icq); } static void cfq_put_cooperator(struct cfq_queue *cfqq) @@ -2705,20 +2722,21 @@ static void cfq_exit_cfqq(struct cfq_data *cfqd, struct cfq_queue *cfqq) cfq_put_queue(cfqq); } -static void cfq_exit_cic(struct cfq_io_context *cic) +static void cfq_exit_icq(struct io_cq *icq) { + struct cfq_io_cq *cic = icq_to_cic(icq); struct cfq_data *cfqd = cic_to_cfqd(cic); - struct io_context *ioc = cic->ioc; + struct io_context *ioc = icq->ioc; - list_del_init(&cic->queue_list); + list_del_init(&icq->q_node); /* - * Both setting lookup hint to and clearing it from @cic are done - * under queue_lock. If it's not pointing to @cic now, it never + * Both setting lookup hint to and clearing it from @icq are done + * under queue_lock. If it's not pointing to @icq now, it never * will. Hint assignment itself can race safely. */ - if (rcu_dereference_raw(ioc->ioc_data) == cic) - rcu_assign_pointer(ioc->ioc_data, NULL); + if (rcu_dereference_raw(ioc->icq_hint) == icq) + rcu_assign_pointer(ioc->icq_hint, NULL); if (cic->cfqq[BLK_RW_ASYNC]) { cfq_exit_cfqq(cfqd, cic->cfqq[BLK_RW_ASYNC]); @@ -2731,19 +2749,18 @@ static void cfq_exit_cic(struct cfq_io_context *cic) } } -static struct cfq_io_context * -cfq_alloc_io_context(struct cfq_data *cfqd, gfp_t gfp_mask) +static struct cfq_io_cq *cfq_alloc_cic(struct cfq_data *cfqd, gfp_t gfp_mask) { - struct cfq_io_context *cic; + struct cfq_io_cq *cic; - cic = kmem_cache_alloc_node(cfq_ioc_pool, gfp_mask | __GFP_ZERO, + cic = kmem_cache_alloc_node(cfq_icq_pool, gfp_mask | __GFP_ZERO, cfqd->queue->node); if (cic) { cic->ttime.last_end_request = jiffies; - INIT_LIST_HEAD(&cic->queue_list); - INIT_HLIST_NODE(&cic->cic_list); - cic->exit = cfq_exit_cic; - cic->release = cfq_release_cic; + INIT_LIST_HEAD(&cic->icq.q_node); + INIT_HLIST_NODE(&cic->icq.ioc_node); + cic->icq.exit = cfq_exit_icq; + cic->icq.release = cfq_release_icq; } return cic; @@ -2791,7 +2808,7 @@ static void cfq_init_prio_data(struct cfq_queue *cfqq, struct io_context *ioc) cfq_clear_cfqq_prio_changed(cfqq); } -static void changed_ioprio(struct cfq_io_context *cic) +static void changed_ioprio(struct cfq_io_cq *cic) { struct cfq_data *cfqd = cic_to_cfqd(cic); struct cfq_queue *cfqq; @@ -2802,7 +2819,7 @@ static void changed_ioprio(struct cfq_io_context *cic) cfqq = cic->cfqq[BLK_RW_ASYNC]; if (cfqq) { struct cfq_queue *new_cfqq; - new_cfqq = cfq_get_queue(cfqd, BLK_RW_ASYNC, cic->ioc, + new_cfqq = cfq_get_queue(cfqd, BLK_RW_ASYNC, cic->icq.ioc, GFP_ATOMIC); if (new_cfqq) { cic->cfqq[BLK_RW_ASYNC] = new_cfqq; @@ -2836,7 +2853,7 @@ static void cfq_init_cfqq(struct cfq_data *cfqd, struct cfq_queue *cfqq, } #ifdef CONFIG_CFQ_GROUP_IOSCHED -static void changed_cgroup(struct cfq_io_context *cic) +static void changed_cgroup(struct cfq_io_cq *cic) { struct cfq_queue *sync_cfqq = cic_to_cfqq(cic, 1); struct cfq_data *cfqd = cic_to_cfqd(cic); @@ -2864,7 +2881,7 @@ cfq_find_alloc_queue(struct cfq_data *cfqd, bool is_sync, struct io_context *ioc, gfp_t gfp_mask) { struct cfq_queue *cfqq, *new_cfqq = NULL; - struct cfq_io_context *cic; + struct cfq_io_cq *cic; struct cfq_group *cfqg; retry: @@ -2956,56 +2973,57 @@ cfq_get_queue(struct cfq_data *cfqd, bool is_sync, struct io_context *ioc, } /** - * cfq_cic_lookup - lookup cfq_io_context + * cfq_cic_lookup - lookup cfq_io_cq * @cfqd: the associated cfq_data * @ioc: the associated io_context * - * Look up cfq_io_context associated with @cfqd - @ioc pair. Must be - * called with queue_lock held. + * Look up cfq_io_cq associated with @cfqd - @ioc pair. Must be called + * with queue_lock held. */ -static struct cfq_io_context * +static struct cfq_io_cq * cfq_cic_lookup(struct cfq_data *cfqd, struct io_context *ioc) { struct request_queue *q = cfqd->queue; - struct cfq_io_context *cic; + struct io_cq *icq; lockdep_assert_held(cfqd->queue->queue_lock); if (unlikely(!ioc)) return NULL; /* - * cic's are indexed from @ioc using radix tree and hint pointer, + * icq's are indexed from @ioc using radix tree and hint pointer, * both of which are protected with RCU. All removals are done * holding both q and ioc locks, and we're holding q lock - if we - * find a cic which points to us, it's guaranteed to be valid. + * find a icq which points to us, it's guaranteed to be valid. */ rcu_read_lock(); - cic = rcu_dereference(ioc->ioc_data); - if (cic && cic->q == q) + icq = rcu_dereference(ioc->icq_hint); + if (icq && icq->q == q) goto out; - cic = radix_tree_lookup(&ioc->radix_root, cfqd->queue->id); - if (cic && cic->q == q) - rcu_assign_pointer(ioc->ioc_data, cic); /* allowed to race */ + icq = radix_tree_lookup(&ioc->icq_tree, cfqd->queue->id); + if (icq && icq->q == q) + rcu_assign_pointer(ioc->icq_hint, icq); /* allowed to race */ else - cic = NULL; + icq = NULL; out: rcu_read_unlock(); - return cic; + return icq_to_cic(icq); } /** - * cfq_create_cic - create and link a cfq_io_context + * cfq_create_cic - create and link a cfq_io_cq * @cfqd: cfqd of interest * @gfp_mask: allocation mask * - * Make sure cfq_io_context linking %current->io_context and @cfqd exists. - * If ioc and/or cic doesn't exist, they will be created using @gfp_mask. + * Make sure cfq_io_cq linking %current->io_context and @cfqd exists. If + * ioc and/or cic doesn't exist, they will be created using @gfp_mask. */ static int cfq_create_cic(struct cfq_data *cfqd, gfp_t gfp_mask) { struct request_queue *q = cfqd->queue; - struct cfq_io_context *cic = NULL; + struct io_cq *icq = NULL; + struct cfq_io_cq *cic; struct io_context *ioc; int ret = -ENOMEM; @@ -3016,26 +3034,27 @@ static int cfq_create_cic(struct cfq_data *cfqd, gfp_t gfp_mask) if (!ioc) goto out; - cic = cfq_alloc_io_context(cfqd, gfp_mask); + cic = cfq_alloc_cic(cfqd, gfp_mask); if (!cic) goto out; + icq = &cic->icq; ret = radix_tree_preload(gfp_mask); if (ret) goto out; - cic->ioc = ioc; - cic->q = cfqd->queue; + icq->ioc = ioc; + icq->q = cfqd->queue; - /* lock both q and ioc and try to link @cic */ + /* lock both q and ioc and try to link @icq */ spin_lock_irq(q->queue_lock); spin_lock(&ioc->lock); - ret = radix_tree_insert(&ioc->radix_root, q->id, cic); + ret = radix_tree_insert(&ioc->icq_tree, q->id, icq); if (likely(!ret)) { - hlist_add_head(&cic->cic_list, &ioc->cic_list); - list_add(&cic->queue_list, &cfqd->cic_list); - cic = NULL; + hlist_add_head(&icq->ioc_node, &ioc->icq_list); + list_add(&icq->q_node, &cfqd->icq_list); + icq = NULL; } else if (ret == -EEXIST) { /* someone else already did it */ ret = 0; @@ -3047,29 +3066,28 @@ static int cfq_create_cic(struct cfq_data *cfqd, gfp_t gfp_mask) radix_tree_preload_end(); out: if (ret) - printk(KERN_ERR "cfq: cic link failed!\n"); - if (cic) - cfq_cic_free(cic); + printk(KERN_ERR "cfq: icq link failed!\n"); + if (icq) + cfq_icq_free(icq); return ret; } /** - * cfq_get_io_context - acquire cfq_io_context and bump refcnt on io_context + * cfq_get_cic - acquire cfq_io_cq and bump refcnt on io_context * @cfqd: cfqd to setup cic for * @gfp_mask: allocation mask * - * Return cfq_io_context associating @cfqd and %current->io_context and + * Return cfq_io_cq associating @cfqd and %current->io_context and * bump refcnt on io_context. If ioc or cic doesn't exist, they're created * using @gfp_mask. * * Must be called under queue_lock which may be released and re-acquired. * This function also may sleep depending on @gfp_mask. */ -static struct cfq_io_context * -cfq_get_io_context(struct cfq_data *cfqd, gfp_t gfp_mask) +static struct cfq_io_cq *cfq_get_cic(struct cfq_data *cfqd, gfp_t gfp_mask) { struct request_queue *q = cfqd->queue; - struct cfq_io_context *cic = NULL; + struct cfq_io_cq *cic = NULL; struct io_context *ioc; int err; @@ -3095,11 +3113,11 @@ cfq_get_io_context(struct cfq_data *cfqd, gfp_t gfp_mask) /* bump @ioc's refcnt and handle changed notifications */ get_io_context(ioc); - if (unlikely(cic->changed)) { - if (test_and_clear_bit(CIC_IOPRIO_CHANGED, &cic->changed)) + if (unlikely(cic->icq.changed)) { + if (test_and_clear_bit(ICQ_IOPRIO_CHANGED, &cic->icq.changed)) changed_ioprio(cic); #ifdef CONFIG_CFQ_GROUP_IOSCHED - if (test_and_clear_bit(CIC_CGROUP_CHANGED, &cic->changed)) + if (test_and_clear_bit(ICQ_CGROUP_CHANGED, &cic->icq.changed)) changed_cgroup(cic); #endif } @@ -3120,7 +3138,7 @@ __cfq_update_io_thinktime(struct cfq_ttime *ttime, unsigned long slice_idle) static void cfq_update_io_thinktime(struct cfq_data *cfqd, struct cfq_queue *cfqq, - struct cfq_io_context *cic) + struct cfq_io_cq *cic) { if (cfq_cfqq_sync(cfqq)) { __cfq_update_io_thinktime(&cic->ttime, cfqd->cfq_slice_idle); @@ -3158,7 +3176,7 @@ cfq_update_io_seektime(struct cfq_data *cfqd, struct cfq_queue *cfqq, */ static void cfq_update_idle_window(struct cfq_data *cfqd, struct cfq_queue *cfqq, - struct cfq_io_context *cic) + struct cfq_io_cq *cic) { int old_idle, enable_idle; @@ -3175,8 +3193,9 @@ cfq_update_idle_window(struct cfq_data *cfqd, struct cfq_queue *cfqq, if (cfqq->next_rq && (cfqq->next_rq->cmd_flags & REQ_NOIDLE)) enable_idle = 0; - else if (!atomic_read(&cic->ioc->nr_tasks) || !cfqd->cfq_slice_idle || - (!cfq_cfqq_deep(cfqq) && CFQQ_SEEKY(cfqq))) + else if (!atomic_read(&cic->icq.ioc->nr_tasks) || + !cfqd->cfq_slice_idle || + (!cfq_cfqq_deep(cfqq) && CFQQ_SEEKY(cfqq))) enable_idle = 0; else if (sample_valid(cic->ttime.ttime_samples)) { if (cic->ttime.ttime_mean > cfqd->cfq_slice_idle) @@ -3308,7 +3327,7 @@ static void cfq_rq_enqueued(struct cfq_data *cfqd, struct cfq_queue *cfqq, struct request *rq) { - struct cfq_io_context *cic = RQ_CIC(rq); + struct cfq_io_cq *cic = RQ_CIC(rq); cfqd->rq_queued++; if (rq->cmd_flags & REQ_PRIO) @@ -3361,7 +3380,7 @@ static void cfq_insert_request(struct request_queue *q, struct request *rq) struct cfq_queue *cfqq = RQ_CFQQ(rq); cfq_log_cfqq(cfqd, cfqq, "insert_request"); - cfq_init_prio_data(cfqq, RQ_CIC(rq)->ioc); + cfq_init_prio_data(cfqq, RQ_CIC(rq)->icq.ioc); rq_set_fifo_time(rq, jiffies + cfqd->cfq_fifo_expire[rq_is_sync(rq)]); list_add_tail(&rq->queuelist, &cfqq->fifo); @@ -3411,7 +3430,7 @@ static void cfq_update_hw_tag(struct cfq_data *cfqd) static bool cfq_should_wait_busy(struct cfq_data *cfqd, struct cfq_queue *cfqq) { - struct cfq_io_context *cic = cfqd->active_cic; + struct cfq_io_cq *cic = cfqd->active_cic; /* If the queue already has requests, don't wait */ if (!RB_EMPTY_ROOT(&cfqq->sort_list)) @@ -3548,7 +3567,7 @@ static int cfq_may_queue(struct request_queue *q, int rw) { struct cfq_data *cfqd = q->elevator->elevator_data; struct task_struct *tsk = current; - struct cfq_io_context *cic; + struct cfq_io_cq *cic; struct cfq_queue *cfqq; /* @@ -3563,7 +3582,7 @@ static int cfq_may_queue(struct request_queue *q, int rw) cfqq = cic_to_cfqq(cic, rw_is_sync(rw)); if (cfqq) { - cfq_init_prio_data(cfqq, cic->ioc); + cfq_init_prio_data(cfqq, cic->icq.ioc); return __cfq_may_queue(cfqq); } @@ -3584,7 +3603,7 @@ static void cfq_put_request(struct request *rq) BUG_ON(!cfqq->allocated[rw]); cfqq->allocated[rw]--; - put_io_context(RQ_CIC(rq)->ioc, cfqq->cfqd->queue); + put_io_context(RQ_CIC(rq)->icq.ioc, cfqq->cfqd->queue); rq->elevator_private[0] = NULL; rq->elevator_private[1] = NULL; @@ -3598,7 +3617,7 @@ static void cfq_put_request(struct request *rq) } static struct cfq_queue * -cfq_merge_cfqqs(struct cfq_data *cfqd, struct cfq_io_context *cic, +cfq_merge_cfqqs(struct cfq_data *cfqd, struct cfq_io_cq *cic, struct cfq_queue *cfqq) { cfq_log_cfqq(cfqd, cfqq, "merging with queue %p", cfqq->new_cfqq); @@ -3613,7 +3632,7 @@ cfq_merge_cfqqs(struct cfq_data *cfqd, struct cfq_io_context *cic, * was the last process referring to said cfqq. */ static struct cfq_queue * -split_cfqq(struct cfq_io_context *cic, struct cfq_queue *cfqq) +split_cfqq(struct cfq_io_cq *cic, struct cfq_queue *cfqq) { if (cfqq_process_refs(cfqq) == 1) { cfqq->pid = current->pid; @@ -3636,7 +3655,7 @@ static int cfq_set_request(struct request_queue *q, struct request *rq, gfp_t gfp_mask) { struct cfq_data *cfqd = q->elevator->elevator_data; - struct cfq_io_context *cic; + struct cfq_io_cq *cic; const int rw = rq_data_dir(rq); const bool is_sync = rq_is_sync(rq); struct cfq_queue *cfqq; @@ -3644,14 +3663,14 @@ cfq_set_request(struct request_queue *q, struct request *rq, gfp_t gfp_mask) might_sleep_if(gfp_mask & __GFP_WAIT); spin_lock_irq(q->queue_lock); - cic = cfq_get_io_context(cfqd, gfp_mask); + cic = cfq_get_cic(cfqd, gfp_mask); if (!cic) goto queue_fail; new_queue: cfqq = cic_to_cfqq(cic, is_sync); if (!cfqq || cfqq == &cfqd->oom_cfqq) { - cfqq = cfq_get_queue(cfqd, is_sync, cic->ioc, gfp_mask); + cfqq = cfq_get_queue(cfqd, is_sync, cic->icq.ioc, gfp_mask); cic_set_cfqq(cic, cfqq, is_sync); } else { /* @@ -3677,7 +3696,7 @@ new_queue: cfqq->allocated[rw]++; cfqq->ref++; - rq->elevator_private[0] = cic; + rq->elevator_private[0] = &cic->icq; rq->elevator_private[1] = cfqq; rq->elevator_private[2] = cfq_ref_get_cfqg(cfqq->cfqg); spin_unlock_irq(q->queue_lock); @@ -3791,15 +3810,14 @@ static void cfq_exit_queue(struct elevator_queue *e) if (cfqd->active_queue) __cfq_slice_expired(cfqd, cfqd->active_queue, 0); - while (!list_empty(&cfqd->cic_list)) { - struct cfq_io_context *cic = list_entry(cfqd->cic_list.next, - struct cfq_io_context, - queue_list); - struct io_context *ioc = cic->ioc; + while (!list_empty(&cfqd->icq_list)) { + struct io_cq *icq = list_entry(cfqd->icq_list.next, + struct io_cq, q_node); + struct io_context *ioc = icq->ioc; spin_lock(&ioc->lock); - cfq_exit_cic(cic); - cfq_release_cic(cic); + cfq_exit_icq(icq); + cfq_release_icq(icq); spin_unlock(&ioc->lock); } @@ -3904,7 +3922,7 @@ static void *cfq_init_queue(struct request_queue *q) cfqd->oom_cfqq.ref++; cfq_link_cfqq_cfqg(&cfqd->oom_cfqq, &cfqd->root_group); - INIT_LIST_HEAD(&cfqd->cic_list); + INIT_LIST_HEAD(&cfqd->icq_list); cfqd->queue = q; @@ -3942,8 +3960,8 @@ static void cfq_slab_kill(void) */ if (cfq_pool) kmem_cache_destroy(cfq_pool); - if (cfq_ioc_pool) - kmem_cache_destroy(cfq_ioc_pool); + if (cfq_icq_pool) + kmem_cache_destroy(cfq_icq_pool); } static int __init cfq_slab_setup(void) @@ -3952,8 +3970,8 @@ static int __init cfq_slab_setup(void) if (!cfq_pool) goto fail; - cfq_ioc_pool = KMEM_CACHE(cfq_io_context, 0); - if (!cfq_ioc_pool) + cfq_icq_pool = KMEM_CACHE(cfq_io_cq, 0); + if (!cfq_icq_pool) goto fail; return 0; diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h index b2b75a54f252..d15ca6591f96 100644 --- a/include/linux/iocontext.h +++ b/include/linux/iocontext.h @@ -5,38 +5,23 @@ #include #include -struct cfq_queue; -struct cfq_ttime { - unsigned long last_end_request; - - unsigned long ttime_total; - unsigned long ttime_samples; - unsigned long ttime_mean; -}; - enum { - CIC_IOPRIO_CHANGED, - CIC_CGROUP_CHANGED, + ICQ_IOPRIO_CHANGED, + ICQ_CGROUP_CHANGED, }; -struct cfq_io_context { - struct request_queue *q; - - struct cfq_queue *cfqq[2]; - - struct io_context *ioc; - - struct cfq_ttime ttime; - - struct list_head queue_list; - struct hlist_node cic_list; +struct io_cq { + struct request_queue *q; + struct io_context *ioc; - unsigned long changed; + struct list_head q_node; + struct hlist_node ioc_node; - void (*exit)(struct cfq_io_context *); - void (*release)(struct cfq_io_context *); + unsigned long changed; + struct rcu_head rcu_head; - struct rcu_head rcu_head; + void (*exit)(struct io_cq *); + void (*release)(struct io_cq *); }; /* @@ -58,9 +43,9 @@ struct io_context { int nr_batch_requests; /* Number of requests left in the batch */ unsigned long last_waited; /* Time last woken after wait for request */ - struct radix_tree_root radix_root; - struct hlist_head cic_list; - void __rcu *ioc_data; + struct radix_tree_root icq_tree; + struct io_cq __rcu *icq_hint; + struct hlist_head icq_list; struct work_struct release_work; }; -- cgit v1.2.3 From 7e5a8794492e43e9eebb68a98a23be055888ccd0 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 14 Dec 2011 00:33:42 +0100 Subject: block, cfq: move io_cq exit/release to blk-ioc.c With kmem_cache managed by blk-ioc, io_cq exit/release can be moved to blk-ioc too. The odd ->io_cq->exit/release() callbacks are replaced with elevator_ops->elevator_exit_icq_fn() with unlinking from both ioc and q, and freeing automatically handled by blk-ioc. The elevator operation only need to perform exit operation specific to the elevator - in cfq's case, exiting the cfqq's. Also, clearing of io_cq's on q detach is moved to block core and automatically performed on elevator switch and q release. Because the q io_cq points to might be freed before RCU callback for the io_cq runs, blk-ioc code should remember to which cache the io_cq needs to be freed when the io_cq is released. New field io_cq->__rcu_icq_cache is added for this purpose. As both the new field and rcu_head are used only after io_cq is released and the q/ioc_node fields aren't, they are put into unions. Signed-off-by: Tejun Heo Signed-off-by: Jens Axboe --- block/blk-ioc.c | 76 ++++++++++++++++++++++++++++++++++++++++++----- block/blk-sysfs.c | 6 +++- block/blk.h | 1 + block/cfq-iosched.c | 47 ++--------------------------- block/elevator.c | 3 +- include/linux/elevator.h | 5 ++++ include/linux/iocontext.h | 20 +++++++++---- 7 files changed, 97 insertions(+), 61 deletions(-) (limited to 'include/linux/iocontext.h') diff --git a/block/blk-ioc.c b/block/blk-ioc.c index 87ecc98b8ade..0910a5584d38 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -44,6 +44,51 @@ EXPORT_SYMBOL(get_io_context); #define ioc_release_depth_dec(q) do { } while (0) #endif +static void icq_free_icq_rcu(struct rcu_head *head) +{ + struct io_cq *icq = container_of(head, struct io_cq, __rcu_head); + + kmem_cache_free(icq->__rcu_icq_cache, icq); +} + +/* + * Exit and free an icq. Called with both ioc and q locked. + */ +static void ioc_exit_icq(struct io_cq *icq) +{ + struct io_context *ioc = icq->ioc; + struct request_queue *q = icq->q; + struct elevator_type *et = q->elevator->type; + + lockdep_assert_held(&ioc->lock); + lockdep_assert_held(q->queue_lock); + + radix_tree_delete(&ioc->icq_tree, icq->q->id); + hlist_del_init(&icq->ioc_node); + list_del_init(&icq->q_node); + + /* + * Both setting lookup hint to and clearing it from @icq are done + * under queue_lock. If it's not pointing to @icq now, it never + * will. Hint assignment itself can race safely. + */ + if (rcu_dereference_raw(ioc->icq_hint) == icq) + rcu_assign_pointer(ioc->icq_hint, NULL); + + if (et->ops.elevator_exit_icq_fn) { + ioc_release_depth_inc(q); + et->ops.elevator_exit_icq_fn(icq); + ioc_release_depth_dec(q); + } + + /* + * @icq->q might have gone away by the time RCU callback runs + * making it impossible to determine icq_cache. Record it in @icq. + */ + icq->__rcu_icq_cache = et->icq_cache; + call_rcu(&icq->__rcu_head, icq_free_icq_rcu); +} + /* * Slow path for ioc release in put_io_context(). Performs double-lock * dancing to unlink all icq's and then frees ioc. @@ -87,10 +132,7 @@ static void ioc_release_fn(struct work_struct *work) spin_lock(&ioc->lock); continue; } - ioc_release_depth_inc(this_q); - icq->exit(icq); - icq->release(icq); - ioc_release_depth_dec(this_q); + ioc_exit_icq(icq); } if (last_q) { @@ -167,10 +209,7 @@ void put_io_context(struct io_context *ioc, struct request_queue *locked_q) last_q = this_q; continue; } - ioc_release_depth_inc(this_q); - icq->exit(icq); - icq->release(icq); - ioc_release_depth_dec(this_q); + ioc_exit_icq(icq); } if (last_q && last_q != locked_q) @@ -203,6 +242,27 @@ void exit_io_context(struct task_struct *task) put_io_context(ioc, NULL); } +/** + * ioc_clear_queue - break any ioc association with the specified queue + * @q: request_queue being cleared + * + * Walk @q->icq_list and exit all io_cq's. Must be called with @q locked. + */ +void ioc_clear_queue(struct request_queue *q) +{ + lockdep_assert_held(q->queue_lock); + + while (!list_empty(&q->icq_list)) { + struct io_cq *icq = list_entry(q->icq_list.next, + struct io_cq, q_node); + struct io_context *ioc = icq->ioc; + + spin_lock(&ioc->lock); + ioc_exit_icq(icq); + spin_unlock(&ioc->lock); + } +} + void create_io_context_slowpath(struct task_struct *task, gfp_t gfp_flags, int node) { diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index 5b4b4ab5e785..cf150011d808 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -479,8 +479,12 @@ static void blk_release_queue(struct kobject *kobj) blk_sync_queue(q); - if (q->elevator) + if (q->elevator) { + spin_lock_irq(q->queue_lock); + ioc_clear_queue(q); + spin_unlock_irq(q->queue_lock); elevator_exit(q->elevator); + } blk_throtl_exit(q); diff --git a/block/blk.h b/block/blk.h index 3c510a4b5054..ed4d9bf2ab16 100644 --- a/block/blk.h +++ b/block/blk.h @@ -200,6 +200,7 @@ static inline int blk_do_io_stat(struct request *rq) */ void get_io_context(struct io_context *ioc); struct io_cq *ioc_lookup_icq(struct io_context *ioc, struct request_queue *q); +void ioc_clear_queue(struct request_queue *q); void create_io_context_slowpath(struct task_struct *task, gfp_t gfp_mask, int node); diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 06e59abcb57f..f6d315551496 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -2674,26 +2674,6 @@ static void cfq_put_queue(struct cfq_queue *cfqq) cfq_put_cfqg(cfqg); } -static void cfq_icq_free_rcu(struct rcu_head *head) -{ - kmem_cache_free(cfq_icq_pool, - icq_to_cic(container_of(head, struct io_cq, rcu_head))); -} - -static void cfq_icq_free(struct io_cq *icq) -{ - call_rcu(&icq->rcu_head, cfq_icq_free_rcu); -} - -static void cfq_release_icq(struct io_cq *icq) -{ - struct io_context *ioc = icq->ioc; - - radix_tree_delete(&ioc->icq_tree, icq->q->id); - hlist_del(&icq->ioc_node); - cfq_icq_free(icq); -} - static void cfq_put_cooperator(struct cfq_queue *cfqq) { struct cfq_queue *__cfqq, *next; @@ -2731,17 +2711,6 @@ static void cfq_exit_icq(struct io_cq *icq) { struct cfq_io_cq *cic = icq_to_cic(icq); struct cfq_data *cfqd = cic_to_cfqd(cic); - struct io_context *ioc = icq->ioc; - - list_del_init(&icq->q_node); - - /* - * Both setting lookup hint to and clearing it from @icq are done - * under queue_lock. If it's not pointing to @icq now, it never - * will. Hint assignment itself can race safely. - */ - if (rcu_dereference_raw(ioc->icq_hint) == icq) - rcu_assign_pointer(ioc->icq_hint, NULL); if (cic->cfqq[BLK_RW_ASYNC]) { cfq_exit_cfqq(cfqd, cic->cfqq[BLK_RW_ASYNC]); @@ -2764,8 +2733,6 @@ static struct cfq_io_cq *cfq_alloc_cic(struct cfq_data *cfqd, gfp_t gfp_mask) cic->ttime.last_end_request = jiffies; INIT_LIST_HEAD(&cic->icq.q_node); INIT_HLIST_NODE(&cic->icq.ioc_node); - cic->icq.exit = cfq_exit_icq; - cic->icq.release = cfq_release_icq; } return cic; @@ -3034,7 +3001,7 @@ out: if (ret) printk(KERN_ERR "cfq: icq link failed!\n"); if (icq) - cfq_icq_free(icq); + kmem_cache_free(cfq_icq_pool, icq); return ret; } @@ -3774,17 +3741,6 @@ static void cfq_exit_queue(struct elevator_queue *e) if (cfqd->active_queue) __cfq_slice_expired(cfqd, cfqd->active_queue, 0); - while (!list_empty(&q->icq_list)) { - struct io_cq *icq = list_entry(q->icq_list.next, - struct io_cq, q_node); - struct io_context *ioc = icq->ioc; - - spin_lock(&ioc->lock); - cfq_exit_icq(icq); - cfq_release_icq(icq); - spin_unlock(&ioc->lock); - } - cfq_put_async_queues(cfqd); cfq_release_cfq_groups(cfqd); @@ -4019,6 +3975,7 @@ static struct elevator_type iosched_cfq = { .elevator_completed_req_fn = cfq_completed_request, .elevator_former_req_fn = elv_rb_former_request, .elevator_latter_req_fn = elv_rb_latter_request, + .elevator_exit_icq_fn = cfq_exit_icq, .elevator_set_req_fn = cfq_set_request, .elevator_put_req_fn = cfq_put_request, .elevator_may_queue_fn = cfq_may_queue, diff --git a/block/elevator.c b/block/elevator.c index cca049fb45c8..91e18f8af9be 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -979,8 +979,9 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e) goto fail_register; } - /* done, replace the old one with new one and turn off BYPASS */ + /* done, clear io_cq's, switch elevators and turn off BYPASS */ spin_lock_irq(q->queue_lock); + ioc_clear_queue(q); old_elevator = q->elevator; q->elevator = e; spin_unlock_irq(q->queue_lock); diff --git a/include/linux/elevator.h b/include/linux/elevator.h index d3d3e28cbfd4..06e4dd568717 100644 --- a/include/linux/elevator.h +++ b/include/linux/elevator.h @@ -5,6 +5,8 @@ #ifdef CONFIG_BLOCK +struct io_cq; + typedef int (elevator_merge_fn) (struct request_queue *, struct request **, struct bio *); @@ -24,6 +26,7 @@ typedef struct request *(elevator_request_list_fn) (struct request_queue *, stru typedef void (elevator_completed_req_fn) (struct request_queue *, struct request *); typedef int (elevator_may_queue_fn) (struct request_queue *, int); +typedef void (elevator_exit_icq_fn) (struct io_cq *); typedef int (elevator_set_req_fn) (struct request_queue *, struct request *, gfp_t); typedef void (elevator_put_req_fn) (struct request *); typedef void (elevator_activate_req_fn) (struct request_queue *, struct request *); @@ -56,6 +59,8 @@ struct elevator_ops elevator_request_list_fn *elevator_former_req_fn; elevator_request_list_fn *elevator_latter_req_fn; + elevator_exit_icq_fn *elevator_exit_icq_fn; + elevator_set_req_fn *elevator_set_req_fn; elevator_put_req_fn *elevator_put_req_fn; diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h index d15ca6591f96..ac390a34c0e7 100644 --- a/include/linux/iocontext.h +++ b/include/linux/iocontext.h @@ -14,14 +14,22 @@ struct io_cq { struct request_queue *q; struct io_context *ioc; - struct list_head q_node; - struct hlist_node ioc_node; + /* + * q_node and ioc_node link io_cq through icq_list of q and ioc + * respectively. Both fields are unused once ioc_exit_icq() is + * called and shared with __rcu_icq_cache and __rcu_head which are + * used for RCU free of io_cq. + */ + union { + struct list_head q_node; + struct kmem_cache *__rcu_icq_cache; + }; + union { + struct hlist_node ioc_node; + struct rcu_head __rcu_head; + }; unsigned long changed; - struct rcu_head rcu_head; - - void (*exit)(struct io_cq *); - void (*release)(struct io_cq *); }; /* -- cgit v1.2.3 From f1f8cc94651738b418ba54c039df536303b91704 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 14 Dec 2011 00:33:42 +0100 Subject: block, cfq: move icq creation and rq->elv.icq association to block core Now block layer knows everything necessary to create and associate icq's with requests. Move ioc_create_icq() to blk-ioc.c and update get_request() such that, if elevator_type->icq_size is set, requests are automatically associated with their matching icq's before elv_set_request(). io_context reference is also managed by block core on request alloc/free. * Only ioprio/cgroup changed handling remains from cfq_get_cic(). Collapsed into cfq_set_request(). * This removes queue kicking on icq allocation failure (for now). As icq allocation failure is rare and the only effect of queue kicking achieved was possibily accelerating queue processing, this change shouldn't be noticeable. There is a larger underlying problem. Unlike request allocation, icq allocation is not guaranteed to succeed eventually after retries. The number of icq is unbound and thus mempool can't be the solution either. This effectively adds allocation dependency on memory free path and thus possibility of deadlock. This usually wouldn't happen because icq allocation is not a hot path and, even when the condition triggers, it's highly unlikely that none of the writeback workers already has icq. However, this is still possible especially if elevator is being switched under high memory pressure, so we better get it fixed. Probably the only solution is just bypassing elevator and appending to dispatch queue on any elevator allocation failure. * Comment added to explain how icq's are managed and synchronized. This completes cleanup of io_context interface. Signed-off-by: Tejun Heo Signed-off-by: Jens Axboe --- block/blk-core.c | 46 +++++++++++++--- block/blk-ioc.c | 60 ++++++++++++++++++++- block/blk.h | 1 + block/cfq-iosched.c | 135 ++++------------------------------------------ include/linux/elevator.h | 8 +-- include/linux/iocontext.h | 59 ++++++++++++++++++++ 6 files changed, 173 insertions(+), 136 deletions(-) (limited to 'include/linux/iocontext.h') diff --git a/block/blk-core.c b/block/blk-core.c index 3c26c7f48703..8fbdac7010bb 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -640,13 +640,18 @@ EXPORT_SYMBOL(blk_get_queue); static inline void blk_free_request(struct request_queue *q, struct request *rq) { - if (rq->cmd_flags & REQ_ELVPRIV) + if (rq->cmd_flags & REQ_ELVPRIV) { elv_put_request(q, rq); + if (rq->elv.icq) + put_io_context(rq->elv.icq->ioc, q); + } + mempool_free(rq, q->rq.rq_pool); } static struct request * -blk_alloc_request(struct request_queue *q, unsigned int flags, gfp_t gfp_mask) +blk_alloc_request(struct request_queue *q, struct io_cq *icq, + unsigned int flags, gfp_t gfp_mask) { struct request *rq = mempool_alloc(q->rq.rq_pool, gfp_mask); @@ -657,10 +662,15 @@ blk_alloc_request(struct request_queue *q, unsigned int flags, gfp_t gfp_mask) rq->cmd_flags = flags | REQ_ALLOCED; - if ((flags & REQ_ELVPRIV) && - unlikely(elv_set_request(q, rq, gfp_mask))) { - mempool_free(rq, q->rq.rq_pool); - return NULL; + if (flags & REQ_ELVPRIV) { + rq->elv.icq = icq; + if (unlikely(elv_set_request(q, rq, gfp_mask))) { + mempool_free(rq, q->rq.rq_pool); + return NULL; + } + /* @rq->elv.icq holds on to io_context until @rq is freed */ + if (icq) + get_io_context(icq->ioc); } return rq; @@ -772,11 +782,14 @@ static struct request *get_request(struct request_queue *q, int rw_flags, { struct request *rq = NULL; struct request_list *rl = &q->rq; + struct elevator_type *et; struct io_context *ioc; + struct io_cq *icq = NULL; const bool is_sync = rw_is_sync(rw_flags) != 0; bool retried = false; int may_queue; retry: + et = q->elevator->type; ioc = current->io_context; if (unlikely(blk_queue_dead(q))) @@ -837,17 +850,36 @@ retry: rl->count[is_sync]++; rl->starved[is_sync] = 0; + /* + * Decide whether the new request will be managed by elevator. If + * so, mark @rw_flags and increment elvpriv. Non-zero elvpriv will + * prevent the current elevator from being destroyed until the new + * request is freed. This guarantees icq's won't be destroyed and + * makes creating new ones safe. + * + * Also, lookup icq while holding queue_lock. If it doesn't exist, + * it will be created after releasing queue_lock. + */ if (blk_rq_should_init_elevator(bio) && !test_bit(QUEUE_FLAG_ELVSWITCH, &q->queue_flags)) { rw_flags |= REQ_ELVPRIV; rl->elvpriv++; + if (et->icq_cache && ioc) + icq = ioc_lookup_icq(ioc, q); } if (blk_queue_io_stat(q)) rw_flags |= REQ_IO_STAT; spin_unlock_irq(q->queue_lock); - rq = blk_alloc_request(q, rw_flags, gfp_mask); + /* create icq if missing */ + if (unlikely(et->icq_cache && !icq)) + icq = ioc_create_icq(q, gfp_mask); + + /* rqs are guaranteed to have icq on elv_set_request() if requested */ + if (likely(!et->icq_cache || icq)) + rq = blk_alloc_request(q, icq, rw_flags, gfp_mask); + if (unlikely(!rq)) { /* * Allocation failed presumably due to memory. Undo anything diff --git a/block/blk-ioc.c b/block/blk-ioc.c index 0910a5584d38..c04d16b02225 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -289,7 +289,6 @@ void create_io_context_slowpath(struct task_struct *task, gfp_t gfp_flags, kmem_cache_free(iocontext_cachep, ioc); task_unlock(task); } -EXPORT_SYMBOL(create_io_context_slowpath); /** * get_task_io_context - get io_context of a task @@ -362,6 +361,65 @@ out: } EXPORT_SYMBOL(ioc_lookup_icq); +/** + * ioc_create_icq - create and link io_cq + * @q: request_queue of interest + * @gfp_mask: allocation mask + * + * Make sure io_cq linking %current->io_context and @q exists. If either + * io_context and/or icq don't exist, they will be created using @gfp_mask. + * + * The caller is responsible for ensuring @ioc won't go away and @q is + * alive and will stay alive until this function returns. + */ +struct io_cq *ioc_create_icq(struct request_queue *q, gfp_t gfp_mask) +{ + struct elevator_type *et = q->elevator->type; + struct io_context *ioc; + struct io_cq *icq; + + /* allocate stuff */ + ioc = create_io_context(current, gfp_mask, q->node); + if (!ioc) + return NULL; + + icq = kmem_cache_alloc_node(et->icq_cache, gfp_mask | __GFP_ZERO, + q->node); + if (!icq) + return NULL; + + if (radix_tree_preload(gfp_mask) < 0) { + kmem_cache_free(et->icq_cache, icq); + return NULL; + } + + icq->ioc = ioc; + icq->q = q; + INIT_LIST_HEAD(&icq->q_node); + INIT_HLIST_NODE(&icq->ioc_node); + + /* lock both q and ioc and try to link @icq */ + spin_lock_irq(q->queue_lock); + spin_lock(&ioc->lock); + + if (likely(!radix_tree_insert(&ioc->icq_tree, q->id, icq))) { + hlist_add_head(&icq->ioc_node, &ioc->icq_list); + list_add(&icq->q_node, &q->icq_list); + if (et->ops.elevator_init_icq_fn) + et->ops.elevator_init_icq_fn(icq); + } else { + kmem_cache_free(et->icq_cache, icq); + icq = ioc_lookup_icq(ioc, q); + if (!icq) + printk(KERN_ERR "cfq: icq link failed!\n"); + } + + spin_unlock(&ioc->lock); + spin_unlock_irq(q->queue_lock); + radix_tree_preload_end(); + return icq; +} + void ioc_set_changed(struct io_context *ioc, int which) { struct io_cq *icq; diff --git a/block/blk.h b/block/blk.h index ed4d9bf2ab16..7efd772336de 100644 --- a/block/blk.h +++ b/block/blk.h @@ -200,6 +200,7 @@ static inline int blk_do_io_stat(struct request *rq) */ void get_io_context(struct io_context *ioc); struct io_cq *ioc_lookup_icq(struct io_context *ioc, struct request_queue *q); +struct io_cq *ioc_create_icq(struct request_queue *q, gfp_t gfp_mask); void ioc_clear_queue(struct request_queue *q); void create_io_context_slowpath(struct task_struct *task, gfp_t gfp_mask, diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 11f49d036845..f3b44c394e6d 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -2935,117 +2935,6 @@ cfq_get_queue(struct cfq_data *cfqd, bool is_sync, struct io_context *ioc, return cfqq; } -/** - * ioc_create_icq - create and link io_cq - * @q: request_queue of interest - * @gfp_mask: allocation mask - * - * Make sure io_cq linking %current->io_context and @q exists. If either - * io_context and/or icq don't exist, they will be created using @gfp_mask. - * - * The caller is responsible for ensuring @ioc won't go away and @q is - * alive and will stay alive until this function returns. - */ -static struct io_cq *ioc_create_icq(struct request_queue *q, gfp_t gfp_mask) -{ - struct elevator_type *et = q->elevator->type; - struct io_context *ioc; - struct io_cq *icq; - - /* allocate stuff */ - ioc = create_io_context(current, gfp_mask, q->node); - if (!ioc) - return NULL; - - icq = kmem_cache_alloc_node(et->icq_cache, gfp_mask | __GFP_ZERO, - q->node); - if (!icq) - return NULL; - - if (radix_tree_preload(gfp_mask) < 0) { - kmem_cache_free(et->icq_cache, icq); - return NULL; - } - - icq->ioc = ioc; - icq->q = q; - INIT_LIST_HEAD(&icq->q_node); - INIT_HLIST_NODE(&icq->ioc_node); - - /* lock both q and ioc and try to link @icq */ - spin_lock_irq(q->queue_lock); - spin_lock(&ioc->lock); - - if (likely(!radix_tree_insert(&ioc->icq_tree, q->id, icq))) { - hlist_add_head(&icq->ioc_node, &ioc->icq_list); - list_add(&icq->q_node, &q->icq_list); - if (et->ops.elevator_init_icq_fn) - et->ops.elevator_init_icq_fn(icq); - } else { - kmem_cache_free(et->icq_cache, icq); - icq = ioc_lookup_icq(ioc, q); - if (!icq) - printk(KERN_ERR "cfq: icq link failed!\n"); - } - - spin_unlock(&ioc->lock); - spin_unlock_irq(q->queue_lock); - radix_tree_preload_end(); - return icq; -} - -/** - * cfq_get_cic - acquire cfq_io_cq and bump refcnt on io_context - * @cfqd: cfqd to setup cic for - * @gfp_mask: allocation mask - * - * Return cfq_io_cq associating @cfqd and %current->io_context and - * bump refcnt on io_context. If ioc or cic doesn't exist, they're created - * using @gfp_mask. - * - * Must be called under queue_lock which may be released and re-acquired. - * This function also may sleep depending on @gfp_mask. - */ -static struct cfq_io_cq *cfq_get_cic(struct cfq_data *cfqd, gfp_t gfp_mask) -{ - struct request_queue *q = cfqd->queue; - struct cfq_io_cq *cic = NULL; - struct io_context *ioc; - - lockdep_assert_held(q->queue_lock); - - while (true) { - /* fast path */ - ioc = current->io_context; - if (likely(ioc)) { - cic = cfq_cic_lookup(cfqd, ioc); - if (likely(cic)) - break; - } - - /* slow path - unlock, create missing ones and retry */ - spin_unlock_irq(q->queue_lock); - cic = icq_to_cic(ioc_create_icq(q, gfp_mask)); - spin_lock_irq(q->queue_lock); - if (!cic) - return NULL; - } - - /* bump @ioc's refcnt and handle changed notifications */ - get_io_context(ioc); - - if (unlikely(cic->icq.changed)) { - if (test_and_clear_bit(ICQ_IOPRIO_CHANGED, &cic->icq.changed)) - changed_ioprio(cic); -#ifdef CONFIG_CFQ_GROUP_IOSCHED - if (test_and_clear_bit(ICQ_CGROUP_CHANGED, &cic->icq.changed)) - changed_cgroup(cic); -#endif - } - - return cic; -} - static void __cfq_update_io_thinktime(struct cfq_ttime *ttime, unsigned long slice_idle) { @@ -3524,8 +3413,6 @@ static void cfq_put_request(struct request *rq) BUG_ON(!cfqq->allocated[rw]); cfqq->allocated[rw]--; - put_io_context(RQ_CIC(rq)->icq.ioc, cfqq->cfqd->queue); - /* Put down rq reference on cfqg */ cfq_put_cfqg(RQ_CFQG(rq)); rq->elv.priv[0] = NULL; @@ -3574,7 +3461,7 @@ static int cfq_set_request(struct request_queue *q, struct request *rq, gfp_t gfp_mask) { struct cfq_data *cfqd = q->elevator->elevator_data; - struct cfq_io_cq *cic; + struct cfq_io_cq *cic = icq_to_cic(rq->elv.icq); const int rw = rq_data_dir(rq); const bool is_sync = rq_is_sync(rq); struct cfq_queue *cfqq; @@ -3582,9 +3469,16 @@ cfq_set_request(struct request_queue *q, struct request *rq, gfp_t gfp_mask) might_sleep_if(gfp_mask & __GFP_WAIT); spin_lock_irq(q->queue_lock); - cic = cfq_get_cic(cfqd, gfp_mask); - if (!cic) - goto queue_fail; + + /* handle changed notifications */ + if (unlikely(cic->icq.changed)) { + if (test_and_clear_bit(ICQ_IOPRIO_CHANGED, &cic->icq.changed)) + changed_ioprio(cic); +#ifdef CONFIG_CFQ_GROUP_IOSCHED + if (test_and_clear_bit(ICQ_CGROUP_CHANGED, &cic->icq.changed)) + changed_cgroup(cic); +#endif + } new_queue: cfqq = cic_to_cfqq(cic, is_sync); @@ -3615,17 +3509,10 @@ new_queue: cfqq->allocated[rw]++; cfqq->ref++; - rq->elv.icq = &cic->icq; rq->elv.priv[0] = cfqq; rq->elv.priv[1] = cfq_ref_get_cfqg(cfqq->cfqg); spin_unlock_irq(q->queue_lock); return 0; - -queue_fail: - cfq_schedule_dispatch(cfqd); - spin_unlock_irq(q->queue_lock); - cfq_log(cfqd, "set_request fail"); - return 1; } static void cfq_kick_queue(struct work_struct *work) diff --git a/include/linux/elevator.h b/include/linux/elevator.h index c8f1e67a8ebe..c24f3d7fbf1e 100644 --- a/include/linux/elevator.h +++ b/include/linux/elevator.h @@ -60,8 +60,8 @@ struct elevator_ops elevator_request_list_fn *elevator_former_req_fn; elevator_request_list_fn *elevator_latter_req_fn; - elevator_init_icq_fn *elevator_init_icq_fn; - elevator_exit_icq_fn *elevator_exit_icq_fn; + elevator_init_icq_fn *elevator_init_icq_fn; /* see iocontext.h */ + elevator_exit_icq_fn *elevator_exit_icq_fn; /* ditto */ elevator_set_req_fn *elevator_set_req_fn; elevator_put_req_fn *elevator_put_req_fn; @@ -90,8 +90,8 @@ struct elevator_type /* fields provided by elevator implementation */ struct elevator_ops ops; - size_t icq_size; - size_t icq_align; + size_t icq_size; /* see iocontext.h */ + size_t icq_align; /* ditto */ struct elv_fs_entry *elevator_attrs; char elevator_name[ELV_NAME_MAX]; struct module *elevator_owner; diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h index ac390a34c0e7..7e1371c4bccf 100644 --- a/include/linux/iocontext.h +++ b/include/linux/iocontext.h @@ -10,6 +10,65 @@ enum { ICQ_CGROUP_CHANGED, }; +/* + * An io_cq (icq) is association between an io_context (ioc) and a + * request_queue (q). This is used by elevators which need to track + * information per ioc - q pair. + * + * Elevator can request use of icq by setting elevator_type->icq_size and + * ->icq_align. Both size and align must be larger than that of struct + * io_cq and elevator can use the tail area for private information. The + * recommended way to do this is defining a struct which contains io_cq as + * the first member followed by private members and using its size and + * align. For example, + * + * struct snail_io_cq { + * struct io_cq icq; + * int poke_snail; + * int feed_snail; + * }; + * + * struct elevator_type snail_elv_type { + * .ops = { ... }, + * .icq_size = sizeof(struct snail_io_cq), + * .icq_align = __alignof__(struct snail_io_cq), + * ... + * }; + * + * If icq_size is set, block core will manage icq's. All requests will + * have its ->elv.icq field set before elevator_ops->elevator_set_req_fn() + * is called and be holding a reference to the associated io_context. + * + * Whenever a new icq is created, elevator_ops->elevator_init_icq_fn() is + * called and, on destruction, ->elevator_exit_icq_fn(). Both functions + * are called with both the associated io_context and queue locks held. + * + * Elevator is allowed to lookup icq using ioc_lookup_icq() while holding + * queue lock but the returned icq is valid only until the queue lock is + * released. Elevators can not and should not try to create or destroy + * icq's. + * + * As icq's are linked from both ioc and q, the locking rules are a bit + * complex. + * + * - ioc lock nests inside q lock. + * + * - ioc->icq_list and icq->ioc_node are protected by ioc lock. + * q->icq_list and icq->q_node by q lock. + * + * - ioc->icq_tree and ioc->icq_hint are protected by ioc lock, while icq + * itself is protected by q lock. However, both the indexes and icq + * itself are also RCU managed and lookup can be performed holding only + * the q lock. + * + * - icq's are not reference counted. They are destroyed when either the + * ioc or q goes away. Each request with icq set holds an extra + * reference to ioc to ensure it stays until the request is completed. + * + * - Linking and unlinking icq's are performed while holding both ioc and q + * locks. Due to the lock ordering, q exit is simple but ioc exit + * requires reverse-order double lock dance. + */ struct io_cq { struct request_queue *q; struct io_context *ioc; -- cgit v1.2.3