* [PATCH 01/10] block, cfq: replace current_io_context() with create_io_context()
2011-10-29 23:49 [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
@ 2011-10-29 23:49 ` Tejun Heo
2011-10-29 23:49 ` [PATCH 02/10] block: reorder elevator switch sequence Tejun Heo
` (9 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2011-10-29 23:49 UTC (permalink / raw)
To: axboe, vgoyal; +Cc: ctalbott, rni, linux-kernel, Tejun Heo
When called under queue_lock, current_io_context() triggers lockdep
warning if it hits allocation path. This is because io_context
installation is protected by task_lock which is not IRQ safe, so it
triggers irq-unsafe-lock -> irq -> irq-safe-lock -> irq-unsafe-lock
deadlock warning.
Given the restriction, accessor + creator rolled into one doesn't work
too well. Drop current_io_context() and let the users access
task->io_context directly inside queue_lock combined with explicit
creation using create_io_context().
Future ioc updates will further consolidate ioc access and the create
interface will be unexported.
While at it, relocate ioc internal interface declarations in blk.h and
add section comments before and after.
This patch does not introduce functional change.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
block/blk-core.c | 25 ++++++++++++++++----
block/blk-ioc.c | 62 ++++++++++++++-------------------------------------
block/blk.h | 36 +++++++++++++++++++++++++++--
block/cfq-iosched.c | 2 +-
4 files changed, 71 insertions(+), 54 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 39f1289..74b44ab 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -766,9 +766,12 @@ static struct request *get_request(struct request_queue *q, int rw_flags,
{
struct request *rq = NULL;
struct request_list *rl = &q->rq;
- struct io_context *ioc = NULL;
+ struct io_context *ioc;
const bool is_sync = rw_is_sync(rw_flags) != 0;
+ bool retried = false;
int may_queue;
+retry:
+ ioc = current->io_context;
if (unlikely(blk_queue_dead(q)))
return NULL;
@@ -779,7 +782,20 @@ static struct request *get_request(struct request_queue *q, int rw_flags,
if (rl->count[is_sync]+1 >= queue_congestion_on_threshold(q)) {
if (rl->count[is_sync]+1 >= q->nr_requests) {
- ioc = current_io_context(GFP_ATOMIC, q->node);
+ /*
+ * We want ioc to record batching state. If it's
+ * not already there, creating a new one requires
+ * dropping queue_lock, which in turn requires
+ * retesting conditions to avoid queue hang.
+ */
+ if (!ioc && !retried) {
+ spin_unlock_irq(q->queue_lock);
+ create_io_context(current, gfp_mask, q->node);
+ spin_lock_irq(q->queue_lock);
+ retried = true;
+ goto retry;
+ }
+
/*
* The queue will fill after this allocation, so set
* it as full, and mark this process as "batching".
@@ -887,7 +903,6 @@ static struct request *get_request_wait(struct request_queue *q, int rw_flags,
rq = get_request(q, rw_flags, bio, GFP_NOIO);
while (!rq) {
DEFINE_WAIT(wait);
- struct io_context *ioc;
struct request_list *rl = &q->rq;
if (unlikely(blk_queue_dead(q)))
@@ -907,8 +922,8 @@ static struct request *get_request_wait(struct request_queue *q, int rw_flags,
* up to a big batch of them for a small period time.
* See ioc_batching, ioc_set_batching
*/
- ioc = current_io_context(GFP_NOIO, q->node);
- ioc_set_batching(q, ioc);
+ create_io_context(current, GFP_NOIO, q->node);
+ ioc_set_batching(q, current->io_context);
spin_lock_irq(q->queue_lock);
finish_wait(&rl->wait[is_sync], &wait);
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index fb23965..e23c797 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -205,16 +205,15 @@ void exit_io_context(struct task_struct *task)
put_io_context(ioc, NULL);
}
-static struct io_context *create_task_io_context(struct task_struct *task,
- gfp_t gfp_flags, int node,
- bool take_ref)
+void create_io_context_slowpath(struct task_struct *task, gfp_t gfp_flags,
+ int node)
{
struct io_context *ioc;
ioc = kmem_cache_alloc_node(iocontext_cachep, gfp_flags | __GFP_ZERO,
node);
if (unlikely(!ioc))
- return NULL;
+ return;
/* initialize */
atomic_long_set(&ioc->refcount, 1);
@@ -226,42 +225,13 @@ static struct io_context *create_task_io_context(struct task_struct *task,
/* try to install, somebody might already have beaten us to it */
task_lock(task);
-
- if (!task->io_context && !(task->flags & PF_EXITING)) {
+ if (!task->io_context && !(task->flags & PF_EXITING))
task->io_context = ioc;
- } else {
+ else
kmem_cache_free(iocontext_cachep, ioc);
- ioc = task->io_context;
- }
-
- if (ioc && take_ref)
- get_io_context(ioc);
-
task_unlock(task);
- return ioc;
}
-
-/**
- * current_io_context - get io_context of %current
- * @gfp_flags: allocation flags, used if allocation is necessary
- * @node: allocation node, used if allocation is necessary
- *
- * Return io_context of %current. If it doesn't exist, it is created with
- * @gfp_flags and @node. The returned io_context does NOT have its
- * reference count incremented. Because io_context is exited only on task
- * exit, %current can be sure that the returned io_context is valid and
- * alive as long as it is executing.
- */
-struct io_context *current_io_context(gfp_t gfp_flags, int node)
-{
- might_sleep_if(gfp_flags & __GFP_WAIT);
-
- if (current->io_context)
- return current->io_context;
-
- return create_task_io_context(current, gfp_flags, node, false);
-}
-EXPORT_SYMBOL(current_io_context);
+EXPORT_SYMBOL(create_io_context_slowpath);
/**
* get_task_io_context - get io_context of a task
@@ -274,7 +244,7 @@ EXPORT_SYMBOL(current_io_context);
* incremented.
*
* This function always goes through task_lock() and it's better to use
- * current_io_context() + get_io_context() for %current.
+ * %current->io_context + get_io_context() for %current.
*/
struct io_context *get_task_io_context(struct task_struct *task,
gfp_t gfp_flags, int node)
@@ -283,16 +253,18 @@ struct io_context *get_task_io_context(struct task_struct *task,
might_sleep_if(gfp_flags & __GFP_WAIT);
- task_lock(task);
- ioc = task->io_context;
- if (likely(ioc)) {
- get_io_context(ioc);
+ do {
+ task_lock(task);
+ ioc = task->io_context;
+ if (likely(ioc)) {
+ get_io_context(ioc);
+ task_unlock(task);
+ return ioc;
+ }
task_unlock(task);
- return ioc;
- }
- task_unlock(task);
+ } while (create_io_context(task, gfp_flags, node));
- return create_task_io_context(task, gfp_flags, node, true);
+ return NULL;
}
EXPORT_SYMBOL(get_task_io_context);
diff --git a/block/blk.h b/block/blk.h
index 8d42115..5bca266 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -127,9 +127,6 @@ 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,
struct bio *bio);
int ll_front_merge_fn(struct request_queue *q, struct request *req,
@@ -198,6 +195,39 @@ static inline int blk_do_io_stat(struct request *rq)
(rq->cmd_flags & REQ_DISCARD));
}
+/*
+ * Internal io_context interface
+ */
+void get_io_context(struct io_context *ioc);
+
+void create_io_context_slowpath(struct task_struct *task, gfp_t gfp_mask,
+ int node);
+
+/**
+ * create_io_context - try to create task->io_context
+ * @task: target task
+ * @gfp_mask: allocation mask
+ * @node: allocation node
+ *
+ * If @task->io_context is %NULL, allocate a new io_context and install it.
+ * Returns the current @task->io_context which may be %NULL if allocation
+ * failed.
+ *
+ * Note that this function can't be called with IRQ disabled because
+ * task_lock which protects @task->io_context is IRQ-unsafe.
+ */
+static inline struct io_context *create_io_context(struct task_struct *task,
+ gfp_t gfp_mask, int node)
+{
+ WARN_ON_ONCE(irqs_disabled());
+ if (unlikely(!task->io_context))
+ create_io_context_slowpath(task, gfp_mask, node);
+ return task->io_context;
+}
+
+/*
+ * Internal throttling interface
+ */
#ifdef CONFIG_BLK_DEV_THROTTLING
extern bool blk_throtl_bio(struct request_queue *q, struct bio *bio);
extern void blk_throtl_drain(struct request_queue *q);
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 3b07ce1..5f7e4d1 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -3012,7 +3012,7 @@ static int cfq_create_cic(struct cfq_data *cfqd, gfp_t gfp_mask)
might_sleep_if(gfp_mask & __GFP_WAIT);
/* allocate stuff */
- ioc = current_io_context(gfp_mask, q->node);
+ ioc = create_io_context(current, gfp_mask, q->node);
if (!ioc)
goto out;
--
1.7.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 02/10] block: reorder elevator switch sequence
2011-10-29 23:49 [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
2011-10-29 23:49 ` [PATCH 01/10] block, cfq: replace current_io_context() with create_io_context() Tejun Heo
@ 2011-10-29 23:49 ` Tejun Heo
2011-10-29 23:49 ` [PATCH 03/10] block: remove elevator_queue->ops Tejun Heo
` (8 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2011-10-29 23:49 UTC (permalink / raw)
To: axboe, vgoyal; +Cc: ctalbott, rni, linux-kernel, Tejun Heo
Elevator switch sequence first attached the new elevator, then tried
registering it (sysfs) and if that failed attached back the old
elevator. However, sysfs registration doesn't require the elevator to
be attached, so there is no reason to do the "detach, attach new,
register, maybe re-attach old" sequence. It can just do "register,
detach, attach".
* elevator_init_queue() is updated to set ->elevator_data directly and
return 0 / -errno. This allows elevator_exit() on an unattached
elevator.
* __elv_unregister_queue() which was necessary to unregister
unattached q is removed in favor of __elv_register_queue() which can
register unattached q.
* elevator_attach() becomes a single assignment and obscures more then
it helps. Dropped.
This will help cleaning up io_context handling across elevator switch.
This patch doesn't introduce visible behavior change.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
block/elevator.c | 91 ++++++++++++++++++++++--------------------------------
1 files changed, 37 insertions(+), 54 deletions(-)
diff --git a/block/elevator.c b/block/elevator.c
index 6a343e8..a16c2d1 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -168,17 +168,13 @@ static struct elevator_type *elevator_get(const char *name)
return e;
}
-static void *elevator_init_queue(struct request_queue *q,
- struct elevator_queue *eq)
+static int elevator_init_queue(struct request_queue *q,
+ struct elevator_queue *eq)
{
- return eq->ops->elevator_init_fn(q);
-}
-
-static void elevator_attach(struct request_queue *q, struct elevator_queue *eq,
- void *data)
-{
- q->elevator = eq;
- eq->elevator_data = data;
+ eq->elevator_data = eq->ops->elevator_init_fn(q);
+ if (eq->elevator_data)
+ return 0;
+ return -ENOMEM;
}
static char chosen_elevator[ELV_NAME_MAX];
@@ -241,7 +237,7 @@ int elevator_init(struct request_queue *q, char *name)
{
struct elevator_type *e = NULL;
struct elevator_queue *eq;
- void *data;
+ int err;
if (unlikely(q->elevator))
return 0;
@@ -278,13 +274,13 @@ int elevator_init(struct request_queue *q, char *name)
if (!eq)
return -ENOMEM;
- data = elevator_init_queue(q, eq);
- if (!data) {
+ err = elevator_init_queue(q, eq);
+ if (err) {
kobject_put(&eq->kobj);
- return -ENOMEM;
+ return err;
}
- elevator_attach(q, eq, data);
+ q->elevator = eq;
return 0;
}
EXPORT_SYMBOL(elevator_init);
@@ -856,9 +852,8 @@ static struct kobj_type elv_ktype = {
.release = elevator_release,
};
-int elv_register_queue(struct request_queue *q)
+int __elv_register_queue(struct request_queue *q, struct elevator_queue *e)
{
- struct elevator_queue *e = q->elevator;
int error;
error = kobject_add(&e->kobj, &q->kobj, "%s", "iosched");
@@ -876,19 +871,22 @@ int elv_register_queue(struct request_queue *q)
}
return error;
}
-EXPORT_SYMBOL(elv_register_queue);
-static void __elv_unregister_queue(struct elevator_queue *e)
+int elv_register_queue(struct request_queue *q)
{
- kobject_uevent(&e->kobj, KOBJ_REMOVE);
- kobject_del(&e->kobj);
- e->registered = 0;
+ return __elv_register_queue(q, q->elevator);
}
+EXPORT_SYMBOL(elv_register_queue);
void elv_unregister_queue(struct request_queue *q)
{
- if (q)
- __elv_unregister_queue(q->elevator);
+ if (q) {
+ struct elevator_queue *e = q->elevator;
+
+ kobject_uevent(&e->kobj, KOBJ_REMOVE);
+ kobject_del(&e->kobj);
+ e->registered = 0;
+ }
}
EXPORT_SYMBOL(elv_unregister_queue);
@@ -928,50 +926,36 @@ EXPORT_SYMBOL_GPL(elv_unregister);
static int elevator_switch(struct request_queue *q, struct elevator_type *new_e)
{
struct elevator_queue *old_elevator, *e;
- void *data;
int err;
- /*
- * Allocate new elevator
- */
+ /* allocate new elevator */
e = elevator_alloc(q, new_e);
if (!e)
return -ENOMEM;
- data = elevator_init_queue(q, e);
- if (!data) {
+ err = elevator_init_queue(q, e);
+ if (err) {
kobject_put(&e->kobj);
- return -ENOMEM;
+ return err;
}
- /*
- * Turn on BYPASS and drain all requests w/ elevator private data
- */
+ /* turn on BYPASS and drain all requests w/ elevator private data */
elv_quiesce_start(q);
- /*
- * Remember old elevator.
- */
- old_elevator = q->elevator;
-
- /*
- * attach and start new elevator
- */
- spin_lock_irq(q->queue_lock);
- elevator_attach(q, e, data);
- spin_unlock_irq(q->queue_lock);
-
- if (old_elevator->registered) {
- __elv_unregister_queue(old_elevator);
-
- err = elv_register_queue(q);
+ /* unregister old queue, register new one and kill old elevator */
+ if (q->elevator->registered) {
+ elv_unregister_queue(q);
+ err = __elv_register_queue(q, e);
if (err)
goto fail_register;
}
- /*
- * finally exit old elevator and turn off BYPASS.
- */
+ /* done, replace the old one with new one and turn off BYPASS */
+ spin_lock_irq(q->queue_lock);
+ old_elevator = q->elevator;
+ q->elevator = e;
+ spin_unlock_irq(q->queue_lock);
+
elevator_exit(old_elevator);
elv_quiesce_end(q);
@@ -985,7 +969,6 @@ fail_register:
* one again (along with re-adding the sysfs dir)
*/
elevator_exit(e);
- q->elevator = old_elevator;
elv_register_queue(q);
elv_quiesce_end(q);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 03/10] block: remove elevator_queue->ops
2011-10-29 23:49 [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
2011-10-29 23:49 ` [PATCH 01/10] block, cfq: replace current_io_context() with create_io_context() Tejun Heo
2011-10-29 23:49 ` [PATCH 02/10] block: reorder elevator switch sequence Tejun Heo
@ 2011-10-29 23:49 ` Tejun Heo
2011-10-29 23:49 ` [PATCH 04/10] block, cfq: reorganize cfq_io_context into generic and cfq specific parts Tejun Heo
` (7 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2011-10-29 23:49 UTC (permalink / raw)
To: axboe, vgoyal; +Cc: ctalbott, rni, linux-kernel, Tejun Heo
elevator_queue->ops points to the same ops struct ->elevator_type.ops
is pointing to. The only effect of caching it in elevator_queue is
shorter notation - it doesn't save any indirect derefence.
Relocate elevator_type->list which used only during module init/exit
to the end of the structure, rename elevator_queue->elevator_type to
->type, and replace elevator_queue->ops with elevator_queue->type.ops.
This doesn't introduce any functional difference.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
block/blk.h | 10 +++---
block/elevator.c | 74 ++++++++++++++++++++++-----------------------
include/linux/elevator.h | 5 +--
3 files changed, 43 insertions(+), 46 deletions(-)
diff --git a/block/blk.h b/block/blk.h
index 5bca266..4943770 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -94,7 +94,7 @@ static inline struct request *__elv_next_request(struct request_queue *q)
return NULL;
}
if (unlikely(blk_queue_dead(q)) ||
- !q->elevator->ops->elevator_dispatch_fn(q, 0))
+ !q->elevator->type->ops.elevator_dispatch_fn(q, 0))
return NULL;
}
}
@@ -103,16 +103,16 @@ static inline void elv_activate_rq(struct request_queue *q, struct request *rq)
{
struct elevator_queue *e = q->elevator;
- if (e->ops->elevator_activate_req_fn)
- e->ops->elevator_activate_req_fn(q, rq);
+ if (e->type->ops.elevator_activate_req_fn)
+ e->type->ops.elevator_activate_req_fn(q, rq);
}
static inline void elv_deactivate_rq(struct request_queue *q, struct request *rq)
{
struct elevator_queue *e = q->elevator;
- if (e->ops->elevator_deactivate_req_fn)
- e->ops->elevator_deactivate_req_fn(q, rq);
+ if (e->type->ops.elevator_deactivate_req_fn)
+ e->type->ops.elevator_deactivate_req_fn(q, rq);
}
#ifdef CONFIG_FAIL_IO_TIMEOUT
diff --git a/block/elevator.c b/block/elevator.c
index a16c2d1..31ffe76 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -61,8 +61,8 @@ static int elv_iosched_allow_merge(struct request *rq, struct bio *bio)
struct request_queue *q = rq->q;
struct elevator_queue *e = q->elevator;
- if (e->ops->elevator_allow_merge_fn)
- return e->ops->elevator_allow_merge_fn(q, rq, bio);
+ if (e->type->ops.elevator_allow_merge_fn)
+ return e->type->ops.elevator_allow_merge_fn(q, rq, bio);
return 1;
}
@@ -171,7 +171,7 @@ static struct elevator_type *elevator_get(const char *name)
static int elevator_init_queue(struct request_queue *q,
struct elevator_queue *eq)
{
- eq->elevator_data = eq->ops->elevator_init_fn(q);
+ eq->elevator_data = eq->type->ops.elevator_init_fn(q);
if (eq->elevator_data)
return 0;
return -ENOMEM;
@@ -203,8 +203,7 @@ static struct elevator_queue *elevator_alloc(struct request_queue *q,
if (unlikely(!eq))
goto err;
- eq->ops = &e->ops;
- eq->elevator_type = e;
+ eq->type = e;
kobject_init(&eq->kobj, &elv_ktype);
mutex_init(&eq->sysfs_lock);
@@ -228,7 +227,7 @@ static void elevator_release(struct kobject *kobj)
struct elevator_queue *e;
e = container_of(kobj, struct elevator_queue, kobj);
- elevator_put(e->elevator_type);
+ elevator_put(e->type);
kfree(e->hash);
kfree(e);
}
@@ -288,9 +287,8 @@ EXPORT_SYMBOL(elevator_init);
void elevator_exit(struct elevator_queue *e)
{
mutex_lock(&e->sysfs_lock);
- if (e->ops->elevator_exit_fn)
- e->ops->elevator_exit_fn(e);
- e->ops = NULL;
+ if (e->type->ops.elevator_exit_fn)
+ e->type->ops.elevator_exit_fn(e);
mutex_unlock(&e->sysfs_lock);
kobject_put(&e->kobj);
@@ -500,8 +498,8 @@ int elv_merge(struct request_queue *q, struct request **req, struct bio *bio)
return ELEVATOR_BACK_MERGE;
}
- if (e->ops->elevator_merge_fn)
- return e->ops->elevator_merge_fn(q, req, bio);
+ if (e->type->ops.elevator_merge_fn)
+ return e->type->ops.elevator_merge_fn(q, req, bio);
return ELEVATOR_NO_MERGE;
}
@@ -544,8 +542,8 @@ void elv_merged_request(struct request_queue *q, struct request *rq, int type)
{
struct elevator_queue *e = q->elevator;
- if (e->ops->elevator_merged_fn)
- e->ops->elevator_merged_fn(q, rq, type);
+ if (e->type->ops.elevator_merged_fn)
+ e->type->ops.elevator_merged_fn(q, rq, type);
if (type == ELEVATOR_BACK_MERGE)
elv_rqhash_reposition(q, rq);
@@ -559,8 +557,8 @@ void elv_merge_requests(struct request_queue *q, struct request *rq,
struct elevator_queue *e = q->elevator;
const int next_sorted = next->cmd_flags & REQ_SORTED;
- if (next_sorted && e->ops->elevator_merge_req_fn)
- e->ops->elevator_merge_req_fn(q, rq, next);
+ if (next_sorted && e->type->ops.elevator_merge_req_fn)
+ e->type->ops.elevator_merge_req_fn(q, rq, next);
elv_rqhash_reposition(q, rq);
@@ -577,8 +575,8 @@ void elv_bio_merged(struct request_queue *q, struct request *rq,
{
struct elevator_queue *e = q->elevator;
- if (e->ops->elevator_bio_merged_fn)
- e->ops->elevator_bio_merged_fn(q, rq, bio);
+ if (e->type->ops.elevator_bio_merged_fn)
+ e->type->ops.elevator_bio_merged_fn(q, rq, bio);
}
void elv_requeue_request(struct request_queue *q, struct request *rq)
@@ -604,12 +602,12 @@ void elv_drain_elevator(struct request_queue *q)
lockdep_assert_held(q->queue_lock);
- while (q->elevator->ops->elevator_dispatch_fn(q, 1))
+ while (q->elevator->type->ops.elevator_dispatch_fn(q, 1))
;
if (q->nr_sorted && printed++ < 10) {
printk(KERN_ERR "%s: forced dispatching is broken "
"(nr_sorted=%u), please report this\n",
- q->elevator->elevator_type->elevator_name, q->nr_sorted);
+ q->elevator->type->elevator_name, q->nr_sorted);
}
}
@@ -698,7 +696,7 @@ void __elv_add_request(struct request_queue *q, struct request *rq, int where)
* rq cannot be accessed after calling
* elevator_add_req_fn.
*/
- q->elevator->ops->elevator_add_req_fn(q, rq);
+ q->elevator->type->ops.elevator_add_req_fn(q, rq);
break;
case ELEVATOR_INSERT_FLUSH:
@@ -727,8 +725,8 @@ struct request *elv_latter_request(struct request_queue *q, struct request *rq)
{
struct elevator_queue *e = q->elevator;
- if (e->ops->elevator_latter_req_fn)
- return e->ops->elevator_latter_req_fn(q, rq);
+ if (e->type->ops.elevator_latter_req_fn)
+ return e->type->ops.elevator_latter_req_fn(q, rq);
return NULL;
}
@@ -736,8 +734,8 @@ struct request *elv_former_request(struct request_queue *q, struct request *rq)
{
struct elevator_queue *e = q->elevator;
- if (e->ops->elevator_former_req_fn)
- return e->ops->elevator_former_req_fn(q, rq);
+ if (e->type->ops.elevator_former_req_fn)
+ return e->type->ops.elevator_former_req_fn(q, rq);
return NULL;
}
@@ -745,8 +743,8 @@ int elv_set_request(struct request_queue *q, struct request *rq, gfp_t gfp_mask)
{
struct elevator_queue *e = q->elevator;
- if (e->ops->elevator_set_req_fn)
- return e->ops->elevator_set_req_fn(q, rq, gfp_mask);
+ if (e->type->ops.elevator_set_req_fn)
+ return e->type->ops.elevator_set_req_fn(q, rq, gfp_mask);
rq->elevator_private[0] = NULL;
return 0;
@@ -756,16 +754,16 @@ void elv_put_request(struct request_queue *q, struct request *rq)
{
struct elevator_queue *e = q->elevator;
- if (e->ops->elevator_put_req_fn)
- e->ops->elevator_put_req_fn(rq);
+ if (e->type->ops.elevator_put_req_fn)
+ e->type->ops.elevator_put_req_fn(rq);
}
int elv_may_queue(struct request_queue *q, int rw)
{
struct elevator_queue *e = q->elevator;
- if (e->ops->elevator_may_queue_fn)
- return e->ops->elevator_may_queue_fn(q, rw);
+ if (e->type->ops.elevator_may_queue_fn)
+ return e->type->ops.elevator_may_queue_fn(q, rw);
return ELV_MQUEUE_MAY;
}
@@ -800,8 +798,8 @@ void elv_completed_request(struct request_queue *q, struct request *rq)
if (blk_account_rq(rq)) {
q->in_flight[rq_is_sync(rq)]--;
if ((rq->cmd_flags & REQ_SORTED) &&
- e->ops->elevator_completed_req_fn)
- e->ops->elevator_completed_req_fn(q, rq);
+ e->type->ops.elevator_completed_req_fn)
+ e->type->ops.elevator_completed_req_fn(q, rq);
}
}
@@ -819,7 +817,7 @@ elv_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
e = container_of(kobj, struct elevator_queue, kobj);
mutex_lock(&e->sysfs_lock);
- error = e->ops ? entry->show(e, page) : -ENOENT;
+ error = e->type ? entry->show(e, page) : -ENOENT;
mutex_unlock(&e->sysfs_lock);
return error;
}
@@ -837,7 +835,7 @@ elv_attr_store(struct kobject *kobj, struct attribute *attr,
e = container_of(kobj, struct elevator_queue, kobj);
mutex_lock(&e->sysfs_lock);
- error = e->ops ? entry->store(e, page, length) : -ENOENT;
+ error = e->type ? entry->store(e, page, length) : -ENOENT;
mutex_unlock(&e->sysfs_lock);
return error;
}
@@ -858,7 +856,7 @@ int __elv_register_queue(struct request_queue *q, struct elevator_queue *e)
error = kobject_add(&e->kobj, &q->kobj, "%s", "iosched");
if (!error) {
- struct elv_fs_entry *attr = e->elevator_type->elevator_attrs;
+ struct elv_fs_entry *attr = e->type->elevator_attrs;
if (attr) {
while (attr->attr.name) {
if (sysfs_create_file(&e->kobj, &attr->attr))
@@ -959,7 +957,7 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e)
elevator_exit(old_elevator);
elv_quiesce_end(q);
- blk_add_trace_msg(q, "elv switch: %s", e->elevator_type->elevator_name);
+ blk_add_trace_msg(q, "elv switch: %s", e->type->elevator_name);
return 0;
@@ -993,7 +991,7 @@ int elevator_change(struct request_queue *q, const char *name)
return -EINVAL;
}
- if (!strcmp(elevator_name, q->elevator->elevator_type->elevator_name)) {
+ if (!strcmp(elevator_name, q->elevator->type->elevator_name)) {
elevator_put(e);
return 0;
}
@@ -1028,7 +1026,7 @@ ssize_t elv_iosched_show(struct request_queue *q, char *name)
if (!q->elevator || !blk_queue_stackable(q))
return sprintf(name, "none\n");
- elv = e->elevator_type;
+ elv = e->type;
spin_lock(&elv_list_lock);
list_for_each_entry(__e, &elv_list, list) {
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index 02604c8..04958ef 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -78,11 +78,11 @@ struct elv_fs_entry {
*/
struct elevator_type
{
- struct list_head list;
struct elevator_ops ops;
struct elv_fs_entry *elevator_attrs;
char elevator_name[ELV_NAME_MAX];
struct module *elevator_owner;
+ struct list_head list;
};
/*
@@ -90,10 +90,9 @@ struct elevator_type
*/
struct elevator_queue
{
- struct elevator_ops *ops;
+ struct elevator_type *type;
void *elevator_data;
struct kobject kobj;
- struct elevator_type *elevator_type;
struct mutex sysfs_lock;
struct hlist_head *hash;
unsigned int registered:1;
--
1.7.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 04/10] block, cfq: reorganize cfq_io_context into generic and cfq specific parts
2011-10-29 23:49 [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
` (2 preceding siblings ...)
2011-10-29 23:49 ` [PATCH 03/10] block: remove elevator_queue->ops Tejun Heo
@ 2011-10-29 23:49 ` Tejun Heo
2011-10-29 23:49 ` [PATCH 05/10] block, cfq: move cfqd->icq_list to request_queue and add request->elv.icq Tejun Heo
` (6 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2011-10-29 23:49 UTC (permalink / raw)
To: axboe, vgoyal; +Cc: ctalbott, rni, linux-kernel, Tejun Heo
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 <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
block/blk-ioc.c | 58 +++++------
block/cfq-iosched.c | 248 ++++++++++++++++++++++++---------------------
include/linux/iocontext.h | 43 +++-----
3 files changed, 175 insertions(+), 174 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index e23c797..dc5e69d 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 5f7e4d1..d2f16fc 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 b2b75a5..d15ca65 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -5,38 +5,23 @@
#include <linux/rcupdate.h>
#include <linux/workqueue.h>
-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;
};
--
1.7.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 05/10] block, cfq: move cfqd->icq_list to request_queue and add request->elv.icq
2011-10-29 23:49 [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
` (3 preceding siblings ...)
2011-10-29 23:49 ` [PATCH 04/10] block, cfq: reorganize cfq_io_context into generic and cfq specific parts Tejun Heo
@ 2011-10-29 23:49 ` Tejun Heo
2011-10-29 23:49 ` [PATCH 06/10] block, cfq: move io_cq lookup to blk-ioc.c Tejun Heo
` (5 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2011-10-29 23:49 UTC (permalink / raw)
To: axboe, vgoyal; +Cc: ctalbott, rni, linux-kernel, Tejun Heo
Most of icq management is about to be moved out of cfq into blk-ioc.
This patch prepares for it.
* Move cfqd->icq_list to request_queue->icq_list
* Make request explicitly point to icq instead of through elevator
private data. ->elevator_private[3] is replaced with sub struct elv
which contains icq pointer and priv[2]. cfq is updated accordingly.
* Meaningless clearing of ->elevator_private[0] removed from
elv_set_request(). At that point in code, the field was guaranteed
to be %NULL anyway.
This patch doesn't introduce any functional change.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
block/blk-core.c | 1 +
block/cfq-iosched.c | 28 +++++++++++-----------------
block/elevator.c | 2 --
include/linux/blkdev.h | 10 ++++++++--
4 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 74b44ab..5315f09 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -492,6 +492,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
laptop_mode_timer_fn, (unsigned long) q);
setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q);
INIT_LIST_HEAD(&q->timeout_list);
+ INIT_LIST_HEAD(&q->icq_list);
INIT_LIST_HEAD(&q->flush_queue[0]);
INIT_LIST_HEAD(&q->flush_queue[1]);
INIT_LIST_HEAD(&q->flush_data_in_flight);
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index d2f16fc..9bc5ecc 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -54,9 +54,9 @@ 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) 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])
+#define RQ_CIC(rq) icq_to_cic((rq)->elv.icq)
+#define RQ_CFQQ(rq) (struct cfq_queue *) ((rq)->elv.priv[0])
+#define RQ_CFQG(rq) (struct cfq_group *) ((rq)->elv.priv[1])
static struct kmem_cache *cfq_pool;
static struct kmem_cache *cfq_icq_pool;
@@ -297,8 +297,6 @@ struct cfq_data {
unsigned int cfq_group_idle;
unsigned int cfq_latency;
- struct list_head icq_list;
-
/*
* Fallback dummy cfqq for extreme OOM conditions
*/
@@ -3053,7 +3051,7 @@ static int cfq_create_cic(struct cfq_data *cfqd, gfp_t gfp_mask)
ret = radix_tree_insert(&ioc->icq_tree, q->id, icq);
if (likely(!ret)) {
hlist_add_head(&icq->ioc_node, &ioc->icq_list);
- list_add(&icq->q_node, &cfqd->icq_list);
+ list_add(&icq->q_node, &q->icq_list);
icq = NULL;
} else if (ret == -EEXIST) {
/* someone else already did it */
@@ -3605,12 +3603,10 @@ static void cfq_put_request(struct request *rq)
put_io_context(RQ_CIC(rq)->icq.ioc, cfqq->cfqd->queue);
- rq->elevator_private[0] = NULL;
- rq->elevator_private[1] = NULL;
-
/* Put down rq reference on cfqg */
cfq_put_cfqg(RQ_CFQG(rq));
- rq->elevator_private[2] = NULL;
+ rq->elv.priv[0] = NULL;
+ rq->elv.priv[1] = NULL;
cfq_put_queue(cfqq);
}
@@ -3696,9 +3692,9 @@ new_queue:
cfqq->allocated[rw]++;
cfqq->ref++;
- rq->elevator_private[0] = &cic->icq;
- rq->elevator_private[1] = cfqq;
- rq->elevator_private[2] = cfq_ref_get_cfqg(cfqq->cfqg);
+ 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;
@@ -3810,8 +3806,8 @@ 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->icq_list)) {
- struct io_cq *icq = list_entry(cfqd->icq_list.next,
+ 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;
@@ -3922,8 +3918,6 @@ 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->icq_list);
-
cfqd->queue = q;
init_timer(&cfqd->idle_slice_timer);
diff --git a/block/elevator.c b/block/elevator.c
index 31ffe76..c5c6214 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -745,8 +745,6 @@ int elv_set_request(struct request_queue *q, struct request *rq, gfp_t gfp_mask)
if (e->type->ops.elevator_set_req_fn)
return e->type->ops.elevator_set_req_fn(q, rq, gfp_mask);
-
- rq->elevator_private[0] = NULL;
return 0;
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e4c10a0..6c7e09c 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -111,10 +111,14 @@ struct request {
* Three pointers are available for the IO schedulers, if they need
* more they have to dynamically allocate it. Flush requests are
* never put on the IO scheduler. So let the flush fields share
- * space with the three elevator_private pointers.
+ * space with the elevator data.
*/
union {
- void *elevator_private[3];
+ struct {
+ struct io_cq *icq;
+ void *priv[2];
+ } elv;
+
struct {
unsigned int seq;
struct list_head list;
@@ -357,6 +361,8 @@ struct request_queue {
struct timer_list timeout;
struct list_head timeout_list;
+ struct list_head icq_list;
+
struct queue_limits limits;
/*
--
1.7.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 06/10] block, cfq: move io_cq lookup to blk-ioc.c
2011-10-29 23:49 [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
` (4 preceding siblings ...)
2011-10-29 23:49 ` [PATCH 05/10] block, cfq: move cfqd->icq_list to request_queue and add request->elv.icq Tejun Heo
@ 2011-10-29 23:49 ` Tejun Heo
2011-10-29 23:49 ` [PATCH 07/10] block, cfq: move icq cache management to block core Tejun Heo
` (4 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2011-10-29 23:49 UTC (permalink / raw)
To: axboe, vgoyal; +Cc: ctalbott, rni, linux-kernel, Tejun Heo
Now that all io_cq related data structures are in block core layer,
io_cq lookup can be moved from cfq-iosched.c to blk-ioc.c.
Lookup logic from cfq_cic_lookup() is moved to ioc_lookup_icq() with
parameter return type changes (cfqd -> request_queue, cfq_io_cq ->
io_cq) and cfq_cic_lookup() becomes thin wrapper around
cfq_cic_lookup().
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
block/blk-ioc.c | 36 ++++++++++++++++++++++++++++++++++++
block/blk.h | 1 +
block/cfq-iosched.c | 48 ++++++++----------------------------------------
3 files changed, 45 insertions(+), 40 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index dc5e69d..87ecc98 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -266,6 +266,42 @@ struct io_context *get_task_io_context(struct task_struct *task,
}
EXPORT_SYMBOL(get_task_io_context);
+/**
+ * ioc_lookup_icq - lookup io_cq from ioc
+ * @ioc: the associated io_context
+ * @q: the associated request_queue
+ *
+ * Look up io_cq associated with @ioc - @q pair from @ioc. Must be called
+ * with @q->queue_lock held.
+ */
+struct io_cq *ioc_lookup_icq(struct io_context *ioc, struct request_queue *q)
+{
+ struct io_cq *icq;
+
+ lockdep_assert_held(q->queue_lock);
+
+ /*
+ * 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 icq which points to us, it's guaranteed to be valid.
+ */
+ rcu_read_lock();
+ icq = rcu_dereference(ioc->icq_hint);
+ if (icq && icq->q == q)
+ goto out;
+
+ icq = radix_tree_lookup(&ioc->icq_tree, q->id);
+ if (icq && icq->q == q)
+ rcu_assign_pointer(ioc->icq_hint, icq); /* allowed to race */
+ else
+ icq = NULL;
+out:
+ rcu_read_unlock();
+ return icq;
+}
+EXPORT_SYMBOL(ioc_lookup_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 4943770..3c510a4 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -199,6 +199,7 @@ static inline int blk_do_io_stat(struct request *rq)
* Internal io_context interface
*/
void get_io_context(struct io_context *ioc);
+struct io_cq *ioc_lookup_icq(struct io_context *ioc, 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 9bc5ecc..048fa69 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -468,7 +468,6 @@ 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_cq *cfq_cic_lookup(struct cfq_data *, struct io_context *);
static inline struct cfq_io_cq *icq_to_cic(struct io_cq *icq)
{
@@ -476,6 +475,14 @@ static inline struct cfq_io_cq *icq_to_cic(struct io_cq *icq)
return container_of(icq, struct cfq_io_cq, icq);
}
+static inline struct cfq_io_cq *cfq_cic_lookup(struct cfq_data *cfqd,
+ struct io_context *ioc)
+{
+ if (ioc)
+ return icq_to_cic(ioc_lookup_icq(ioc, cfqd->queue));
+ return NULL;
+}
+
static inline struct cfq_queue *cic_to_cfqq(struct cfq_io_cq *cic, bool is_sync)
{
return cic->cfqq[is_sync];
@@ -2971,45 +2978,6 @@ cfq_get_queue(struct cfq_data *cfqd, bool is_sync, struct io_context *ioc,
}
/**
- * cfq_cic_lookup - lookup cfq_io_cq
- * @cfqd: the associated cfq_data
- * @ioc: the associated io_context
- *
- * Look up cfq_io_cq associated with @cfqd - @ioc pair. Must be called
- * with queue_lock held.
- */
-static struct cfq_io_cq *
-cfq_cic_lookup(struct cfq_data *cfqd, struct io_context *ioc)
-{
- struct request_queue *q = cfqd->queue;
- struct io_cq *icq;
-
- lockdep_assert_held(cfqd->queue->queue_lock);
- if (unlikely(!ioc))
- return NULL;
-
- /*
- * 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 icq which points to us, it's guaranteed to be valid.
- */
- rcu_read_lock();
- icq = rcu_dereference(ioc->icq_hint);
- if (icq && icq->q == q)
- goto out;
-
- 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
- icq = NULL;
-out:
- rcu_read_unlock();
- return icq_to_cic(icq);
-}
-
-/**
* cfq_create_cic - create and link a cfq_io_cq
* @cfqd: cfqd of interest
* @gfp_mask: allocation mask
--
1.7.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 07/10] block, cfq: move icq cache management to block core
2011-10-29 23:49 [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
` (5 preceding siblings ...)
2011-10-29 23:49 ` [PATCH 06/10] block, cfq: move io_cq lookup to blk-ioc.c Tejun Heo
@ 2011-10-29 23:49 ` Tejun Heo
2011-10-29 23:49 ` [PATCH 08/10] block, cfq: move io_cq exit/release to blk-ioc.c Tejun Heo
` (3 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2011-10-29 23:49 UTC (permalink / raw)
To: axboe, vgoyal; +Cc: ctalbott, rni, linux-kernel, Tejun Heo
Let elevators set ->icq_size and ->icq_align in elevator_type and
elv_register() and elv_unregister() respectively create and destroy
kmem_cache for icq.
* elv_register() now can return failure. All callers updated.
* icq caches are automatically named "ELVNAME_io_cq".
* cfq_slab_setup/kill() are collapsed into cfq_init/exit().
* While at it, minor indentation change for iosched_cfq.elevator_name
for consistency.
This will help moving icq management to block core. This doesn't
introduce any functional change.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
block/cfq-iosched.c | 48 ++++++++++++++-------------------------------
block/deadline-iosched.c | 4 +--
block/elevator.c | 37 +++++++++++++++++++++++++++++++++-
block/noop-iosched.c | 4 +--
include/linux/elevator.h | 11 +++++++++-
5 files changed, 62 insertions(+), 42 deletions(-)
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 048fa69..06e59ab 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -3914,34 +3914,6 @@ static void *cfq_init_queue(struct request_queue *q)
return cfqd;
}
-static void cfq_slab_kill(void)
-{
- /*
- * Caller already ensured that pending RCU callbacks are completed,
- * so we should have no busy allocations at this point.
- */
- if (cfq_pool)
- kmem_cache_destroy(cfq_pool);
- if (cfq_icq_pool)
- kmem_cache_destroy(cfq_icq_pool);
-}
-
-static int __init cfq_slab_setup(void)
-{
- cfq_pool = KMEM_CACHE(cfq_queue, 0);
- if (!cfq_pool)
- goto fail;
-
- cfq_icq_pool = KMEM_CACHE(cfq_io_cq, 0);
- if (!cfq_icq_pool)
- goto fail;
-
- return 0;
-fail:
- cfq_slab_kill();
- return -ENOMEM;
-}
-
/*
* sysfs parts below -->
*/
@@ -4053,8 +4025,10 @@ static struct elevator_type iosched_cfq = {
.elevator_init_fn = cfq_init_queue,
.elevator_exit_fn = cfq_exit_queue,
},
+ .icq_size = sizeof(struct cfq_io_cq),
+ .icq_align = __alignof__(struct cfq_io_cq),
.elevator_attrs = cfq_attrs,
- .elevator_name = "cfq",
+ .elevator_name = "cfq",
.elevator_owner = THIS_MODULE,
};
@@ -4072,6 +4046,8 @@ static struct blkio_policy_type blkio_policy_cfq;
static int __init cfq_init(void)
{
+ int ret;
+
/*
* could be 0 on HZ < 1000 setups
*/
@@ -4086,10 +4062,17 @@ static int __init cfq_init(void)
#else
cfq_group_idle = 0;
#endif
- if (cfq_slab_setup())
+ cfq_pool = KMEM_CACHE(cfq_queue, 0);
+ if (!cfq_pool)
return -ENOMEM;
- elv_register(&iosched_cfq);
+ ret = elv_register(&iosched_cfq);
+ if (ret) {
+ kmem_cache_destroy(cfq_pool);
+ return ret;
+ }
+ cfq_icq_pool = iosched_cfq.icq_cache;
+
blkio_policy_register(&blkio_policy_cfq);
return 0;
@@ -4099,8 +4082,7 @@ static void __exit cfq_exit(void)
{
blkio_policy_unregister(&blkio_policy_cfq);
elv_unregister(&iosched_cfq);
- rcu_barrier(); /* make sure all cic RCU frees are complete */
- cfq_slab_kill();
+ kmem_cache_destroy(cfq_pool);
}
module_init(cfq_init);
diff --git a/block/deadline-iosched.c b/block/deadline-iosched.c
index c644137..7bf12d7 100644
--- a/block/deadline-iosched.c
+++ b/block/deadline-iosched.c
@@ -448,9 +448,7 @@ static struct elevator_type iosched_deadline = {
static int __init deadline_init(void)
{
- elv_register(&iosched_deadline);
-
- return 0;
+ return elv_register(&iosched_deadline);
}
static void __exit deadline_exit(void)
diff --git a/block/elevator.c b/block/elevator.c
index c5c6214..cca049f 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -886,15 +886,36 @@ void elv_unregister_queue(struct request_queue *q)
}
EXPORT_SYMBOL(elv_unregister_queue);
-void elv_register(struct elevator_type *e)
+int elv_register(struct elevator_type *e)
{
char *def = "";
+ /* create icq_cache if requested */
+ if (e->icq_size) {
+ if (WARN_ON(e->icq_size < sizeof(struct io_cq)) ||
+ WARN_ON(e->icq_align < __alignof__(struct io_cq)))
+ return -EINVAL;
+
+ snprintf(e->icq_cache_name, sizeof(e->icq_cache_name),
+ "%s_io_cq", e->elevator_name);
+ e->icq_cache = kmem_cache_create(e->icq_cache_name, e->icq_size,
+ e->icq_align, 0, NULL);
+ if (!e->icq_cache)
+ return -ENOMEM;
+ }
+
+ /* register, don't allow duplicate names */
spin_lock(&elv_list_lock);
- BUG_ON(elevator_find(e->elevator_name));
+ if (elevator_find(e->elevator_name)) {
+ spin_unlock(&elv_list_lock);
+ if (e->icq_cache)
+ kmem_cache_destroy(e->icq_cache);
+ return -EBUSY;
+ }
list_add_tail(&e->list, &elv_list);
spin_unlock(&elv_list_lock);
+ /* print pretty message */
if (!strcmp(e->elevator_name, chosen_elevator) ||
(!*chosen_elevator &&
!strcmp(e->elevator_name, CONFIG_DEFAULT_IOSCHED)))
@@ -902,14 +923,26 @@ void elv_register(struct elevator_type *e)
printk(KERN_INFO "io scheduler %s registered%s\n", e->elevator_name,
def);
+ return 0;
}
EXPORT_SYMBOL_GPL(elv_register);
void elv_unregister(struct elevator_type *e)
{
+ /* unregister */
spin_lock(&elv_list_lock);
list_del_init(&e->list);
spin_unlock(&elv_list_lock);
+
+ /*
+ * Destroy icq_cache if it exists. icq's are RCU managed. Make
+ * sure all RCU operations are complete before proceeding.
+ */
+ if (e->icq_cache) {
+ rcu_barrier();
+ kmem_cache_destroy(e->icq_cache);
+ e->icq_cache = NULL;
+ }
}
EXPORT_SYMBOL_GPL(elv_unregister);
diff --git a/block/noop-iosched.c b/block/noop-iosched.c
index 06389e9..413a0b1 100644
--- a/block/noop-iosched.c
+++ b/block/noop-iosched.c
@@ -94,9 +94,7 @@ static struct elevator_type elevator_noop = {
static int __init noop_init(void)
{
- elv_register(&elevator_noop);
-
- return 0;
+ return elv_register(&elevator_noop);
}
static void __exit noop_exit(void)
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index 04958ef..d3d3e28 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -78,10 +78,19 @@ struct elv_fs_entry {
*/
struct elevator_type
{
+ /* managed by elevator core */
+ struct kmem_cache *icq_cache;
+
+ /* fields provided by elevator implementation */
struct elevator_ops ops;
+ size_t icq_size;
+ size_t icq_align;
struct elv_fs_entry *elevator_attrs;
char elevator_name[ELV_NAME_MAX];
struct module *elevator_owner;
+
+ /* managed by elevator core */
+ char icq_cache_name[ELV_NAME_MAX + 5]; /* elvname + "_io_cq" */
struct list_head list;
};
@@ -127,7 +136,7 @@ extern void elv_drain_elevator(struct request_queue *);
/*
* io scheduler registration
*/
-extern void elv_register(struct elevator_type *);
+extern int elv_register(struct elevator_type *);
extern void elv_unregister(struct elevator_type *);
/*
--
1.7.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 08/10] block, cfq: move io_cq exit/release to blk-ioc.c
2011-10-29 23:49 [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
` (6 preceding siblings ...)
2011-10-29 23:49 ` [PATCH 07/10] block, cfq: move icq cache management to block core Tejun Heo
@ 2011-10-29 23:49 ` Tejun Heo
2011-10-29 23:49 ` [PATCH 09/10] block, cfq: restructure io_cq creation path for io_context interface cleanup Tejun Heo
` (2 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2011-10-29 23:49 UTC (permalink / raw)
To: axboe, vgoyal; +Cc: ctalbott, rni, linux-kernel, Tejun Heo
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 <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
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(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 87ecc98..0910a55 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 5b4b4ab..cf15001 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 3c510a4..ed4d9bf 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 06e59ab..f6d3155 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 cca049f..91e18f8 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 d3d3e28..06e4dd56 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 d15ca65..ac390a3 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 *);
};
/*
--
1.7.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 09/10] block, cfq: restructure io_cq creation path for io_context interface cleanup
2011-10-29 23:49 [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
` (7 preceding siblings ...)
2011-10-29 23:49 ` [PATCH 08/10] block, cfq: move io_cq exit/release to blk-ioc.c Tejun Heo
@ 2011-10-29 23:49 ` Tejun Heo
2011-10-29 23:49 ` [PATCH 10/10] block, cfq: move icq creation and rq->elv.icq association to block core Tejun Heo
2011-11-03 15:08 ` [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
10 siblings, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2011-10-29 23:49 UTC (permalink / raw)
To: axboe, vgoyal; +Cc: ctalbott, rni, linux-kernel, Tejun Heo
Add elevator_ops->elevator_init_icq_fn() and restructure
cfq_create_cic() and rename it to ioc_create_icq().
The new function expects its caller to pass in io_context, uses
elevator_type->icq_cache, handles generic init, calls the new elevator
operation for elevator specific initialization, and returns pointer to
created or looked up icq. This leaves cfq_icq_pool variable without
any user. Removed.
This prepares for io_context interface cleanup and doesn't introduce
any functional difference.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
block/cfq-iosched.c | 94 ++++++++++++++++++++--------------------------
include/linux/elevator.h | 2 +
2 files changed, 43 insertions(+), 53 deletions(-)
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index f6d3155..11f49d0 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -59,7 +59,6 @@ static const int cfq_hist_divisor = 4;
#define RQ_CFQG(rq) (struct cfq_group *) ((rq)->elv.priv[1])
static struct kmem_cache *cfq_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)
@@ -2707,6 +2706,13 @@ static void cfq_exit_cfqq(struct cfq_data *cfqd, struct cfq_queue *cfqq)
cfq_put_queue(cfqq);
}
+static void cfq_init_icq(struct io_cq *icq)
+{
+ struct cfq_io_cq *cic = icq_to_cic(icq);
+
+ cic->ttime.last_end_request = jiffies;
+}
+
static void cfq_exit_icq(struct io_cq *icq)
{
struct cfq_io_cq *cic = icq_to_cic(icq);
@@ -2723,21 +2729,6 @@ static void cfq_exit_icq(struct io_cq *icq)
}
}
-static struct cfq_io_cq *cfq_alloc_cic(struct cfq_data *cfqd, gfp_t gfp_mask)
-{
- struct cfq_io_cq *cic;
-
- 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->icq.q_node);
- INIT_HLIST_NODE(&cic->icq.ioc_node);
- }
-
- return cic;
-}
-
static void cfq_init_prio_data(struct cfq_queue *cfqq, struct io_context *ioc)
{
struct task_struct *tsk = current;
@@ -2945,64 +2936,62 @@ cfq_get_queue(struct cfq_data *cfqd, bool is_sync, struct io_context *ioc,
}
/**
- * cfq_create_cic - create and link a cfq_io_cq
- * @cfqd: cfqd of interest
+ * ioc_create_icq - create and link io_cq
+ * @q: request_queue of interest
* @gfp_mask: allocation 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.
+ * 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 int cfq_create_cic(struct cfq_data *cfqd, gfp_t gfp_mask)
+static struct io_cq *ioc_create_icq(struct request_queue *q, gfp_t gfp_mask)
{
- struct request_queue *q = cfqd->queue;
- struct io_cq *icq = NULL;
- struct cfq_io_cq *cic;
+ struct elevator_type *et = q->elevator->type;
struct io_context *ioc;
- int ret = -ENOMEM;
-
- might_sleep_if(gfp_mask & __GFP_WAIT);
+ struct io_cq *icq;
/* allocate stuff */
ioc = create_io_context(current, gfp_mask, q->node);
if (!ioc)
- goto out;
+ return NULL;
- cic = cfq_alloc_cic(cfqd, gfp_mask);
- if (!cic)
- goto out;
- icq = &cic->icq;
+ icq = kmem_cache_alloc_node(et->icq_cache, gfp_mask | __GFP_ZERO,
+ q->node);
+ if (!icq)
+ return NULL;
- ret = radix_tree_preload(gfp_mask);
- if (ret)
- goto out;
+ if (radix_tree_preload(gfp_mask) < 0) {
+ kmem_cache_free(et->icq_cache, icq);
+ return NULL;
+ }
icq->ioc = ioc;
- icq->q = cfqd->queue;
+ 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);
- ret = radix_tree_insert(&ioc->icq_tree, q->id, icq);
- if (likely(!ret)) {
+ 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);
- icq = NULL;
- } else if (ret == -EEXIST) {
- /* someone else already did it */
- ret = 0;
+ 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();
-out:
- if (ret)
- printk(KERN_ERR "cfq: icq link failed!\n");
- if (icq)
- kmem_cache_free(cfq_icq_pool, icq);
- return ret;
+ return icq;
}
/**
@@ -3022,7 +3011,6 @@ 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;
- int err;
lockdep_assert_held(q->queue_lock);
@@ -3037,9 +3025,9 @@ static struct cfq_io_cq *cfq_get_cic(struct cfq_data *cfqd, gfp_t gfp_mask)
/* slow path - unlock, create missing ones and retry */
spin_unlock_irq(q->queue_lock);
- err = cfq_create_cic(cfqd, gfp_mask);
+ cic = icq_to_cic(ioc_create_icq(q, gfp_mask));
spin_lock_irq(q->queue_lock);
- if (err)
+ if (!cic)
return NULL;
}
@@ -3975,6 +3963,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_init_icq_fn = cfq_init_icq,
.elevator_exit_icq_fn = cfq_exit_icq,
.elevator_set_req_fn = cfq_set_request,
.elevator_put_req_fn = cfq_put_request,
@@ -4028,7 +4017,6 @@ static int __init cfq_init(void)
kmem_cache_destroy(cfq_pool);
return ret;
}
- cfq_icq_pool = iosched_cfq.icq_cache;
blkio_policy_register(&blkio_policy_cfq);
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index 06e4dd56..c8f1e67 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -26,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_init_icq_fn) (struct io_cq *);
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 *);
@@ -59,6 +60,7 @@ 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_set_req_fn *elevator_set_req_fn;
--
1.7.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 10/10] block, cfq: move icq creation and rq->elv.icq association to block core
2011-10-29 23:49 [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
` (8 preceding siblings ...)
2011-10-29 23:49 ` [PATCH 09/10] block, cfq: restructure io_cq creation path for io_context interface cleanup Tejun Heo
@ 2011-10-29 23:49 ` Tejun Heo
2011-11-03 15:08 ` [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
10 siblings, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2011-10-29 23:49 UTC (permalink / raw)
To: axboe, vgoyal; +Cc: ctalbott, rni, linux-kernel, Tejun Heo
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 <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
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(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 5315f09..2da3cd6 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -635,13 +635,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);
@@ -652,10 +657,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;
@@ -767,11 +777,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)))
@@ -832,17 +845,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 0910a55..c04d16b 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 ed4d9bf..7efd772 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 11f49d0..f3b44c3 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 c8f1e67..c24f3d7 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 ac390a3..7e1371c 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;
--
1.7.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCHSET block:for-3.2/core] cleanup io_context interface
2011-10-29 23:49 [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
` (9 preceding siblings ...)
2011-10-29 23:49 ` [PATCH 10/10] block, cfq: move icq creation and rq->elv.icq association to block core Tejun Heo
@ 2011-11-03 15:08 ` Tejun Heo
2011-11-04 9:16 ` Jens Axboe
10 siblings, 1 reply; 18+ messages in thread
From: Tejun Heo @ 2011-11-03 15:08 UTC (permalink / raw)
To: axboe, vgoyal; +Cc: ctalbott, rni, linux-kernel
Jens, ping?
--
tejun
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCHSET block:for-3.2/core] cleanup io_context interface
2011-11-03 15:08 ` [PATCHSET block:for-3.2/core] cleanup io_context interface Tejun Heo
@ 2011-11-04 9:16 ` Jens Axboe
2011-11-04 14:22 ` Tejun Heo
0 siblings, 1 reply; 18+ messages in thread
From: Jens Axboe @ 2011-11-04 9:16 UTC (permalink / raw)
To: Tejun Heo; +Cc: vgoyal, ctalbott, rni, linux-kernel
On 2011-11-03 16:08, Tejun Heo wrote:
> Jens, ping?
I'll queue this up as soon as the current stuff has been shipped off,
it's just too close (and inside of) the merge window to slide in at this
point.
--
Jens Axboe
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHSET block:for-3.2/core] cleanup io_context interface
2011-11-04 9:16 ` Jens Axboe
@ 2011-11-04 14:22 ` Tejun Heo
2011-11-04 14:29 ` Jens Axboe
0 siblings, 1 reply; 18+ messages in thread
From: Tejun Heo @ 2011-11-04 14:22 UTC (permalink / raw)
To: Jens Axboe; +Cc: vgoyal, ctalbott, rni, linux-kernel
Hello,
On Fri, Nov 4, 2011 at 2:16 AM, Jens Axboe <axboe@kernel.dk> wrote:
> On 2011-11-03 16:08, Tejun Heo wrote:
>> Jens, ping?
>
> I'll queue this up as soon as the current stuff has been shipped off,
> it's just too close (and inside of) the merge window to slide in at this
> point.
Oh, all these definitely are not for this window. Was just wondering
as there was no response. Alright, will wait for the merge window to
close.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHSET block:for-3.2/core] cleanup io_context interface
2011-11-04 14:22 ` Tejun Heo
@ 2011-11-04 14:29 ` Jens Axboe
2011-11-14 17:40 ` Tejun Heo
0 siblings, 1 reply; 18+ messages in thread
From: Jens Axboe @ 2011-11-04 14:29 UTC (permalink / raw)
To: Tejun Heo; +Cc: vgoyal, ctalbott, rni, linux-kernel
On 2011-11-04 15:22, Tejun Heo wrote:
> Hello,
>
> On Fri, Nov 4, 2011 at 2:16 AM, Jens Axboe <axboe@kernel.dk> wrote:
>> On 2011-11-03 16:08, Tejun Heo wrote:
>>> Jens, ping?
>>
>> I'll queue this up as soon as the current stuff has been shipped off,
>> it's just too close (and inside of) the merge window to slide in at this
>> point.
>
> Oh, all these definitely are not for this window. Was just wondering
> as there was no response. Alright, will wait for the merge window to
> close.
That matches my expectations as well. They will be queued up shortly,
pushed to for-next in a week or so. I'll have time to go over these
early next week!
--
Jens Axboe
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHSET block:for-3.2/core] cleanup io_context interface
2011-11-04 14:29 ` Jens Axboe
@ 2011-11-14 17:40 ` Tejun Heo
2011-11-14 17:52 ` Jens Axboe
0 siblings, 1 reply; 18+ messages in thread
From: Tejun Heo @ 2011-11-14 17:40 UTC (permalink / raw)
To: Jens Axboe; +Cc: vgoyal, ctalbott, rni, linux-kernel
Hey, Jens.
On Fri, Nov 04, 2011 at 03:29:09PM +0100, Jens Axboe wrote:
> That matches my expectations as well. They will be queued up shortly,
> pushed to for-next in a week or so. I'll have time to go over these
> early next week!
Any progress?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHSET block:for-3.2/core] cleanup io_context interface
2011-11-14 17:40 ` Tejun Heo
@ 2011-11-14 17:52 ` Jens Axboe
2011-11-14 17:58 ` Tejun Heo
0 siblings, 1 reply; 18+ messages in thread
From: Jens Axboe @ 2011-11-14 17:52 UTC (permalink / raw)
To: Tejun Heo; +Cc: vgoyal, ctalbott, rni, linux-kernel
On 2011-11-14 18:40, Tejun Heo wrote:
> Hey, Jens.
>
> On Fri, Nov 04, 2011 at 03:29:09PM +0100, Jens Axboe wrote:
>> That matches my expectations as well. They will be queued up shortly,
>> pushed to for-next in a week or so. I'll have time to go over these
>> early next week!
>
> Any progress?
We agreed to queue these for 3.3, right?
--
Jens Axboe
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHSET block:for-3.2/core] cleanup io_context interface
2011-11-14 17:52 ` Jens Axboe
@ 2011-11-14 17:58 ` Tejun Heo
0 siblings, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2011-11-14 17:58 UTC (permalink / raw)
To: Jens Axboe; +Cc: vgoyal, ctalbott, rni, linux-kernel
Hello, Jens.
On Mon, Nov 14, 2011 at 06:52:18PM +0100, Jens Axboe wrote:
> We agreed to queue these for 3.3, right?
Ah... I'm getting confused about version numbers. This is 3.2-rc1, so
yes this is for 3.3-rc1. Just wondering whether/when you are gonna
pull this into -next. If you agree with the series, I'll be going
ahead and making similar changes to blkcg.
Thank you.
--
tejun
^ permalink raw reply [flat|nested] 18+ messages in thread