From b855b04a0b2213dbc1b59cf936056726e7ed97ad Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Tue, 6 Mar 2012 21:24:55 +0100 Subject: block: blk-throttle should be drained regardless of q->elevator Currently, blk_cleanup_queue() doesn't call elv_drain_elevator() if q->elevator doesn't exist; however, bio based drivers don't have elevator initialized but can still use blk-throttle. This patch moves q->elevator test inside blk_drain_queue() such that only elv_drain_elevator() is skipped if !q->elevator. -v2: loop can have registered queue which has NULL request_fn. Make sure we don't call into __blk_run_queue() in such cases. Signed-off-by: Tejun Heo Reported-by: Vivek Goyal Fold in bug fix from Vivek. Signed-off-by: Jens Axboe --- block/blk-core.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 3a78b00edd71..fccb25021121 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -365,17 +365,24 @@ void blk_drain_queue(struct request_queue *q, bool drain_all) spin_lock_irq(q->queue_lock); - elv_drain_elevator(q); + /* + * The caller might be trying to drain @q before its + * elevator is initialized. + */ + if (q->elevator) + elv_drain_elevator(q); + if (drain_all) blk_throtl_drain(q); /* * This function might be called on a queue which failed - * driver init after queue creation. Some drivers - * (e.g. fd) get unhappy in such cases. Kick queue iff - * dispatch queue has something on it. + * driver init after queue creation or is not yet fully + * active yet. Some drivers (e.g. fd and loop) get unhappy + * in such cases. Kick queue iff dispatch queue has + * something on it and @q has request_fn set. */ - if (!list_empty(&q->queue_head)) + if (!list_empty(&q->queue_head) && q->request_fn) __blk_run_queue(q); drain |= q->rq.elvpriv; @@ -428,13 +435,8 @@ void blk_cleanup_queue(struct request_queue *q) spin_unlock_irq(lock); mutex_unlock(&q->sysfs_lock); - /* - * Drain all requests queued before DEAD marking. The caller might - * be trying to tear down @q before its elevator is initialized, in - * which case we don't want to call into draining. - */ - if (q->elevator) - blk_drain_queue(q, true); + /* drain all requests queued before DEAD marking */ + blk_drain_queue(q, true); /* @q won't process any more request, flush async actions */ del_timer_sync(&q->backing_dev_info.laptop_mode_wb_timer); @@ -504,6 +506,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id) setup_timer(&q->backing_dev_info.laptop_mode_wb_timer, laptop_mode_timer_fn, (unsigned long) q); setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q); + INIT_LIST_HEAD(&q->queue_head); INIT_LIST_HEAD(&q->timeout_list); INIT_LIST_HEAD(&q->icq_list); INIT_LIST_HEAD(&q->flush_queue[0]); -- cgit v1.2.2 From 32e380aedc3de454c06ce1c254fe3bea35a676e1 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 5 Mar 2012 13:14:54 -0800 Subject: blkcg: make CONFIG_BLK_CGROUP bool Block cgroup core can be built as module; however, it isn't too useful as blk-throttle can only be built-in and cfq-iosched is usually the default built-in scheduler. Scheduled blkcg cleanup requires calling into blkcg from block core. To simplify that, disallow building blkcg as module by making CONFIG_BLK_CGROUP bool. If building blkcg core as module really matters, which I doubt, we can revisit it after blkcg API cleanup. -v2: Vivek pointed out that IOSCHED_CFQ was incorrectly updated to depend on BLK_CGROUP. Fixed. Signed-off-by: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/Kconfig.iosched | 4 ---- block/blk-cgroup.c | 17 ----------------- block/blk-cgroup.h | 10 ++-------- init/Kconfig | 2 +- 4 files changed, 3 insertions(+), 30 deletions(-) diff --git a/block/Kconfig.iosched b/block/Kconfig.iosched index 3199b76f795d..421bef9c4c48 100644 --- a/block/Kconfig.iosched +++ b/block/Kconfig.iosched @@ -23,8 +23,6 @@ config IOSCHED_DEADLINE config IOSCHED_CFQ tristate "CFQ I/O scheduler" - # If BLK_CGROUP is a module, CFQ has to be built as module. - depends on (BLK_CGROUP=m && m) || !BLK_CGROUP || BLK_CGROUP=y default y ---help--- The CFQ I/O scheduler tries to distribute bandwidth equally @@ -34,8 +32,6 @@ config IOSCHED_CFQ This is the default I/O scheduler. - Note: If BLK_CGROUP=m, then CFQ can be built only as module. - config CFQ_GROUP_IOSCHED bool "CFQ Group Scheduling support" depends on IOSCHED_CFQ && BLK_CGROUP diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 75642a352a8f..970a717a056f 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -50,10 +50,7 @@ struct cgroup_subsys blkio_subsys = { .attach = blkiocg_attach, .destroy = blkiocg_destroy, .populate = blkiocg_populate, -#ifdef CONFIG_BLK_CGROUP - /* note: blkio_subsys_id is otherwise defined in blk-cgroup.h */ .subsys_id = blkio_subsys_id, -#endif .use_id = 1, .module = THIS_MODULE, }; @@ -1679,17 +1676,3 @@ void blkio_policy_unregister(struct blkio_policy_type *blkiop) spin_unlock(&blkio_list_lock); } EXPORT_SYMBOL_GPL(blkio_policy_unregister); - -static int __init init_cgroup_blkio(void) -{ - return cgroup_load_subsys(&blkio_subsys); -} - -static void __exit exit_cgroup_blkio(void) -{ - cgroup_unload_subsys(&blkio_subsys); -} - -module_init(init_cgroup_blkio); -module_exit(exit_cgroup_blkio); -MODULE_LICENSE("GPL"); diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index 6f3ace7e792f..355168772f51 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -24,13 +24,7 @@ enum blkio_policy_id { /* Max limits for throttle policy */ #define THROTL_IOPS_MAX UINT_MAX -#if defined(CONFIG_BLK_CGROUP) || defined(CONFIG_BLK_CGROUP_MODULE) - -#ifndef CONFIG_BLK_CGROUP -/* When blk-cgroup is a module, its subsys_id isn't a compile-time constant */ -extern struct cgroup_subsys blkio_subsys; -#define blkio_subsys_id blkio_subsys.subsys_id -#endif +#ifdef CONFIG_BLK_CGROUP enum stat_type { /* Total time spent (in ns) between request dispatch to the driver and @@ -303,7 +297,7 @@ static inline void blkiocg_update_idle_time_stats(struct blkio_group *blkg) {} static inline void blkiocg_set_start_empty_time(struct blkio_group *blkg) {} #endif -#if defined(CONFIG_BLK_CGROUP) || defined(CONFIG_BLK_CGROUP_MODULE) +#ifdef CONFIG_BLK_CGROUP extern struct blkio_cgroup blkio_root_cgroup; extern struct blkio_cgroup *cgroup_to_blkio_cgroup(struct cgroup *cgroup); extern struct blkio_cgroup *task_blkio_cgroup(struct task_struct *tsk); diff --git a/init/Kconfig b/init/Kconfig index 3f42cd66f0f8..da9222d5ae97 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -766,7 +766,7 @@ config RT_GROUP_SCHED endif #CGROUP_SCHED config BLK_CGROUP - tristate "Block IO controller" + bool "Block IO controller" depends on BLOCK default n ---help--- -- cgit v1.2.2 From b95ada558c9e69c69ffd6950eb644ee8a3dba18f Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 5 Mar 2012 13:14:55 -0800 Subject: cfq: don't register propio policy if !CONFIG_CFQ_GROUP_IOSCHED cfq has been registering zeroed blkio_poilcy_cfq if CFQ_GROUP_IOSCHED is disabled. This fortunately doesn't collide with blk-throtl as BLKIO_POLICY_PROP is zero but is unnecessary and risky. Just don't register it if not enabled. Signed-off-by: Tejun Heo Acked-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/cfq-iosched.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 457295253566..388fe01de18e 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -3878,8 +3878,6 @@ static struct blkio_policy_type blkio_policy_cfq = { }, .plid = BLKIO_POLICY_PROP, }; -#else -static struct blkio_policy_type blkio_policy_cfq; #endif static int __init cfq_init(void) @@ -3910,14 +3908,17 @@ static int __init cfq_init(void) return ret; } +#ifdef CONFIG_CFQ_GROUP_IOSCHED blkio_policy_register(&blkio_policy_cfq); - +#endif return 0; } static void __exit cfq_exit(void) { +#ifdef CONFIG_CFQ_GROUP_IOSCHED blkio_policy_unregister(&blkio_policy_cfq); +#endif elv_unregister(&iosched_cfq); kmem_cache_destroy(cfq_pool); } -- cgit v1.2.2 From 5a5bafdc396b1da7570f84fb96a0f8a288970c5e Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 5 Mar 2012 13:14:56 -0800 Subject: elevator: clear auxiliary data earlier during elevator switch Elevator switch tries hard to keep as much as context until new elevator is ready so that it can revert to the original state if initializing the new elevator fails for some reason. Unfortunately, with more auxiliary contexts to manage, this makes elevator init and exit paths too complex and fragile. This patch makes elevator_switch() unregister the current elevator and flush icq's before start initializing the new one. As we still keep the old elevator itself, the only difference is that we lose icq's on rare occassions of switching failure, which isn't critical at all. Note that this makes explicit elevator parameter to elevator_init_queue() and __elv_register_queue() unnecessary as they always can use the current elevator. This patch enables block cgroup cleanups. -v2: blk_add_trace_msg() prints elevator name from @new_e instead of @e->type as the local variable no longer exists. This caused build failure on CONFIG_BLK_DEV_IO_TRACE. Signed-off-by: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/elevator.c | 90 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/block/elevator.c b/block/elevator.c index f016855a46b0..f8c08e1bff2b 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -121,11 +121,10 @@ static struct elevator_type *elevator_get(const char *name) return e; } -static int elevator_init_queue(struct request_queue *q, - struct elevator_queue *eq) +static int elevator_init_queue(struct request_queue *q) { - eq->elevator_data = eq->type->ops.elevator_init_fn(q); - if (eq->elevator_data) + q->elevator->elevator_data = q->elevator->type->ops.elevator_init_fn(q); + if (q->elevator->elevator_data) return 0; return -ENOMEM; } @@ -188,7 +187,6 @@ static void elevator_release(struct kobject *kobj) int elevator_init(struct request_queue *q, char *name) { struct elevator_type *e = NULL; - struct elevator_queue *eq; int err; if (unlikely(q->elevator)) @@ -222,17 +220,16 @@ int elevator_init(struct request_queue *q, char *name) } } - eq = elevator_alloc(q, e); - if (!eq) + q->elevator = elevator_alloc(q, e); + if (!q->elevator) return -ENOMEM; - err = elevator_init_queue(q, eq); + err = elevator_init_queue(q); if (err) { - kobject_put(&eq->kobj); + kobject_put(&q->elevator->kobj); return err; } - q->elevator = eq; return 0; } EXPORT_SYMBOL(elevator_init); @@ -801,8 +798,9 @@ static struct kobj_type elv_ktype = { .release = elevator_release, }; -int __elv_register_queue(struct request_queue *q, struct elevator_queue *e) +int elv_register_queue(struct request_queue *q) { + struct elevator_queue *e = q->elevator; int error; error = kobject_add(&e->kobj, &q->kobj, "%s", "iosched"); @@ -820,11 +818,6 @@ int __elv_register_queue(struct request_queue *q, struct elevator_queue *e) } return error; } - -int elv_register_queue(struct request_queue *q) -{ - return __elv_register_queue(q, q->elevator); -} EXPORT_SYMBOL(elv_register_queue); void elv_unregister_queue(struct request_queue *q) @@ -907,51 +900,58 @@ EXPORT_SYMBOL_GPL(elv_unregister); */ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e) { - struct elevator_queue *old_elevator, *e; + struct elevator_queue *old = q->elevator; + bool registered = old->registered; int err; - /* allocate new elevator */ - e = elevator_alloc(q, new_e); - if (!e) - return -ENOMEM; - - err = elevator_init_queue(q, e); - if (err) { - kobject_put(&e->kobj); - return err; - } - - /* turn on BYPASS and drain all requests w/ elevator private data */ + /* + * Turn on BYPASS and drain all requests w/ elevator private data. + * Block layer doesn't call into a quiesced elevator - all requests + * are directly put on the dispatch list without elevator data + * using INSERT_BACK. All requests have SOFTBARRIER set and no + * merge happens either. + */ elv_quiesce_start(q); - /* unregister old queue, register new one and kill old elevator */ - if (q->elevator->registered) { + /* unregister and clear all auxiliary data of the old elevator */ + if (registered) elv_unregister_queue(q); - err = __elv_register_queue(q, e); - if (err) - goto fail_register; - } - /* 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); - elevator_exit(old_elevator); + /* allocate, init and register new elevator */ + err = -ENOMEM; + q->elevator = elevator_alloc(q, new_e); + if (!q->elevator) + goto fail_init; + + err = elevator_init_queue(q); + if (err) { + kobject_put(&q->elevator->kobj); + goto fail_init; + } + + if (registered) { + err = elv_register_queue(q); + if (err) + goto fail_register; + } + + /* done, kill the old one and finish */ + elevator_exit(old); elv_quiesce_end(q); - blk_add_trace_msg(q, "elv switch: %s", e->type->elevator_name); + blk_add_trace_msg(q, "elv switch: %s", new_e->elevator_name); return 0; fail_register: - /* - * switch failed, exit the new io scheduler and reattach the old - * one again (along with re-adding the sysfs dir) - */ - elevator_exit(e); + elevator_exit(q->elevator); +fail_init: + /* switch failed, restore and re-register old elevator */ + q->elevator = old; elv_register_queue(q); elv_quiesce_end(q); -- cgit v1.2.2 From b2fab5acd28ead6f0dd6c3996ba23f0ef1772f15 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 5 Mar 2012 13:14:57 -0800 Subject: elevator: make elevator_init_fn() return 0/-errno elevator_ops->elevator_init_fn() has a weird return value. It returns a void * which the caller should assign to q->elevator->elevator_data and %NULL return denotes init failure. Update such that it returns integer 0/-errno and sets elevator_data directly as necessary. This makes the interface more conventional and eases further cleanup. Signed-off-by: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/cfq-iosched.c | 9 +++++---- block/deadline-iosched.c | 8 +++++--- block/elevator.c | 12 ++---------- block/noop-iosched.c | 8 +++++--- include/linux/elevator.h | 2 +- 5 files changed, 18 insertions(+), 21 deletions(-) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 388fe01de18e..72680a6715fc 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -3656,7 +3656,7 @@ static void cfq_exit_queue(struct elevator_queue *e) kfree(cfqd); } -static void *cfq_init_queue(struct request_queue *q) +static int cfq_init_queue(struct request_queue *q) { struct cfq_data *cfqd; int i, j; @@ -3665,7 +3665,7 @@ static void *cfq_init_queue(struct request_queue *q) cfqd = kmalloc_node(sizeof(*cfqd), GFP_KERNEL | __GFP_ZERO, q->node); if (!cfqd) - return NULL; + return -ENOMEM; /* Init root service tree */ cfqd->grp_service_tree = CFQ_RB_ROOT; @@ -3692,7 +3692,7 @@ static void *cfq_init_queue(struct request_queue *q) if (blkio_alloc_blkg_stats(&cfqg->blkg)) { kfree(cfqg); kfree(cfqd); - return NULL; + return -ENOMEM; } rcu_read_lock(); @@ -3723,6 +3723,7 @@ static void *cfq_init_queue(struct request_queue *q) cfq_link_cfqq_cfqg(&cfqd->oom_cfqq, &cfqd->root_group); cfqd->queue = q; + q->elevator->elevator_data = cfqd; init_timer(&cfqd->idle_slice_timer); cfqd->idle_slice_timer.function = cfq_idle_slice_timer; @@ -3747,7 +3748,7 @@ static void *cfq_init_queue(struct request_queue *q) * second, in order to have larger depth for async operations. */ cfqd->last_delayed_sync = jiffies - HZ; - return cfqd; + return 0; } /* diff --git a/block/deadline-iosched.c b/block/deadline-iosched.c index 7bf12d793fcd..599b12e5380f 100644 --- a/block/deadline-iosched.c +++ b/block/deadline-iosched.c @@ -337,13 +337,13 @@ static void deadline_exit_queue(struct elevator_queue *e) /* * initialize elevator private data (deadline_data). */ -static void *deadline_init_queue(struct request_queue *q) +static int deadline_init_queue(struct request_queue *q) { struct deadline_data *dd; dd = kmalloc_node(sizeof(*dd), GFP_KERNEL | __GFP_ZERO, q->node); if (!dd) - return NULL; + return -ENOMEM; INIT_LIST_HEAD(&dd->fifo_list[READ]); INIT_LIST_HEAD(&dd->fifo_list[WRITE]); @@ -354,7 +354,9 @@ static void *deadline_init_queue(struct request_queue *q) dd->writes_starved = writes_starved; dd->front_merges = 1; dd->fifo_batch = fifo_batch; - return dd; + + q->elevator->elevator_data = dd; + return 0; } /* diff --git a/block/elevator.c b/block/elevator.c index f8c08e1bff2b..f81c061dad15 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -121,14 +121,6 @@ static struct elevator_type *elevator_get(const char *name) return e; } -static int elevator_init_queue(struct request_queue *q) -{ - q->elevator->elevator_data = q->elevator->type->ops.elevator_init_fn(q); - if (q->elevator->elevator_data) - return 0; - return -ENOMEM; -} - static char chosen_elevator[ELV_NAME_MAX]; static int __init elevator_setup(char *str) @@ -224,7 +216,7 @@ int elevator_init(struct request_queue *q, char *name) if (!q->elevator) return -ENOMEM; - err = elevator_init_queue(q); + err = e->ops.elevator_init_fn(q); if (err) { kobject_put(&q->elevator->kobj); return err; @@ -927,7 +919,7 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e) if (!q->elevator) goto fail_init; - err = elevator_init_queue(q); + err = new_e->ops.elevator_init_fn(q); if (err) { kobject_put(&q->elevator->kobj); goto fail_init; diff --git a/block/noop-iosched.c b/block/noop-iosched.c index 413a0b1d788c..5d1bf70e33d5 100644 --- a/block/noop-iosched.c +++ b/block/noop-iosched.c @@ -59,15 +59,17 @@ noop_latter_request(struct request_queue *q, struct request *rq) return list_entry(rq->queuelist.next, struct request, queuelist); } -static void *noop_init_queue(struct request_queue *q) +static int noop_init_queue(struct request_queue *q) { struct noop_data *nd; nd = kmalloc_node(sizeof(*nd), GFP_KERNEL, q->node); if (!nd) - return NULL; + return -ENOMEM; + INIT_LIST_HEAD(&nd->queue); - return nd; + q->elevator->elevator_data = nd; + return 0; } static void noop_exit_queue(struct elevator_queue *e) diff --git a/include/linux/elevator.h b/include/linux/elevator.h index 7d4e0356f329..97fb2557a18c 100644 --- a/include/linux/elevator.h +++ b/include/linux/elevator.h @@ -33,7 +33,7 @@ typedef void (elevator_put_req_fn) (struct request *); typedef void (elevator_activate_req_fn) (struct request_queue *, struct request *); typedef void (elevator_deactivate_req_fn) (struct request_queue *, struct request *); -typedef void *(elevator_init_fn) (struct request_queue *); +typedef int (elevator_init_fn) (struct request_queue *); typedef void (elevator_exit_fn) (struct elevator_queue *); struct elevator_ops -- cgit v1.2.2 From d732580b4eb31553c63744a47d590f770cafb8f0 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 5 Mar 2012 13:14:58 -0800 Subject: block: implement blk_queue_bypass_start/end() Rename and extend elv_queisce_start/end() to blk_queue_bypass_start/end() which are exported and supports nesting via @q->bypass_depth. Also add blk_queue_bypass() to test bypass state. This will be further extended and used for blkio_group management. Signed-off-by: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-core.c | 39 +++++++++++++++++++++++++++++++++++++-- block/blk.h | 6 ++---- block/elevator.c | 25 +++---------------------- include/linux/blkdev.h | 5 ++++- 4 files changed, 46 insertions(+), 29 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index fccb25021121..98ddef430093 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -409,6 +409,42 @@ void blk_drain_queue(struct request_queue *q, bool drain_all) } } +/** + * blk_queue_bypass_start - enter queue bypass mode + * @q: queue of interest + * + * In bypass mode, only the dispatch FIFO queue of @q is used. This + * function makes @q enter bypass mode and drains all requests which were + * issued before. On return, it's guaranteed that no request has ELVPRIV + * set. + */ +void blk_queue_bypass_start(struct request_queue *q) +{ + spin_lock_irq(q->queue_lock); + q->bypass_depth++; + queue_flag_set(QUEUE_FLAG_BYPASS, q); + spin_unlock_irq(q->queue_lock); + + blk_drain_queue(q, false); +} +EXPORT_SYMBOL_GPL(blk_queue_bypass_start); + +/** + * blk_queue_bypass_end - leave queue bypass mode + * @q: queue of interest + * + * Leave bypass mode and restore the normal queueing behavior. + */ +void blk_queue_bypass_end(struct request_queue *q) +{ + spin_lock_irq(q->queue_lock); + if (!--q->bypass_depth) + queue_flag_clear(QUEUE_FLAG_BYPASS, q); + WARN_ON_ONCE(q->bypass_depth < 0); + spin_unlock_irq(q->queue_lock); +} +EXPORT_SYMBOL_GPL(blk_queue_bypass_end); + /** * blk_cleanup_queue - shutdown a request queue * @q: request queue to shutdown @@ -862,8 +898,7 @@ retry: * 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)) { + if (blk_rq_should_init_elevator(bio) && !blk_queue_bypass(q)) { rw_flags |= REQ_ELVPRIV; rl->elvpriv++; if (et->icq_cache && ioc) diff --git a/block/blk.h b/block/blk.h index 9c12f80882b0..7422f3133c5d 100644 --- a/block/blk.h +++ b/block/blk.h @@ -23,7 +23,8 @@ void blk_rq_bio_prep(struct request_queue *q, struct request *rq, struct bio *bio); int blk_rq_append_bio(struct request_queue *q, struct request *rq, struct bio *bio); -void blk_drain_queue(struct request_queue *q, bool drain_all); +void blk_queue_bypass_start(struct request_queue *q); +void blk_queue_bypass_end(struct request_queue *q); void blk_dequeue_request(struct request *rq); void __blk_queue_free_tags(struct request_queue *q); bool __blk_end_bidi_request(struct request *rq, int error, @@ -144,9 +145,6 @@ void blk_queue_congestion_threshold(struct request_queue *q); int blk_dev_init(void); -void elv_quiesce_start(struct request_queue *q); -void elv_quiesce_end(struct request_queue *q); - /* * Return the threshold (number of used requests) at which the queue is diff --git a/block/elevator.c b/block/elevator.c index f81c061dad15..0bdea0ed03a3 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -553,25 +553,6 @@ void elv_drain_elevator(struct request_queue *q) } } -void elv_quiesce_start(struct request_queue *q) -{ - if (!q->elevator) - return; - - spin_lock_irq(q->queue_lock); - queue_flag_set(QUEUE_FLAG_ELVSWITCH, q); - spin_unlock_irq(q->queue_lock); - - blk_drain_queue(q, false); -} - -void elv_quiesce_end(struct request_queue *q) -{ - spin_lock_irq(q->queue_lock); - queue_flag_clear(QUEUE_FLAG_ELVSWITCH, q); - spin_unlock_irq(q->queue_lock); -} - void __elv_add_request(struct request_queue *q, struct request *rq, int where) { trace_block_rq_insert(q, rq); @@ -903,7 +884,7 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e) * using INSERT_BACK. All requests have SOFTBARRIER set and no * merge happens either. */ - elv_quiesce_start(q); + blk_queue_bypass_start(q); /* unregister and clear all auxiliary data of the old elevator */ if (registered) @@ -933,7 +914,7 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e) /* done, kill the old one and finish */ elevator_exit(old); - elv_quiesce_end(q); + blk_queue_bypass_end(q); blk_add_trace_msg(q, "elv switch: %s", new_e->elevator_name); @@ -945,7 +926,7 @@ fail_init: /* switch failed, restore and re-register old elevator */ q->elevator = old; elv_register_queue(q); - elv_quiesce_end(q); + blk_queue_bypass_end(q); return err; } diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 606cf339bb56..315db1d91bc4 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -389,6 +389,8 @@ struct request_queue { struct mutex sysfs_lock; + int bypass_depth; + #if defined(CONFIG_BLK_DEV_BSG) bsg_job_fn *bsg_job_fn; int bsg_job_size; @@ -406,7 +408,7 @@ struct request_queue { #define QUEUE_FLAG_SYNCFULL 3 /* read queue has been filled */ #define QUEUE_FLAG_ASYNCFULL 4 /* write queue has been filled */ #define QUEUE_FLAG_DEAD 5 /* queue being torn down */ -#define QUEUE_FLAG_ELVSWITCH 6 /* don't use elevator, just do FIFO */ +#define QUEUE_FLAG_BYPASS 6 /* act as dumb FIFO queue */ #define QUEUE_FLAG_BIDI 7 /* queue supports bidi requests */ #define QUEUE_FLAG_NOMERGES 8 /* disable merge attempts */ #define QUEUE_FLAG_SAME_COMP 9 /* complete on same CPU-group */ @@ -494,6 +496,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q) #define blk_queue_tagged(q) test_bit(QUEUE_FLAG_QUEUED, &(q)->queue_flags) #define blk_queue_stopped(q) test_bit(QUEUE_FLAG_STOPPED, &(q)->queue_flags) #define blk_queue_dead(q) test_bit(QUEUE_FLAG_DEAD, &(q)->queue_flags) +#define blk_queue_bypass(q) test_bit(QUEUE_FLAG_BYPASS, &(q)->queue_flags) #define blk_queue_nomerges(q) test_bit(QUEUE_FLAG_NOMERGES, &(q)->queue_flags) #define blk_queue_noxmerges(q) \ test_bit(QUEUE_FLAG_NOXMERGES, &(q)->queue_flags) -- cgit v1.2.2 From 6ecf23afab13c39d3bb0e2d826d0984b0dd53733 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 5 Mar 2012 13:14:59 -0800 Subject: block: extend queue bypassing to cover blkcg policies Extend queue bypassing such that dying queue is always bypassing and blk-throttle is drained on bypass. With blkcg policies updated to test blk_queue_bypass() instead of blk_queue_dead(), this ensures that no bio or request is held by or going through blkcg policies on a bypassing queue. This will be used to implement blkg cleanup on elevator switches and policy changes. Signed-off-by: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-core.c | 12 ++++++++---- block/blk-throttle.c | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 98ddef430093..7713c73d9590 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -372,8 +372,7 @@ void blk_drain_queue(struct request_queue *q, bool drain_all) if (q->elevator) elv_drain_elevator(q); - if (drain_all) - blk_throtl_drain(q); + blk_throtl_drain(q); /* * This function might be called on a queue which failed @@ -415,8 +414,8 @@ void blk_drain_queue(struct request_queue *q, bool drain_all) * * In bypass mode, only the dispatch FIFO queue of @q is used. This * function makes @q enter bypass mode and drains all requests which were - * issued before. On return, it's guaranteed that no request has ELVPRIV - * set. + * throttled or issued before. On return, it's guaranteed that no request + * is being throttled or has ELVPRIV set. */ void blk_queue_bypass_start(struct request_queue *q) { @@ -461,6 +460,11 @@ void blk_cleanup_queue(struct request_queue *q) queue_flag_set_unlocked(QUEUE_FLAG_DEAD, q); spin_lock_irq(lock); + + /* dead queue is permanently in bypass mode till released */ + q->bypass_depth++; + queue_flag_set(QUEUE_FLAG_BYPASS, q); + queue_flag_set(QUEUE_FLAG_NOMERGES, q); queue_flag_set(QUEUE_FLAG_NOXMERGES, q); queue_flag_set(QUEUE_FLAG_DEAD, q); diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 5eed6a76721d..702c0e64e09f 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -310,7 +310,7 @@ static struct throtl_grp * throtl_get_tg(struct throtl_data *td) struct request_queue *q = td->queue; /* no throttling for dead queue */ - if (unlikely(blk_queue_dead(q))) + if (unlikely(blk_queue_bypass(q))) return NULL; rcu_read_lock(); @@ -335,7 +335,7 @@ static struct throtl_grp * throtl_get_tg(struct throtl_data *td) spin_lock_irq(q->queue_lock); /* Make sure @q is still alive */ - if (unlikely(blk_queue_dead(q))) { + if (unlikely(blk_queue_bypass(q))) { kfree(tg); return NULL; } -- cgit v1.2.2 From 72e06c255181537d0b3e1f657a9ed81655d745b1 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 5 Mar 2012 13:15:00 -0800 Subject: blkcg: shoot down blkio_groups on elevator switch Elevator switch may involve changes to blkcg policies. Implement shoot down of blkio_groups. Combined with the previous bypass updates, the end goal is updating blkcg core such that it can ensure that blkcg's being affected become quiescent and don't have any per-blkg data hanging around before commencing any policy updates. Until queues are made aware of the policies that applies to them, as an interim step, all per-policy blkg data will be shot down. * blk-throtl doesn't need this change as it can't be disabled for a live queue; however, update it anyway as the scheduled blkg unification requires this behavior change. This means that blk-throtl configuration will be unnecessarily lost over elevator switch. This oddity will be removed after blkcg learns to associate individual policies with request_queues. * blk-throtl dosen't shoot down root_tg. This is to ease transition. Unified blkg will always have persistent root group and not shooting down root_tg for now eases transition to that point by avoiding having to update td->root_tg and is safe as blk-throtl can never be disabled -v2: Vivek pointed out that group list is not guaranteed to be empty on return from clear function if it raced cgroup removal and lost. Fix it by waiting a bit and retrying. This kludge will soon be removed once locking is updated such that blkg is never in limbo state between blkcg and request_queue locks. blk-throtl no longer shoots down root_tg to avoid breaking td->root_tg. Also, Nest queue_lock inside blkio_list_lock not the other way around to avoid introduce possible deadlock via blkcg lock. -v3: blkcg_clear_queue() repositioned and renamed to blkg_destroy_all() to increase consistency with later changes. cfq_clear_queue() updated to check q->elevator before dereferencing it to avoid NULL dereference on not fully initialized queues (used by later change). Signed-off-by: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 34 +++++++++++++++++++++++++++++++++- block/blk-cgroup.h | 5 ++++- block/blk-throttle.c | 27 +++++++++++++++++++++++++-- block/cfq-iosched.c | 20 +++++++++++++++++++- block/elevator.c | 3 +++ 5 files changed, 84 insertions(+), 5 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 970a717a056f..159aef59589f 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -17,8 +17,9 @@ #include #include #include -#include "blk-cgroup.h" #include +#include +#include "blk-cgroup.h" #define MAX_KEY_LEN 100 @@ -546,6 +547,37 @@ struct blkio_group *blkiocg_lookup_group(struct blkio_cgroup *blkcg, void *key) } EXPORT_SYMBOL_GPL(blkiocg_lookup_group); +void blkg_destroy_all(struct request_queue *q) +{ + struct blkio_policy_type *pol; + + while (true) { + bool done = true; + + spin_lock(&blkio_list_lock); + spin_lock_irq(q->queue_lock); + + /* + * clear_queue_fn() might return with non-empty group list + * if it raced cgroup removal and lost. cgroup removal is + * guaranteed to make forward progress and retrying after a + * while is enough. This ugliness is scheduled to be + * removed after locking update. + */ + list_for_each_entry(pol, &blkio_list, list) + if (!pol->ops.blkio_clear_queue_fn(q)) + done = false; + + spin_unlock_irq(q->queue_lock); + spin_unlock(&blkio_list_lock); + + if (done) + break; + + msleep(10); /* just some random duration I like */ + } +} + static void blkio_reset_stats_cpu(struct blkio_group *blkg) { struct blkio_group_stats_cpu *stats_cpu; diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index 355168772f51..e5cfcbd4d2f4 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -203,7 +203,7 @@ extern unsigned int blkcg_get_write_iops(struct blkio_cgroup *blkcg, dev_t dev); typedef void (blkio_unlink_group_fn) (void *key, struct blkio_group *blkg); - +typedef bool (blkio_clear_queue_fn)(struct request_queue *q); typedef void (blkio_update_group_weight_fn) (void *key, struct blkio_group *blkg, unsigned int weight); typedef void (blkio_update_group_read_bps_fn) (void * key, @@ -217,6 +217,7 @@ typedef void (blkio_update_group_write_iops_fn) (void *key, struct blkio_policy_ops { blkio_unlink_group_fn *blkio_unlink_group_fn; + blkio_clear_queue_fn *blkio_clear_queue_fn; blkio_update_group_weight_fn *blkio_update_group_weight_fn; blkio_update_group_read_bps_fn *blkio_update_group_read_bps_fn; blkio_update_group_write_bps_fn *blkio_update_group_write_bps_fn; @@ -233,6 +234,7 @@ struct blkio_policy_type { /* Blkio controller policy registration */ extern void blkio_policy_register(struct blkio_policy_type *); extern void blkio_policy_unregister(struct blkio_policy_type *); +extern void blkg_destroy_all(struct request_queue *q); static inline char *blkg_path(struct blkio_group *blkg) { @@ -249,6 +251,7 @@ struct blkio_policy_type { static inline void blkio_policy_register(struct blkio_policy_type *blkiop) { } static inline void blkio_policy_unregister(struct blkio_policy_type *blkiop) { } +static inline void blkg_destroy_all(struct request_queue *q) { } static inline char *blkg_path(struct blkio_group *blkg) { return NULL; } diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 702c0e64e09f..3699ab40d494 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -989,12 +989,17 @@ throtl_destroy_tg(struct throtl_data *td, struct throtl_grp *tg) td->nr_undestroyed_grps--; } -static void throtl_release_tgs(struct throtl_data *td) +static bool throtl_release_tgs(struct throtl_data *td, bool release_root) { struct hlist_node *pos, *n; struct throtl_grp *tg; + bool empty = true; hlist_for_each_entry_safe(tg, pos, n, &td->tg_list, tg_node) { + /* skip root? */ + if (!release_root && tg == td->root_tg) + continue; + /* * If cgroup removal path got to blk_group first and removed * it from cgroup list, then it will take care of destroying @@ -1002,7 +1007,10 @@ static void throtl_release_tgs(struct throtl_data *td) */ if (!blkiocg_del_blkio_group(&tg->blkg)) throtl_destroy_tg(td, tg); + else + empty = false; } + return empty; } /* @@ -1029,6 +1037,20 @@ void throtl_unlink_blkio_group(void *key, struct blkio_group *blkg) spin_unlock_irqrestore(td->queue->queue_lock, flags); } +static bool throtl_clear_queue(struct request_queue *q) +{ + lockdep_assert_held(q->queue_lock); + + /* + * Clear tgs but leave the root one alone. This is necessary + * because root_tg is expected to be persistent and safe because + * blk-throtl can never be disabled while @q is alive. This is a + * kludge to prepare for unified blkg. This whole function will be + * removed soon. + */ + return throtl_release_tgs(q->td, false); +} + static void throtl_update_blkio_group_common(struct throtl_data *td, struct throtl_grp *tg) { @@ -1097,6 +1119,7 @@ static void throtl_shutdown_wq(struct request_queue *q) static struct blkio_policy_type blkio_policy_throtl = { .ops = { .blkio_unlink_group_fn = throtl_unlink_blkio_group, + .blkio_clear_queue_fn = throtl_clear_queue, .blkio_update_group_read_bps_fn = throtl_update_blkio_group_read_bps, .blkio_update_group_write_bps_fn = @@ -1282,7 +1305,7 @@ void blk_throtl_exit(struct request_queue *q) throtl_shutdown_wq(q); spin_lock_irq(q->queue_lock); - throtl_release_tgs(td); + throtl_release_tgs(td, true); /* If there are other groups */ if (td->nr_undestroyed_grps > 0) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 72680a6715fc..61693d3404d0 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -1225,10 +1225,11 @@ static void cfq_destroy_cfqg(struct cfq_data *cfqd, struct cfq_group *cfqg) cfq_put_cfqg(cfqg); } -static void cfq_release_cfq_groups(struct cfq_data *cfqd) +static bool cfq_release_cfq_groups(struct cfq_data *cfqd) { struct hlist_node *pos, *n; struct cfq_group *cfqg; + bool empty = true; hlist_for_each_entry_safe(cfqg, pos, n, &cfqd->cfqg_list, cfqd_node) { /* @@ -1238,7 +1239,10 @@ static void cfq_release_cfq_groups(struct cfq_data *cfqd) */ if (!cfq_blkiocg_del_blkio_group(&cfqg->blkg)) cfq_destroy_cfqg(cfqd, cfqg); + else + empty = false; } + return empty; } /* @@ -1265,6 +1269,19 @@ static void cfq_unlink_blkio_group(void *key, struct blkio_group *blkg) spin_unlock_irqrestore(cfqd->queue->queue_lock, flags); } +static struct elevator_type iosched_cfq; + +static bool cfq_clear_queue(struct request_queue *q) +{ + lockdep_assert_held(q->queue_lock); + + /* shoot down blkgs iff the current elevator is cfq */ + if (!q->elevator || q->elevator->type != &iosched_cfq) + return true; + + return cfq_release_cfq_groups(q->elevator->elevator_data); +} + #else /* GROUP_IOSCHED */ static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd) { @@ -3875,6 +3892,7 @@ static struct elevator_type iosched_cfq = { static struct blkio_policy_type blkio_policy_cfq = { .ops = { .blkio_unlink_group_fn = cfq_unlink_blkio_group, + .blkio_clear_queue_fn = cfq_clear_queue, .blkio_update_group_weight_fn = cfq_update_blkio_group_weight, }, .plid = BLKIO_POLICY_PROP, diff --git a/block/elevator.c b/block/elevator.c index 0bdea0ed03a3..8c7561fd2c79 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -38,6 +38,7 @@ #include #include "blk.h" +#include "blk-cgroup.h" static DEFINE_SPINLOCK(elv_list_lock); static LIST_HEAD(elv_list); @@ -894,6 +895,8 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e) ioc_clear_queue(q); spin_unlock_irq(q->queue_lock); + blkg_destroy_all(q); + /* allocate, init and register new elevator */ err = -ENOMEM; q->elevator = elevator_alloc(q, new_e); -- cgit v1.2.2 From 2a7f124414b35645049e9c1b125a6f0b470aa5ae Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 5 Mar 2012 13:15:01 -0800 Subject: blkcg: move rcu_read_lock() outside of blkio_group get functions rcu_read_lock() in throtl_get_tb() and cfq_get_cfqg() holds onto @blkcg while looking up blkg. For API cleanup, the next patch will make the caller responsible for determining @blkcg to look blkg from and let them specify it as a parameter. Move rcu read locking out to the callers to prepare for the change. -v2: Originally this patch was described as a fix for RCU read locking bug around @blkg, which Vivek pointed out to be incorrect. It was from misunderstanding the role of rcu locking as protecting @blkg not @blkcg. Patch description updated. Signed-off-by: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-throttle.c | 18 ++++++------------ block/cfq-iosched.c | 11 +++++------ 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 3699ab40d494..9beaac7fb397 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -313,25 +313,23 @@ static struct throtl_grp * throtl_get_tg(struct throtl_data *td) if (unlikely(blk_queue_bypass(q))) return NULL; - rcu_read_lock(); blkcg = task_blkio_cgroup(current); tg = throtl_find_tg(td, blkcg); - if (tg) { - rcu_read_unlock(); + if (tg) return tg; - } /* * Need to allocate a group. Allocation of group also needs allocation * of per cpu stats which in-turn takes a mutex() and can block. Hence * we need to drop rcu lock and queue_lock before we call alloc. */ - rcu_read_unlock(); spin_unlock_irq(q->queue_lock); + rcu_read_unlock(); tg = throtl_alloc_tg(td); /* Group allocated and queue is still alive. take the lock */ + rcu_read_lock(); spin_lock_irq(q->queue_lock); /* Make sure @q is still alive */ @@ -343,7 +341,6 @@ static struct throtl_grp * throtl_get_tg(struct throtl_data *td) /* * Initialize the new group. After sleeping, read the blkcg again. */ - rcu_read_lock(); blkcg = task_blkio_cgroup(current); /* @@ -354,7 +351,6 @@ static struct throtl_grp * throtl_get_tg(struct throtl_data *td) if (__tg) { kfree(tg); - rcu_read_unlock(); return __tg; } @@ -365,7 +361,6 @@ static struct throtl_grp * throtl_get_tg(struct throtl_data *td) } throtl_init_add_tg_lists(td, tg, blkcg); - rcu_read_unlock(); return tg; } @@ -1150,7 +1145,6 @@ bool blk_throtl_bio(struct request_queue *q, struct bio *bio) * basic fields like stats and io rates. If a group has no rules, * just update the dispatch stats in lockless manner and return. */ - rcu_read_lock(); blkcg = task_blkio_cgroup(current); tg = throtl_find_tg(td, blkcg); @@ -1160,11 +1154,9 @@ bool blk_throtl_bio(struct request_queue *q, struct bio *bio) if (tg_no_rule_group(tg, rw)) { blkiocg_update_dispatch_stats(&tg->blkg, bio->bi_size, rw, rw_is_sync(bio->bi_rw)); - rcu_read_unlock(); - goto out; + goto out_unlock_rcu; } } - rcu_read_unlock(); /* * Either group has not been allocated yet or it is not an unlimited @@ -1222,6 +1214,8 @@ queue_bio: out_unlock: spin_unlock_irq(q->queue_lock); +out_unlock_rcu: + rcu_read_unlock(); out: return throttled; } diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 61693d3404d0..6063c4482b86 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -1128,13 +1128,10 @@ static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd) struct cfq_group *cfqg = NULL, *__cfqg = NULL; struct request_queue *q = cfqd->queue; - rcu_read_lock(); blkcg = task_blkio_cgroup(current); cfqg = cfq_find_cfqg(cfqd, blkcg); - if (cfqg) { - rcu_read_unlock(); + if (cfqg) return cfqg; - } /* * Need to allocate a group. Allocation of group also needs allocation @@ -1164,7 +1161,6 @@ static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd) if (__cfqg) { kfree(cfqg); - rcu_read_unlock(); return __cfqg; } @@ -1172,7 +1168,6 @@ static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd) cfqg = &cfqd->root_group; cfq_init_add_cfqg_lists(cfqd, cfqg, blkcg); - rcu_read_unlock(); return cfqg; } @@ -2870,6 +2865,8 @@ cfq_find_alloc_queue(struct cfq_data *cfqd, bool is_sync, struct cfq_group *cfqg; retry: + rcu_read_lock(); + cfqg = cfq_get_cfqg(cfqd); cic = cfq_cic_lookup(cfqd, ioc); /* cic always exists here */ @@ -2885,6 +2882,7 @@ retry: cfqq = new_cfqq; new_cfqq = NULL; } else if (gfp_mask & __GFP_WAIT) { + rcu_read_unlock(); spin_unlock_irq(cfqd->queue->queue_lock); new_cfqq = kmem_cache_alloc_node(cfq_pool, gfp_mask | __GFP_ZERO, @@ -2910,6 +2908,7 @@ retry: if (new_cfqq) kmem_cache_free(cfq_pool, new_cfqq); + rcu_read_unlock(); return cfqq; } -- cgit v1.2.2 From 0a5a7d0e32be6643b881f0e7cd9d0d06fadde27a Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 5 Mar 2012 13:15:02 -0800 Subject: blkcg: update blkg get functions take blkio_cgroup as parameter In both blkg get functions - throtl_get_tg() and cfq_get_cfqg(), instead of obtaining blkcg of %current explicitly, let the caller specify the blkcg to use as parameter and make both functions hold on to the blkcg. This is part of block cgroup interface cleanup and will help making blkcg API more modular. Signed-off-by: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-throttle.c | 16 +++++++--------- block/cfq-iosched.c | 20 ++++++++++++-------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 9beaac7fb397..c252df9169db 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -303,21 +303,23 @@ throtl_grp *throtl_find_tg(struct throtl_data *td, struct blkio_cgroup *blkcg) return tg; } -static struct throtl_grp * throtl_get_tg(struct throtl_data *td) +static struct throtl_grp *throtl_get_tg(struct throtl_data *td, + struct blkio_cgroup *blkcg) { struct throtl_grp *tg = NULL, *__tg = NULL; - struct blkio_cgroup *blkcg; struct request_queue *q = td->queue; /* no throttling for dead queue */ if (unlikely(blk_queue_bypass(q))) return NULL; - blkcg = task_blkio_cgroup(current); tg = throtl_find_tg(td, blkcg); if (tg) return tg; + if (!css_tryget(&blkcg->css)) + return NULL; + /* * Need to allocate a group. Allocation of group also needs allocation * of per cpu stats which in-turn takes a mutex() and can block. Hence @@ -331,6 +333,7 @@ static struct throtl_grp * throtl_get_tg(struct throtl_data *td) /* Group allocated and queue is still alive. take the lock */ rcu_read_lock(); spin_lock_irq(q->queue_lock); + css_put(&blkcg->css); /* Make sure @q is still alive */ if (unlikely(blk_queue_bypass(q))) { @@ -338,11 +341,6 @@ static struct throtl_grp * throtl_get_tg(struct throtl_data *td) return NULL; } - /* - * Initialize the new group. After sleeping, read the blkcg again. - */ - blkcg = task_blkio_cgroup(current); - /* * If some other thread already allocated the group while we were * not holding queue lock, free up the group @@ -1163,7 +1161,7 @@ bool blk_throtl_bio(struct request_queue *q, struct bio *bio) * IO group */ spin_lock_irq(q->queue_lock); - tg = throtl_get_tg(td); + tg = throtl_get_tg(td, blkcg); if (unlikely(!tg)) goto out_unlock; diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 6063c4482b86..0f7a81fc7c73 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -1122,17 +1122,19 @@ cfq_find_cfqg(struct cfq_data *cfqd, struct blkio_cgroup *blkcg) * Search for the cfq group current task belongs to. request_queue lock must * be held. */ -static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd) +static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd, + struct blkio_cgroup *blkcg) { - struct blkio_cgroup *blkcg; struct cfq_group *cfqg = NULL, *__cfqg = NULL; struct request_queue *q = cfqd->queue; - blkcg = task_blkio_cgroup(current); cfqg = cfq_find_cfqg(cfqd, blkcg); if (cfqg) return cfqg; + if (!css_tryget(&blkcg->css)) + return NULL; + /* * Need to allocate a group. Allocation of group also needs allocation * of per cpu stats which in-turn takes a mutex() and can block. Hence @@ -1142,16 +1144,14 @@ static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd) * around by the time we return. CFQ queue allocation code does * the same. It might be racy though. */ - rcu_read_unlock(); spin_unlock_irq(q->queue_lock); cfqg = cfq_alloc_cfqg(cfqd); spin_lock_irq(q->queue_lock); - rcu_read_lock(); - blkcg = task_blkio_cgroup(current); + css_put(&blkcg->css); /* * If some other thread already allocated the group while we were @@ -1278,7 +1278,8 @@ static bool cfq_clear_queue(struct request_queue *q) } #else /* GROUP_IOSCHED */ -static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd) +static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd, + struct blkio_cgroup *blkcg) { return &cfqd->root_group; } @@ -2860,6 +2861,7 @@ static struct cfq_queue * cfq_find_alloc_queue(struct cfq_data *cfqd, bool is_sync, struct io_context *ioc, gfp_t gfp_mask) { + struct blkio_cgroup *blkcg; struct cfq_queue *cfqq, *new_cfqq = NULL; struct cfq_io_cq *cic; struct cfq_group *cfqg; @@ -2867,7 +2869,9 @@ cfq_find_alloc_queue(struct cfq_data *cfqd, bool is_sync, retry: rcu_read_lock(); - cfqg = cfq_get_cfqg(cfqd); + blkcg = task_blkio_cgroup(current); + + cfqg = cfq_get_cfqg(cfqd, blkcg); cic = cfq_cic_lookup(cfqd, ioc); /* cic always exists here */ cfqq = cic_to_cfqq(cic, is_sync); -- cgit v1.2.2 From ca32aefc7f2539ed88d42763330d54ee3e61769a Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 5 Mar 2012 13:15:03 -0800 Subject: blkcg: use q and plid instead of opaque void * for blkio_group association blkgio_group is association between a block cgroup and a queue for a given policy. Using opaque void * for association makes things confusing and hinders factoring of common code. Use request_queue * and, if necessary, policy id instead. This will help block cgroup API cleanup. Signed-off-by: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 32 +++++++++++++++----------------- block/blk-cgroup.h | 22 ++++++++++++---------- block/blk-throttle.c | 50 +++++++++++++++++++++++--------------------------- block/cfq-iosched.c | 30 ++++++++++++++++-------------- block/cfq.h | 7 ++++--- 5 files changed, 70 insertions(+), 71 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 159aef59589f..5e50ca1f5b47 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -129,7 +129,7 @@ blkio_update_group_weight(struct blkio_group *blkg, unsigned int weight) if (blkiop->plid != blkg->plid) continue; if (blkiop->ops.blkio_update_group_weight_fn) - blkiop->ops.blkio_update_group_weight_fn(blkg->key, + blkiop->ops.blkio_update_group_weight_fn(blkg->q, blkg, weight); } } @@ -147,12 +147,12 @@ static inline void blkio_update_group_bps(struct blkio_group *blkg, u64 bps, if (fileid == BLKIO_THROTL_read_bps_device && blkiop->ops.blkio_update_group_read_bps_fn) - blkiop->ops.blkio_update_group_read_bps_fn(blkg->key, + blkiop->ops.blkio_update_group_read_bps_fn(blkg->q, blkg, bps); if (fileid == BLKIO_THROTL_write_bps_device && blkiop->ops.blkio_update_group_write_bps_fn) - blkiop->ops.blkio_update_group_write_bps_fn(blkg->key, + blkiop->ops.blkio_update_group_write_bps_fn(blkg->q, blkg, bps); } } @@ -170,12 +170,12 @@ static inline void blkio_update_group_iops(struct blkio_group *blkg, if (fileid == BLKIO_THROTL_read_iops_device && blkiop->ops.blkio_update_group_read_iops_fn) - blkiop->ops.blkio_update_group_read_iops_fn(blkg->key, + blkiop->ops.blkio_update_group_read_iops_fn(blkg->q, blkg, iops); if (fileid == BLKIO_THROTL_write_iops_device && blkiop->ops.blkio_update_group_write_iops_fn) - blkiop->ops.blkio_update_group_write_iops_fn(blkg->key, + blkiop->ops.blkio_update_group_write_iops_fn(blkg->q, blkg,iops); } } @@ -478,14 +478,14 @@ int blkio_alloc_blkg_stats(struct blkio_group *blkg) EXPORT_SYMBOL_GPL(blkio_alloc_blkg_stats); void blkiocg_add_blkio_group(struct blkio_cgroup *blkcg, - struct blkio_group *blkg, void *key, dev_t dev, + struct blkio_group *blkg, struct request_queue *q, dev_t dev, enum blkio_policy_id plid) { unsigned long flags; spin_lock_irqsave(&blkcg->lock, flags); spin_lock_init(&blkg->stats_lock); - rcu_assign_pointer(blkg->key, key); + rcu_assign_pointer(blkg->q, q); blkg->blkcg_id = css_id(&blkcg->css); hlist_add_head_rcu(&blkg->blkcg_node, &blkcg->blkg_list); blkg->plid = plid; @@ -531,18 +531,16 @@ int blkiocg_del_blkio_group(struct blkio_group *blkg) EXPORT_SYMBOL_GPL(blkiocg_del_blkio_group); /* called under rcu_read_lock(). */ -struct blkio_group *blkiocg_lookup_group(struct blkio_cgroup *blkcg, void *key) +struct blkio_group *blkiocg_lookup_group(struct blkio_cgroup *blkcg, + struct request_queue *q, + enum blkio_policy_id plid) { struct blkio_group *blkg; struct hlist_node *n; - void *__key; - hlist_for_each_entry_rcu(blkg, n, &blkcg->blkg_list, blkcg_node) { - __key = blkg->key; - if (__key == key) + hlist_for_each_entry_rcu(blkg, n, &blkcg->blkg_list, blkcg_node) + if (blkg->q == q && blkg->plid == plid) return blkg; - } - return NULL; } EXPORT_SYMBOL_GPL(blkiocg_lookup_group); @@ -1582,7 +1580,7 @@ static void blkiocg_destroy(struct cgroup_subsys *subsys, struct cgroup *cgroup) struct blkio_cgroup *blkcg = cgroup_to_blkio_cgroup(cgroup); unsigned long flags; struct blkio_group *blkg; - void *key; + struct request_queue *q; struct blkio_policy_type *blkiop; struct blkio_policy_node *pn, *pntmp; @@ -1597,7 +1595,7 @@ static void blkiocg_destroy(struct cgroup_subsys *subsys, struct cgroup *cgroup) blkg = hlist_entry(blkcg->blkg_list.first, struct blkio_group, blkcg_node); - key = rcu_dereference(blkg->key); + q = rcu_dereference(blkg->q); __blkiocg_del_blkio_group(blkg); spin_unlock_irqrestore(&blkcg->lock, flags); @@ -1611,7 +1609,7 @@ static void blkiocg_destroy(struct cgroup_subsys *subsys, struct cgroup *cgroup) list_for_each_entry(blkiop, &blkio_list, list) { if (blkiop->plid != blkg->plid) continue; - blkiop->ops.blkio_unlink_group_fn(key, blkg); + blkiop->ops.blkio_unlink_group_fn(q, blkg); } spin_unlock(&blkio_list_lock); } while (1); diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index e5cfcbd4d2f4..41c960b99c57 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -153,8 +153,8 @@ struct blkio_group_stats_cpu { }; struct blkio_group { - /* An rcu protected unique identifier for the group */ - void *key; + /* Pointer to the associated request_queue, RCU protected */ + struct request_queue __rcu *q; struct hlist_node blkcg_node; unsigned short blkcg_id; /* Store cgroup path */ @@ -202,17 +202,18 @@ extern unsigned int blkcg_get_read_iops(struct blkio_cgroup *blkcg, extern unsigned int blkcg_get_write_iops(struct blkio_cgroup *blkcg, dev_t dev); -typedef void (blkio_unlink_group_fn) (void *key, struct blkio_group *blkg); +typedef void (blkio_unlink_group_fn)(struct request_queue *q, + struct blkio_group *blkg); typedef bool (blkio_clear_queue_fn)(struct request_queue *q); -typedef void (blkio_update_group_weight_fn) (void *key, +typedef void (blkio_update_group_weight_fn)(struct request_queue *q, struct blkio_group *blkg, unsigned int weight); -typedef void (blkio_update_group_read_bps_fn) (void * key, +typedef void (blkio_update_group_read_bps_fn)(struct request_queue *q, struct blkio_group *blkg, u64 read_bps); -typedef void (blkio_update_group_write_bps_fn) (void *key, +typedef void (blkio_update_group_write_bps_fn)(struct request_queue *q, struct blkio_group *blkg, u64 write_bps); -typedef void (blkio_update_group_read_iops_fn) (void *key, +typedef void (blkio_update_group_read_iops_fn)(struct request_queue *q, struct blkio_group *blkg, unsigned int read_iops); -typedef void (blkio_update_group_write_iops_fn) (void *key, +typedef void (blkio_update_group_write_iops_fn)(struct request_queue *q, struct blkio_group *blkg, unsigned int write_iops); struct blkio_policy_ops { @@ -305,12 +306,13 @@ extern struct blkio_cgroup blkio_root_cgroup; extern struct blkio_cgroup *cgroup_to_blkio_cgroup(struct cgroup *cgroup); extern struct blkio_cgroup *task_blkio_cgroup(struct task_struct *tsk); extern void blkiocg_add_blkio_group(struct blkio_cgroup *blkcg, - struct blkio_group *blkg, void *key, dev_t dev, + struct blkio_group *blkg, struct request_queue *q, dev_t dev, enum blkio_policy_id plid); extern int blkio_alloc_blkg_stats(struct blkio_group *blkg); extern int blkiocg_del_blkio_group(struct blkio_group *blkg); extern struct blkio_group *blkiocg_lookup_group(struct blkio_cgroup *blkcg, - void *key); + struct request_queue *q, + enum blkio_policy_id plid); void blkiocg_update_timeslice_used(struct blkio_group *blkg, unsigned long time, unsigned long unaccounted_time); diff --git a/block/blk-throttle.c b/block/blk-throttle.c index c252df9169db..6613de78e364 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -252,7 +252,7 @@ static void throtl_init_add_tg_lists(struct throtl_data *td, __throtl_tg_fill_dev_details(td, tg); /* Add group onto cgroup list */ - blkiocg_add_blkio_group(blkcg, &tg->blkg, (void *)td, + blkiocg_add_blkio_group(blkcg, &tg->blkg, td->queue, tg->blkg.dev, BLKIO_POLICY_THROTL); tg->bps[READ] = blkcg_get_read_bps(blkcg, tg->blkg.dev); @@ -288,7 +288,6 @@ static struct throtl_grp *throtl_find_tg(struct throtl_data *td, struct blkio_cgroup *blkcg) { struct throtl_grp *tg = NULL; - void *key = td; /* * This is the common case when there are no blkio cgroups. @@ -297,7 +296,8 @@ throtl_grp *throtl_find_tg(struct throtl_data *td, struct blkio_cgroup *blkcg) if (blkcg == &blkio_root_cgroup) tg = td->root_tg; else - tg = tg_of_blkg(blkiocg_lookup_group(blkcg, key)); + tg = tg_of_blkg(blkiocg_lookup_group(blkcg, td->queue, + BLKIO_POLICY_THROTL)); __throtl_tg_fill_dev_details(td, tg); return tg; @@ -1012,22 +1012,22 @@ static bool throtl_release_tgs(struct throtl_data *td, bool release_root) * no new IO will come in this group. So get rid of this group as soon as * any pending IO in the group is finished. * - * This function is called under rcu_read_lock(). key is the rcu protected - * pointer. That means "key" is a valid throtl_data pointer as long as we are - * rcu read lock. + * This function is called under rcu_read_lock(). @q is the rcu protected + * pointer. That means @q is a valid request_queue pointer as long as we + * are rcu read lock. * - * "key" was fetched from blkio_group under blkio_cgroup->lock. That means + * @q was fetched from blkio_group under blkio_cgroup->lock. That means * it should not be NULL as even if queue was going away, cgroup deltion * path got to it first. */ -void throtl_unlink_blkio_group(void *key, struct blkio_group *blkg) +void throtl_unlink_blkio_group(struct request_queue *q, + struct blkio_group *blkg) { unsigned long flags; - struct throtl_data *td = key; - spin_lock_irqsave(td->queue->queue_lock, flags); - throtl_destroy_tg(td, tg_of_blkg(blkg)); - spin_unlock_irqrestore(td->queue->queue_lock, flags); + spin_lock_irqsave(q->queue_lock, flags); + throtl_destroy_tg(q->td, tg_of_blkg(blkg)); + spin_unlock_irqrestore(q->queue_lock, flags); } static bool throtl_clear_queue(struct request_queue *q) @@ -1054,52 +1054,48 @@ static void throtl_update_blkio_group_common(struct throtl_data *td, } /* - * For all update functions, key should be a valid pointer because these + * For all update functions, @q should be a valid pointer because these * update functions are called under blkcg_lock, that means, blkg is - * valid and in turn key is valid. queue exit path can not race because + * valid and in turn @q is valid. queue exit path can not race because * of blkcg_lock * * Can not take queue lock in update functions as queue lock under blkcg_lock * is not allowed. Under other paths we take blkcg_lock under queue_lock. */ -static void throtl_update_blkio_group_read_bps(void *key, +static void throtl_update_blkio_group_read_bps(struct request_queue *q, struct blkio_group *blkg, u64 read_bps) { - struct throtl_data *td = key; struct throtl_grp *tg = tg_of_blkg(blkg); tg->bps[READ] = read_bps; - throtl_update_blkio_group_common(td, tg); + throtl_update_blkio_group_common(q->td, tg); } -static void throtl_update_blkio_group_write_bps(void *key, +static void throtl_update_blkio_group_write_bps(struct request_queue *q, struct blkio_group *blkg, u64 write_bps) { - struct throtl_data *td = key; struct throtl_grp *tg = tg_of_blkg(blkg); tg->bps[WRITE] = write_bps; - throtl_update_blkio_group_common(td, tg); + throtl_update_blkio_group_common(q->td, tg); } -static void throtl_update_blkio_group_read_iops(void *key, +static void throtl_update_blkio_group_read_iops(struct request_queue *q, struct blkio_group *blkg, unsigned int read_iops) { - struct throtl_data *td = key; struct throtl_grp *tg = tg_of_blkg(blkg); tg->iops[READ] = read_iops; - throtl_update_blkio_group_common(td, tg); + throtl_update_blkio_group_common(q->td, tg); } -static void throtl_update_blkio_group_write_iops(void *key, +static void throtl_update_blkio_group_write_iops(struct request_queue *q, struct blkio_group *blkg, unsigned int write_iops) { - struct throtl_data *td = key; struct throtl_grp *tg = tg_of_blkg(blkg); tg->iops[WRITE] = write_iops; - throtl_update_blkio_group_common(td, tg); + throtl_update_blkio_group_common(q->td, tg); } static void throtl_shutdown_wq(struct request_queue *q) @@ -1306,7 +1302,7 @@ void blk_throtl_exit(struct request_queue *q) spin_unlock_irq(q->queue_lock); /* - * Wait for tg->blkg->key accessors to exit their grace periods. + * Wait for tg->blkg->q accessors to exit their grace periods. * Do this wait only if there are other undestroyed groups out * there (other than root group). This can happen if cgroup deletion * path claimed the responsibility of cleaning up a group before diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 0f7a81fc7c73..37e2da9cbb09 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -1020,7 +1020,8 @@ static inline struct cfq_group *cfqg_of_blkg(struct blkio_group *blkg) return NULL; } -static void cfq_update_blkio_group_weight(void *key, struct blkio_group *blkg, +static void cfq_update_blkio_group_weight(struct request_queue *q, + struct blkio_group *blkg, unsigned int weight) { struct cfq_group *cfqg = cfqg_of_blkg(blkg); @@ -1043,10 +1044,10 @@ static void cfq_init_add_cfqg_lists(struct cfq_data *cfqd, if (bdi->dev) { sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); cfq_blkiocg_add_blkio_group(blkcg, &cfqg->blkg, - (void *)cfqd, MKDEV(major, minor)); + cfqd->queue, MKDEV(major, minor)); } else cfq_blkiocg_add_blkio_group(blkcg, &cfqg->blkg, - (void *)cfqd, 0); + cfqd->queue, 0); cfqd->nr_blkcg_linked_grps++; cfqg->weight = blkcg_get_weight(blkcg, cfqg->blkg.dev); @@ -1097,7 +1098,6 @@ static struct cfq_group * cfq_find_cfqg(struct cfq_data *cfqd, struct blkio_cgroup *blkcg) { struct cfq_group *cfqg = NULL; - void *key = cfqd; struct backing_dev_info *bdi = &cfqd->queue->backing_dev_info; unsigned int major, minor; @@ -1108,7 +1108,8 @@ cfq_find_cfqg(struct cfq_data *cfqd, struct blkio_cgroup *blkcg) if (blkcg == &blkio_root_cgroup) cfqg = &cfqd->root_group; else - cfqg = cfqg_of_blkg(blkiocg_lookup_group(blkcg, key)); + cfqg = cfqg_of_blkg(blkiocg_lookup_group(blkcg, cfqd->queue, + BLKIO_POLICY_PROP)); if (cfqg && !cfqg->blkg.dev && bdi->dev && dev_name(bdi->dev)) { sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); @@ -1247,21 +1248,22 @@ static bool cfq_release_cfq_groups(struct cfq_data *cfqd) * any pending IO in the group is finished. * * This function is called under rcu_read_lock(). key is the rcu protected - * pointer. That means "key" is a valid cfq_data pointer as long as we are rcu - * read lock. + * pointer. That means @q is a valid request_queue pointer as long as we + * are rcu read lock. * - * "key" was fetched from blkio_group under blkio_cgroup->lock. That means + * @q was fetched from blkio_group under blkio_cgroup->lock. That means * it should not be NULL as even if elevator was exiting, cgroup deltion * path got to it first. */ -static void cfq_unlink_blkio_group(void *key, struct blkio_group *blkg) +static void cfq_unlink_blkio_group(struct request_queue *q, + struct blkio_group *blkg) { - unsigned long flags; - struct cfq_data *cfqd = key; + struct cfq_data *cfqd = q->elevator->elevator_data; + unsigned long flags; - spin_lock_irqsave(cfqd->queue->queue_lock, flags); + spin_lock_irqsave(q->queue_lock, flags); cfq_destroy_cfqg(cfqd, cfqg_of_blkg(blkg)); - spin_unlock_irqrestore(cfqd->queue->queue_lock, flags); + spin_unlock_irqrestore(q->queue_lock, flags