From d705ae6b133f9f6a8beee617b1224b6a5c99c5da Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 15 Feb 2012 09:45:49 +0100 Subject: block: replace icq->changed with icq->flags icq->changed was used for ICQ_*_CHANGED bits. Rename it to flags and access it under ioc->lock instead of using atomic bitops. ioc_get_changed() is added so that the changed part can be fetched and cleared as before. icq->flags will be used to carry other flags. Signed-off-by: Tejun Heo Tested-by: Shaohua Li Signed-off-by: Jens Axboe --- include/linux/iocontext.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'include/linux/iocontext.h') diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h index 119773eebe31..17839c7b9614 100644 --- a/include/linux/iocontext.h +++ b/include/linux/iocontext.h @@ -6,8 +6,10 @@ #include enum { - ICQ_IOPRIO_CHANGED, - ICQ_CGROUP_CHANGED, + ICQ_IOPRIO_CHANGED = 1 << 0, + ICQ_CGROUP_CHANGED = 1 << 1, + + ICQ_CHANGED_MASK = ICQ_IOPRIO_CHANGED | ICQ_CGROUP_CHANGED, }; /* @@ -88,7 +90,7 @@ struct io_cq { struct rcu_head __rcu_head; }; - unsigned long changed; + unsigned int flags; }; /* @@ -139,6 +141,7 @@ struct io_context *get_task_io_context(struct task_struct *task, gfp_t gfp_flags, int node); void ioc_ioprio_changed(struct io_context *ioc, int ioprio); void ioc_cgroup_changed(struct io_context *ioc); +unsigned int icq_get_changed(struct io_cq *icq); #else struct io_context; static inline void put_io_context(struct io_context *ioc) { } -- cgit v1.2.3 From 621032ad6eaabf2fe771c4fa0d8f58e1fcfcdba6 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 15 Feb 2012 09:45:53 +0100 Subject: block: exit_io_context() should call elevator_exit_icq_fn() While updating locking, b2efa05265 "block, cfq: unlink cfq_io_context's immediately" moved elevator_exit_icq_fn() invocation from exit_io_context() to the final ioc put. While this doesn't cause catastrophic failure, it effectively removes task exit notification to elevator and cause noticeable IO performance degradation with CFQ. On task exit, CFQ used to immediately expire the slice if it was being used by the exiting task as no more IO would be issued by the task; however, after b2efa05265, the notification is lost and disk could sit idle needlessly, leading to noticeable IO performance degradation for certain workloads. This patch renames ioc_exit_icq() to ioc_destroy_icq(), separates elevator_exit_icq_fn() invocation into ioc_exit_icq() and invokes it from exit_io_context(). ICQ_EXITED flag is added to avoid invoking the callback more than once for the same icq. Walking icq_list from ioc side and invoking elevator callback requires reverse double locking. This may be better implemented using RCU; unfortunately, using RCU isn't trivial. e.g. RCU protection would need to cover request_queue and queue_lock switch on cleanup makes grabbing queue_lock from RCU unsafe. Reverse double locking should do, at least for now. Signed-off-by: Tejun Heo Reported-and-bisected-by: Shaohua Li LKML-Reference: Tested-by: Shaohua Li Signed-off-by: Jens Axboe --- block/blk-ioc.c | 55 ++++++++++++++++++++++++++++++++++++++++------- include/linux/iocontext.h | 1 + 2 files changed, 48 insertions(+), 8 deletions(-) (limited to 'include/linux/iocontext.h') diff --git a/block/blk-ioc.c b/block/blk-ioc.c index f53c80ecaf07..92bf55540d87 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -36,10 +36,22 @@ static void icq_free_icq_rcu(struct rcu_head *head) kmem_cache_free(icq->__rcu_icq_cache, icq); } -/* - * Exit and free an icq. Called with both ioc and q locked. - */ +/* Exit an icq. Called with both ioc and q locked. */ static void ioc_exit_icq(struct io_cq *icq) +{ + struct elevator_type *et = icq->q->elevator->type; + + if (icq->flags & ICQ_EXITED) + return; + + if (et->ops.elevator_exit_icq_fn) + et->ops.elevator_exit_icq_fn(icq); + + icq->flags |= ICQ_EXITED; +} + +/* Release an icq. Called with both ioc and q locked. */ +static void ioc_destroy_icq(struct io_cq *icq) { struct io_context *ioc = icq->ioc; struct request_queue *q = icq->q; @@ -60,8 +72,7 @@ static void ioc_exit_icq(struct io_cq *icq) if (rcu_dereference_raw(ioc->icq_hint) == icq) rcu_assign_pointer(ioc->icq_hint, NULL); - if (et->ops.elevator_exit_icq_fn) - et->ops.elevator_exit_icq_fn(icq); + ioc_exit_icq(icq); /* * @icq->q might have gone away by the time RCU callback runs @@ -95,7 +106,7 @@ static void ioc_release_fn(struct work_struct *work) struct request_queue *q = icq->q; if (spin_trylock(q->queue_lock)) { - ioc_exit_icq(icq); + ioc_destroy_icq(icq); spin_unlock(q->queue_lock); } else { spin_unlock_irqrestore(&ioc->lock, flags); @@ -142,13 +153,41 @@ EXPORT_SYMBOL(put_io_context); void exit_io_context(struct task_struct *task) { struct io_context *ioc; + struct io_cq *icq; + struct hlist_node *n; + unsigned long flags; task_lock(task); ioc = task->io_context; task->io_context = NULL; task_unlock(task); - atomic_dec(&ioc->nr_tasks); + if (!atomic_dec_and_test(&ioc->nr_tasks)) { + put_io_context(ioc); + return; + } + + /* + * Need ioc lock to walk icq_list and q lock to exit icq. Perform + * reverse double locking. Read comment in ioc_release_fn() for + * explanation on the nested locking annotation. + */ +retry: + spin_lock_irqsave_nested(&ioc->lock, flags, 1); + hlist_for_each_entry(icq, n, &ioc->icq_list, ioc_node) { + if (icq->flags & ICQ_EXITED) + continue; + if (spin_trylock(icq->q->queue_lock)) { + ioc_exit_icq(icq); + spin_unlock(icq->q->queue_lock); + } else { + spin_unlock_irqrestore(&ioc->lock, flags); + cpu_relax(); + goto retry; + } + } + spin_unlock_irqrestore(&ioc->lock, flags); + put_io_context(ioc); } @@ -168,7 +207,7 @@ void ioc_clear_queue(struct request_queue *q) struct io_context *ioc = icq->ioc; spin_lock(&ioc->lock); - ioc_exit_icq(icq); + ioc_destroy_icq(icq); spin_unlock(&ioc->lock); } } diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h index 17839c7b9614..1a3018063034 100644 --- a/include/linux/iocontext.h +++ b/include/linux/iocontext.h @@ -8,6 +8,7 @@ enum { ICQ_IOPRIO_CHANGED = 1 << 0, ICQ_CGROUP_CHANGED = 1 << 1, + ICQ_EXITED = 1 << 2, ICQ_CHANGED_MASK = ICQ_IOPRIO_CHANGED | ICQ_CGROUP_CHANGED, }; -- cgit v1.2.3