blkcg: kill policy node and blkg->dev, take#3
Subsystems (2)
Tejun Heo
maintainerPATCH 1/1 blkcg: kill policy node and blkg->dev, take#3
Hello, guys.
This is the third take of blkcg-kill-policy-node-and-blkg_dev patchset. Changes from the last take[L] are,
* 0001-blkcg-make-CONFIG_BLK_CGROUP-bool.patch fixed as suggested by Vivek.
* 0007-blkcg-make-blkio_list_lock-irq-safe.patch dropped and lock order inversion involving blkio_list_lock fixed.
* 0007-blkcg-shoot-down-blkio_groups-on-elevator-switch.patch updated to avoid shooting down blk-throtl's root_group and retry when lost race against blkcg removal.
blk-cgroup, blk-throttle and cfq-iosched are pretty tightly tangled and untangling requires seemingly unrelated changes and addition of some transitional stuff. Some are in this patchset but they'll be more prominent in the second patchset. This patchset concentrates on removing blkio_policy_node and blkio_group->dev, and preparing for the next patchset which will unify blkg so that there's single blkg per cgroup - request_queue pair.
blkio_policy_node carries configuration per cgroup-request_queue association separately from the matching blkio_group. The goal seems to be allowing configuration of missing devices and retaining configuration across hot unplug/plug cycles. Such behavior is very different from existing conventions, misleading and unnecessary. This patchset moves configuration to blkio_group and removes blkio_policy_node.
blkg->dev removal is much simpler. blkg->dev is used to print out device number when printing out per-device configurations or statistics. The device name can be easily determined by following the associated request_queue from blkg instead.
This patchset makes the following two userland visible behavior changes.
* Per-device blkcg configuration can't be done for non-existing devices. This behavior change is intentional and permanent.
* Switching elevator clears per-device blk-throtl configuration. This behavior change is transitional and will be restored with later patches.
This patchset contains the following 16 patches.
0001-blkcg-make-CONFIG_BLK_CGROUP-bool.patch 0002-cfq-don-t-register-propio-policy-if-CONFIG_CFQ_GROUP.patch 0003-elevator-clear-auxiliary-data-earlier-during-elevato.patch 0004-elevator-make-elevator_init_fn-return-0-errno.patch 0005-block-implement-blk_queue_bypass_start-end.patch 0006-block-extend-queue-bypassing-to-cover-blkcg-policies.patch 0007-blkcg-shoot-down-blkio_groups-on-elevator-switch.patch 0008-blkcg-move-rcu_read_lock-outside-of-blkio_group-get-.patch 0009-blkcg-update-blkg-get-functions-take-blkio_cgroup-as.patch 0010-blkcg-use-q-and-plid-instead-of-opaque-void-for-blki.patch 0011-blkcg-add-blkio_policy-array-and-allow-one-policy-pe.patch 0012-blkcg-use-the-usual-get-blkg-path-for-root-blkio_gro.patch 0013-blkcg-factor-out-blkio_group-creation.patch 0014-blkcg-don-t-allow-or-retain-configuration-of-missing.patch 0015-blkcg-kill-blkio_policy_node.patch 0016-blkcg-kill-the-mind-bending-blkg-dev.patch
0001-0004: misc preps including elevator switch update
0005-0007: ensure blkcg policies are quiescent across elvswitch
0008-0013: move common part of blkg management into blkcg core
0014-0015: kill blkio_policy_node
0016 : kill blkg->dev
This patchset is on top of v3.3-rc1 and also available in the following git branch.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git blkcg-kill-pn
block/Kconfig.iosched | 4 block/blk-cgroup.c | 686 ++++++++++++++++------------------------------- block/blk-cgroup.h | 106 ++----- block/blk-core.c | 53 +++ block/blk-throttle.c | 286 ++++++------------- block/blk.h | 6 block/cfq-iosched.c | 296 ++++++++------------ block/cfq.h | 7 block/deadline-iosched.c | 8 block/elevator.c | 116 +++---- block/noop-iosched.c | 8 include/linux/blkdev.h | 5 include/linux/elevator.h | 2 init/Kconfig | 2 14 files changed, 606 insertions(+), 979 deletions(-)
Thanks.
--
tejun
Tejun Heo
maintainerPATCH 1/16 blkcg: make CONFIG_BLK_CGROUP bool
From: Tejun Heo <[email protected]>
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.
--- block/Kconfig.iosched | 4 ---- block/blk-cgroup.c | 17 ----------------- block/blk-cgroup.h | 10 ++-------- init/Kconfig | 2 +- 4 files changed, 3 insertions(+), 30 deletions(-)
block/Kconfig.iosched +0 -4
@@ -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
block/blk-cgroup.c +0 -17
@@ -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" );
block/blk-cgroup.h +2 -8
@@ -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);
init/Kconfig +1 -1
@@ -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---
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 2/16 cfq: don't register propio policy if !CONFIG_CFQ_GROUP_IOSCHED
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
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.
--- block/cfq-iosched.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
block/cfq-iosched.c +4 -3
@@ -3884,8 +3884,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 )
@@ -3916,14 +3914,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); }
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 3/16 elevator: clear auxiliary data earlier during elevator switch
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
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.
--- block/elevator.c | 88 +++++++++++++++++++++++++++--------------------------- 1 files changed, 44 insertions(+), 44 deletions(-)
block/elevator.c +44 -44
@@ -168,11 +168,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 ; }
@@ -235,7 +234,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))
@@ -269,17 +267,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);
@@ -848,8 +845,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" );
@@ -867,11 +865,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)
@@ -954,39 +947,47 @@ 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);
@@ -994,11 +995,10 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e)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);
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 4/16 elevator: make elevator_init_fn() return 0/-errno
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
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.
--- 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(-)
block/cfq-iosched.c +5 -4
@@ -3662,7 +3662,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;
@@ -3671,7 +3671,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 ;
@@ -3698,7 +3698,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 ();
@@ -3729,6 +3729,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;
@@ -3753,7 +3754,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 ; }/*
block/deadline-iosched.c +5 -3
@@ -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 ; }/*
block/elevator.c +2 -10
@@ -168,14 +168,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)
@@ -271,7 +263,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;
@@ -974,7 +966,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;
block/noop-iosched.c +5 -3
@@ -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)
include/linux/elevator.h +1 -1
@@ -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
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 5/16 block: implement blk_queue_bypass_start/end()
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
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.
--- 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(-)
block/blk-core.c +37 -2
@@ -403,6 +403,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 *
@@ -859,8 +895,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)
block/blk.h +2 -4
@@ -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,
@@ -142,9 +143,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
block/elevator.c +3 -22
@@ -600,25 +600,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);
@@ -950,7 +931,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)
@@ -980,7 +961,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" , e->type->elevator_name);
@@ -992,7 +973,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; }
include/linux/blkdev.h +4 -1
@@ -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;
@@ -409,7 +411,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 */
@@ -497,6 +499,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)
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 6/16 block: extend queue bypassing to cover blkcg policies
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
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.
--- block/blk-core.c | 12 ++++++++---- block/blk-throttle.c | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-)
block/blk-core.c +8 -4
@@ -366,8 +366,7 @@ void blk_drain_queue(struct request_queue *q, bool drain_all)spin_lock_irq (q->queue_lock);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
@@ -408,8 +407,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) {
@@ -454,6 +453,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);
block/blk-throttle.c +2 -2
@@ -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 ; }
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 7/16 blkcg: shoot down blkio_groups on elevator switch
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
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.
--- block/blk-cgroup.c | 34 +++++++++++++++++++++++++++++++++- block/blk-cgroup.h | 6 +++++- block/blk-throttle.c | 27 +++++++++++++++++++++++++-- block/cfq-iosched.c | 21 ++++++++++++++++++++- block/elevator.c | 3 +++ 5 files changed, 86 insertions(+), 5 deletions(-)
block/blk-cgroup.c +33 -1
@@ -17,8 +17,9 @@#include <linux/err.h> #include <linux/blkdev.h> #include <linux/slab.h> -#include "blk-cgroup.h" #include <linux/genhd.h> +#include <linux/delay.h> +#include "blk-cgroup.h" #define MAX_KEY_LEN 100
@@ -1619,6 +1620,37 @@ done:return &blkcg->css; } +void blkcg_clear_queue (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 */ + } +} +/* * We cannot support shared io contexts, as we have no mean to support * two tasks with the same ioc in two different groups without major rework
block/blk-cgroup.h +5 -1
@@ -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;
@@ -230,6 +231,8 @@ struct blkio_policy_type {enum blkio_policy_id plid; }; +extern void blkcg_clear_queue (struct request_queue *q); +/* Blkio controller policy registration */ extern void blkio_policy_register (struct blkio_policy_type *);extern void blkio_policy_unregister (struct blkio_policy_type *);
@@ -247,6 +250,7 @@ struct blkio_group {struct blkio_policy_type { }; +static inline void blkcg_clear_queue (struct request_queue *q) { }static inline void blkio_policy_register (struct blkio_policy_type *blkiop) { }static inline void blkio_policy_unregister (struct blkio_policy_type *blkiop) { }
block/blk-throttle.c +25 -2
@@ -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 )
block/cfq-iosched.c +20 -1
@@ -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,20 @@ 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) +{ +struct cfq_data *cfqd = q->elevator->elevator_data; + +lockdep_assert_held (q->queue_lock); + +/* shoot down blkgs iff the current elevator is cfq */ +if (q->elevator->type == &iosched_cfq) +return cfq_release_cfq_groups (cfqd); +return true; +} +#else /* GROUP_IOSCHED */ static struct cfq_group *cfq_get_cfqg (struct cfq_data *cfqd) {
@@ -3881,6 +3899,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 ,
block/elevator.c +3 -0
@@ -38,6 +38,7 @@#include <trace/events/block.h> #include "blk.h" +#include "blk-cgroup.h" static DEFINE_SPINLOCK (elv_list_lock);static LIST_HEAD (elv_list);
@@ -941,6 +942,8 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e)ioc_clear_queue (q);spin_unlock_irq (q->queue_lock); +blkcg_clear_queue (q); +/* allocate, init and register new elevator */ err = -ENOMEM ; q->elevator =elevator_alloc (q, new_e);
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 8/16 blkcg: move rcu_read_lock() outside of blkio_group get functions
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
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.
--- block/blk-throttle.c | 18 ++++++------------ block/cfq-iosched.c | 11 +++++------ 2 files changed, 11 insertions(+), 18 deletions(-)
block/blk-throttle.c +6 -12
@@ -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; }
block/cfq-iosched.c +5 -6
@@ -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; }
@@ -2878,6 +2873,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 */
@@ -2893,6 +2890,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,
@@ -2918,6 +2916,7 @@ retry:if (new_cfqq)kmem_cache_free (cfq_pool, new_cfqq); +rcu_read_unlock ();return cfqq; }
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 9/16 blkcg: update blkg get functions take blkio_cgroup as parameter
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
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.
--- block/blk-throttle.c | 16 +++++++--------- block/cfq-iosched.c | 20 ++++++++++++-------- 2 files changed, 19 insertions(+), 17 deletions(-)
block/blk-throttle.c +7 -9
@@ -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))) {
@@ -339,11 +342,6 @@ static struct throtl_grp * throtl_get_tg(struct throtl_data *td) }/* -* 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;
block/cfq-iosched.c +12 -8
@@ -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
@@ -1279,7 +1279,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; }
@@ -2868,6 +2869,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;
@@ -2875,7 +2877,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);
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 10/16 blkcg: use q and plid instead of opaque void * for blkio_group association
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
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.
--- 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(-)
block/blk-cgroup.c +15 -17
@@ -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);
@@ -1551,7 +1549,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;
@@ -1566,7 +1564,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);
@@ -1580,7 +1578,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 );
block/blk-cgroup.h +12 -10
@@ -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 {
@@ -306,12 +307,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);
block/blk-throttle.c +23 -27
125 lines not on this page · raw message
block/cfq-iosched.c +16 -14
@@ -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); }static struct elevator_type iosched_cfq;
@@ -3725,7 +3727,7 @@ static int cfq_init_queue(struct request_queue *q)rcu_read_lock ();cfq_blkiocg_add_blkio_group (&blkio_root_cgroup, &cfqg->blkg, - (void *)cfqd,0 ); + cfqd->queue,0 );rcu_read_unlock (); cfqd->nr_blkcg_linked_grps++;
block/cfq.h +4 -3
@@ -68,8 +68,9 @@ static inline void cfq_blkiocg_update_completion_stats(struct blkio_group *blkg, }static inline void cfq_blkiocg_add_blkio_group (struct blkio_cgroup *blkcg, -struct blkio_group *blkg,void *key,dev_t dev) { -blkiocg_add_blkio_group (blkcg, blkg, key, dev,BLKIO_POLICY_PROP ); +struct blkio_group *blkg,struct request_queue *q,dev_t dev) +{ +blkiocg_add_blkio_group (blkcg, blkg, q, dev,BLKIO_POLICY_PROP ); }static inline int cfq_blkiocg_del_blkio_group (struct blkio_group *blkg)
@@ -105,7 +106,7 @@ static inline void cfq_blkiocg_update_dispatch_stats(struct blkio_group *blkg,static inline void cfq_blkiocg_update_completion_stats (struct blkio_group *blkg,uint64_t start_time,uint64_t io_start_time,bool direction,bool sync) {}static inline void cfq_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) {}static inline int cfq_blkiocg_del_blkio_group (struct blkio_group *blkg) {return 0 ;
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 11/16 blkcg: add blkio_policy[] array and allow one policy per policy ID
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
Block cgroup policies are maintained in a linked list and, theoretically, multiple policies sharing the same policy ID are allowed.
This patch temporarily restricts one policy per plid and adds blkio_policy[] array which indexes registered policy types by plid. Both the restriction and blkio_policy[] array are transitional and will be removed once API cleanup is complete.
--- block/blk-cgroup.c | 10 ++++++++++ block/blk-cgroup.h | 2 ++ 2 files changed, 12 insertions(+), 0 deletions(-)
block/blk-cgroup.c +10 -0
@@ -29,6 +29,8 @@ static LIST_HEAD(blkio_list);struct blkio_cgroup blkio_root_cgroup = { .weight =2 *BLKIO_WEIGHT_DEFAULT };EXPORT_SYMBOL_GPL (blkio_root_cgroup); +static struct blkio_policy_type *blkio_policy[BLKIO_NR_POLICIES ]; +static struct cgroup_subsys_state *blkiocg_create (struct cgroup_subsys *,struct cgroup *); static int blkiocg_can_attach(struct cgroup_subsys *, struct cgroup *,
@@ -1694,7 +1696,11 @@ static void blkiocg_attach(struct cgroup_subsys *ss, struct cgroup *cgrp,void blkio_policy_register (struct blkio_policy_type *blkiop) {spin_lock (&blkio_list_lock); + +BUG_ON (blkio_policy[blkiop->plid]); + blkio_policy[blkiop->plid] = blkiop;list_add_tail (&blkiop->list, &blkio_list); +spin_unlock (&blkio_list_lock); }EXPORT_SYMBOL_GPL (blkio_policy_register);
@@ -1702,7 +1708,11 @@ EXPORT_SYMBOL_GPL(blkio_policy_register);void blkio_policy_unregister (struct blkio_policy_type *blkiop) {spin_lock (&blkio_list_lock); + +BUG_ON (blkio_policy[blkiop->plid] != blkiop); + blkio_policy[blkiop->plid] =NULL ;list_del_init (&blkiop->list); +spin_unlock (&blkio_list_lock); }EXPORT_SYMBOL_GPL (blkio_policy_unregister);
block/blk-cgroup.h +2 -0
@@ -19,6 +19,8 @@enum blkio_policy_id {BLKIO_POLICY_PROP =0 ,/* Proportional Bandwidth division */ BLKIO_POLICY_THROTL ,/* Throttling */ + +BLKIO_NR_POLICIES , };/* Max limits for throttle policy */
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 12/16 blkcg: use the usual get blkg path for root blkio_group
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
For root blkg, blk_throtl_init() was using throtl_alloc_tg() explicitly and cfq_init_queue() was manually initializing embedded cfqd->root_group, adding unnecessarily different code paths to blkg handling.
Make both use the usual blkio_group get functions - throtl_get_tg() and cfq_get_cfqg() - for the root blkio_group too. Note that blk_throtl_init() callsite is pushed downwards in blk_alloc_queue_node() so that @q is sufficiently initialized for throtl_get_tg().
This simplifies root blkg handling noticeably for cfq and will allow further modularization of blkcg API.
-v2: Vivek pointed out that using cfq_get_cfqg() won't work if
CONFIG_CFQ_GROUP_IOSCHED is disabled. Fix it by factoring out initialization of base part of cfqg into cfq_init_cfqg_base() and alloc/init/free explicitly if !CONFIG_CFQ_GROUP_IOSCHED.
--- block/blk-core.c | 6 +- block/blk-throttle.c | 18 ++++---- block/cfq-iosched.c | 105 +++++++++++++++++++++++++------------------------- 3 files changed, 65 insertions(+), 64 deletions(-)
block/blk-core.c +3 -3
@@ -538,9 +538,6 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)if (err) goto fail_id; -if (blk_throtl_init (q)) - goto fail_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);
@@ -562,6 +559,9 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)*/ q->queue_lock = &q->__queue_lock; +if (blk_throtl_init (q)) + goto fail_id; +return q; fail_id:
block/blk-throttle.c +9 -9
@@ -1252,7 +1252,6 @@ void blk_throtl_drain(struct request_queue *q)int blk_throtl_init (struct request_queue *q) {struct throtl_data *td; -struct throtl_grp *tg; td =kzalloc_node (sizeof (*td),GFP_KERNEL , q->node);if (!td)
@@ -1265,19 +1264,20 @@ int blk_throtl_init(struct request_queue *q)/* alloc and Init root group. */ td->queue = q; - tg =throtl_alloc_tg (td); -if (!tg) { -kfree (td); -return -ENOMEM ; - } +rcu_read_lock (); +spin_lock_irq (q->queue_lock); - td->root_tg = tg; + td->root_tg =throtl_get_tg (td, &blkio_root_cgroup); -rcu_read_lock (); -throtl_init_add_tg_lists (td, tg, &blkio_root_cgroup); +spin_unlock_irq (q->queue_lock);rcu_read_unlock (); +if (!td->root_tg) { +kfree (td); +return -ENOMEM ; + } +/* Attach throtl data to request queue */ q->td = td;return 0 ;
block/cfq-iosched.c +53 -52
188 lines not on this page · raw message
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 13/16 blkcg: factor out blkio_group creation
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
Currently both blk-throttle and cfq-iosched implement their own blkio_group creation code in throtl_get_tg() and cfq_get_cfqg(). This patch factors out the common code.
* New plkio_policy_ops methods blkio_alloc_group_fn() and blkio_link_group_fn added. Both are transitional and will be removed once the blkg management code is fully moved into blk-cgroup.c.
* blkio_alloc_group_fn() allocates policy-specific blkg which is usually a larger data structure with blkg as the first entry and intiailizes it. Note that initialization of blkg proper, including percpu stats, is responsibility of blk-cgroup proper.
Note that default config (weight, bps...) initialization is done from this method; otherwise, we end up violating locking order between blkcg and q locks via blkcg_get_CONF() functions.
* blkio_link_group_fn() is called under queue_lock and responsible for linking the blkg to the queue. blkcg side is handled by blk-cgroup proper.
* The common blkg creation function is named blkg_lookup_create() and blkiocg_lookup_group() is renamed to blkg_lookup() for consistency. Also, throtl / cfq related functions are similarly [re]named for consistency.
* Code to fill cfqg->blkg.dev is open coded in cfq_find_alloc_queue(). This is transitional and will be removed soon.
This simplifies blkcg policy implementations and enables further cleanup.
--- block/blk-cgroup.c | 113 ++++++++++++++++++++++++++++----------- block/blk-cgroup.h | 29 +++++----- block/blk-throttle.c | 144 ++++++++++++++------------------------------------ block/cfq-iosched.c | 134 ++++++++++++---------------------------------- block/cfq.h | 8 --- 5 files changed, 172 insertions(+), 256 deletions(-)
block/blk-cgroup.c +82 -31
@@ -465,38 +465,89 @@ void blkiocg_update_io_merged_stats(struct blkio_group *blkg, bool direction, }EXPORT_SYMBOL_GPL (blkiocg_update_io_merged_stats); -/* -* This function allocates the per cpu stats for blkio_group. Should be called -* from sleepable context as alloc_per_cpu() requires that. -*/ -int blkio_alloc_blkg_stats (struct blkio_group *blkg) +struct blkio_group *blkg_lookup_create (struct blkio_cgroup *blkcg, +struct request_queue *q, +enum blkio_policy_id plid) + __releases(q->queue_lock) __acquires(q->queue_lock) { -/* Allocate memory for per cpu stats */ - blkg->stats_cpu = alloc_percpu(struct blkio_group_stats_cpu); -if (!blkg->stats_cpu) -return -ENOMEM ; -return 0 ; -} -EXPORT_SYMBOL_GPL (blkio_alloc_blkg_stats); +struct blkio_policy_type *pol = blkio_policy[plid]; +struct blkio_group *blkg =NULL ; +struct blkio_group *new_blkg =NULL ; -void blkiocg_add_blkio_group (struct blkio_cgroup *blkcg, -struct blkio_group *blkg,struct request_queue *q,dev_t dev, -enum blkio_policy_id plid) -{ -unsigned long flags; +WARN_ON_ONCE (!rcu_read_lock_held ()); +lockdep_assert_held (q->queue_lock); -spin_lock_irqsave (&blkcg->lock, flags); -spin_lock_init (&blkg->stats_lock); -rcu_assign_pointer (blkg->q, q); - blkg->blkcg_id =css_id (&blkcg->css); +/* +* This could be the first entry point of blkcg implementation and +* we shouldn't allow anything to go through for a dead queue. The +* following can be removed if blkg lookup is guaranteed to fail on +* a dead queue. +*/ +if (unlikely (blk_queue_dead (q))) +return NULL ; + + blkg =blkg_lookup (blkcg, q, plid); +if (blkg) +return blkg; + +if (!css_tryget (&blkcg->css)) +return NULL ; + +/* +* Allocate and initialize. +* +* FIXME: The following is broken. Percpu memory allocation +* requires %GFP_KERNEL context and can't be performed from IO +* path. Allocation here should inherently be atomic and the +* following lock dancing can be removed once the broken percpu +* allocation is fixed. +*/ +spin_unlock_irq (q->queue_lock); +rcu_read_unlock (); + + new_blkg = pol->ops.blkio_alloc_group_fn (q, blkcg); +if (new_blkg) { + new_blkg->stats_cpu = alloc_percpu(struct blkio_group_stats_cpu); + +spin_lock_init (&new_blkg->stats_lock); +rcu_assign_pointer (new_blkg->q, q); + new_blkg->blkcg_id =css_id (&blkcg->css); + new_blkg->plid = plid; +cgroup_path (blkcg->css.cgroup, new_blkg->path, +sizeof (new_blkg->path)); + } + +rcu_read_lock (); +spin_lock_irq (q->queue_lock); +css_put (&blkcg->css); + +/* did the device die inbetween? */ +if (unlikely (blk_queue_dead (q))) + goto out; + +/* did someone beat us to it? */ + blkg =blkg_lookup (blkcg, q, plid); +if (unlikely (blkg)) + goto out; + +/* did alloc fail? */ +if (unlikely (!new_blkg || !new_blkg->stats_cpu)) + goto out; + +/* insert */ +spin_lock (&blkcg->lock); +swap (blkg, new_blkg);hlist_add_head_rcu (&blkg->blkcg_node, &blkcg->blkg_list); - blkg->plid = plid; -spin_unlock_irqrestore (&blkcg->lock, flags); -/* Need to take css reference ? */ -cgroup_path (blkcg->css.cgroup, blkg->path,sizeof (blkg->path)); - blkg->dev = dev; + pol->ops.blkio_link_group_fn (q, blkg); +spin_unlock (&blkcg->lock); +out: +if (new_blkg) { +free_percpu (new_blkg->stats_cpu); +kfree (new_blkg); + } +return blkg; } -EXPORT_SYMBOL_GPL (blkiocg_add_blkio_group); +EXPORT_SYMBOL_GPL (blkg_lookup_create);static void __blkiocg_del_blkio_group (struct blkio_group *blkg) {
@@ -533,9 +584,9 @@ 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, -struct request_queue *q, -enum blkio_policy_id plid) +struct blkio_group *blkg_lookup (struct blkio_cgroup *blkcg, +struct request_queue *q, +enum blkio_policy_id plid) {struct blkio_group *blkg;struct hlist_node *n;
@@ -545,7 +596,7 @@ struct blkio_group *blkiocg_lookup_group(struct blkio_cgroup *blkcg,return blkg;return NULL ; } -EXPORT_SYMBOL_GPL (blkiocg_lookup_group); +EXPORT_SYMBOL_GPL (blkg_lookup);static void blkio_reset_stats_cpu (struct blkio_group *blkg) {
block/blk-cgroup.h +14 -15
@@ -204,6 +204,10 @@ 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 struct blkio_group *(blkio_alloc_group_fn )(struct request_queue *q, +struct blkio_cgroup *blkcg); +typedef void (blkio_link_group_fn )(struct request_queue *q, +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);
@@ -219,6 +223,8 @@ typedef void (blkio_update_group_write_iops_fn)(struct request_queue *q, struct blkio_group *blkg, unsigned int write_iops);struct blkio_policy_ops { +blkio_alloc_group_fn *blkio_alloc_group_fn; +blkio_link_group_fn *blkio_link_group_fn;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;
@@ -308,14 +314,13 @@ static inline void blkiocg_set_start_empty_time(struct blkio_group *blkg) {}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,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, -struct request_queue *q, -enum blkio_policy_id plid); +extern struct blkio_group *blkg_lookup (struct blkio_cgroup *blkcg, +struct request_queue *q, +enum blkio_policy_id plid); +struct blkio_group *blkg_lookup_create (struct blkio_cgroup *blkcg, +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);
@@ -336,17 +341,11 @@ cgroup_to_blkio_cgroup(struct cgroup *cgroup) { return NULL; }static inline struct blkio_cgroup *task_blkio_cgroup (struct task_struct *tsk) {return NULL ; } -static inline void blkiocg_add_blkio_group (struct blkio_cgroup *blkcg, -struct blkio_group *blkg,void *key,dev_t dev, -enum blkio_policy_id plid) {} - -static inline int blkio_alloc_blkg_stats (struct blkio_group *blkg) {return 0 ; } -static inline int blkiocg_del_blkio_group (struct blkio_group *blkg) {return 0 ; } -static inline struct blkio_group * -blkiocg_lookup_group (struct blkio_cgroup *blkcg,void *key) {return NULL ; } +static inline struct blkio_group *blkg_lookup (struct blkio_cgroup *blkcg, +void *key) {return NULL ; }static inline void blkiocg_update_timeslice_used (struct blkio_group *blkg,unsigned long time,unsigned long unaccounted_time)
block/blk-throttle.c +40 -104
228 lines not on this page · raw message
block/cfq-iosched.c +36 -98
205 lines not on this page · raw message
block/cfq.h +0 -8
@@ -67,12 +67,6 @@ static inline void cfq_blkiocg_update_completion_stats(struct blkio_group *blkg, direction, sync); } -static inline void cfq_blkiocg_add_blkio_group (struct blkio_cgroup *blkcg, -struct blkio_group *blkg,struct request_queue *q,dev_t dev) -{ -blkiocg_add_blkio_group (blkcg, blkg, q, dev,BLKIO_POLICY_PROP ); -} -static inline int cfq_blkiocg_del_blkio_group (struct blkio_group *blkg) {return blkiocg_del_blkio_group (blkg);
@@ -105,8 +99,6 @@ static inline void cfq_blkiocg_update_dispatch_stats(struct blkio_group *blkg, uint64_t bytes, bool direction, bool sync) {} static inline void cfq_blkiocg_update_completion_stats(struct blkio_group *blkg, uint64_t start_time, uint64_t io_start_time, bool direction, bool sync) {} -static inline void cfq_blkiocg_add_blkio_group(struct blkio_cgroup *blkcg, - struct blkio_group *blkg, struct request_queue *q, dev_t dev) {} static inline int cfq_blkiocg_del_blkio_group(struct blkio_group *blkg) {return 0 ;
Signature · 1 line
1.7.7.3
PATCH 13/16 blkcg: factor out blkio_group creation
Currently both blk-throttle and cfq-iosched implement their own blkio_group creation code in throtl_get_tg() and cfq_get_cfqg(). This patch factors out the common code.
* New plkio_policy_ops methods blkio_alloc_group_fn() and blkio_link_group_fn added. Both are transitional and will be removed once the blkg management code is fully moved into blk-cgroup.c.
* blkio_alloc_group_fn() allocates policy-specific blkg which is usually a larger data structure with blkg as the first entry and intiailizes it. Note that initialization of blkg proper, including percpu stats, is responsibility of blk-cgroup proper.
Note that default config (weight, bps...) initialization is done from this method; otherwise, we end up violating locking order between blkcg and q locks via blkcg_get_CONF() functions.
* blkio_link_group_fn() is called under queue_lock and responsible for linking the blkg to the queue. blkcg side is handled by blk-cgroup proper.
* The common blkg creation function is named blkg_lookup_create() and blkiocg_lookup_group() is renamed to blkg_lookup() for consistency. Also, throtl / cfq related functions are similarly [re]named for consistency.
* Code to fill cfqg->blkg.dev is open coded in cfq_find_alloc_queue(). This is transitional and will be removed soon.
This simplifies blkcg policy implementations and enables further cleanup.
-v2: Vivek noticed that blkg_lookup_create() incorrectly tested
blk_queue_dead() instead of blk_queue_bypass() leading a user of the function ending up creating a new blkg on bypassing queue. This is a bug introduced while relocating bypass patches before this one. Fixed.
--- block/blk-cgroup.c | 115 +++++++++++++++++++++++++++++----------- block/blk-cgroup.h | 29 ++++------ block/blk-throttle.c | 144 ++++++++++++++------------------------------------- block/cfq-iosched.c | 134 ++++++++++++----------------------------------- block/cfq.h | 8 -- 5 files changed, 173 insertions(+), 257 deletions(-)
block/blk-cgroup.h +14 -15
@@ -204,6 +204,10 @@ extern unsigned int blkcg_get_read_iops(extern unsigned int blkcg_get_write_iops (struct blkio_cgroup *blkcg,dev_t dev); +typedef struct blkio_group *(blkio_alloc_group_fn )(struct request_queue *q, +struct blkio_cgroup *blkcg); +typedef void (blkio_link_group_fn )(struct request_queue *q, +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);
@@ -219,6 +223,8 @@ typedef void (blkio_update_group_write_i struct blkio_group *blkg, unsigned int write_iops);struct blkio_policy_ops { +blkio_alloc_group_fn *blkio_alloc_group_fn; +blkio_link_group_fn *blkio_link_group_fn;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;
@@ -308,14 +314,13 @@ static inline void blkiocg_set_start_empextern 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,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, -struct request_queue *q, -enum blkio_policy_id plid); +extern struct blkio_group *blkg_lookup (struct blkio_cgroup *blkcg, +struct request_queue *q, +enum blkio_policy_id plid); +struct blkio_group *blkg_lookup_create (struct blkio_cgroup *blkcg, +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);
@@ -336,17 +341,11 @@ cgroup_to_blkio_cgroup(struct cgroup *cgstatic inline struct blkio_cgroup *task_blkio_cgroup (struct task_struct *tsk) {return NULL ; } -static inline void blkiocg_add_blkio_group (struct blkio_cgroup *blkcg, -struct blkio_group *blkg,void *key,dev_t dev, -enum blkio_policy_id plid) {} - -static inline int blkio_alloc_blkg_stats (struct blkio_group *blkg) {return 0 ; } -static inline int blkiocg_del_blkio_group (struct blkio_group *blkg) {return 0 ; } -static inline struct blkio_group * -blkiocg_lookup_group (struct blkio_cgroup *blkcg,void *key) {return NULL ; } +static inline struct blkio_group *blkg_lookup (struct blkio_cgroup *blkcg, +void *key) {return NULL ; }static inline void blkiocg_update_timeslice_used (struct blkio_group *blkg,unsigned long time,unsigned long unaccounted_time)
block/blk-throttle.c +40 -104
228 lines not on this page · raw message
block/blk-cgroup.c +83 -32
137 lines not on this page · raw message
block/cfq-iosched.c +36 -98
205 lines not on this page · raw message
block/cfq.h +0 -8
@@ -67,12 +67,6 @@ static inline void cfq_blkiocg_update_co direction, sync); } -static inline void cfq_blkiocg_add_blkio_group (struct blkio_cgroup *blkcg, -struct blkio_group *blkg,struct request_queue *q,dev_t dev) -{ -blkiocg_add_blkio_group (blkcg, blkg, q, dev,BLKIO_POLICY_PROP ); -} -static inline int cfq_blkiocg_del_blkio_group (struct blkio_group *blkg) {return blkiocg_del_blkio_group (blkg);
@@ -105,8 +99,6 @@ static inline void cfq_blkiocg_update_di uint64_t bytes, bool direction, bool sync) {} static inline void cfq_blkiocg_update_completion_stats(struct blkio_group *blkg, uint64_t start_time, uint64_t io_start_time, bool direction, bool sync) {} -static inline void cfq_blkiocg_add_blkio_group(struct blkio_cgroup *blkcg, - struct blkio_group *blkg, struct request_queue *q, dev_t dev) {} static inline int cfq_blkiocg_del_blkio_group(struct blkio_group *blkg) {return 0 ;
Tejun Heo
maintainerPATCH 13/16 blkcg: factor out blkio_group creation
In reply to Tejun Heoblkio_group creation code in throtl_get_tg() and cfq_get_cfqg(). This patch factors out the common code.
* New plkio_policy_ops methods blkio_alloc_group_fn() and blkio_link_group_fn added. Both are transitional and will be removed once the blkg management code is fully moved into blk-cgroup.c.
* blkio_alloc_group_fn() allocates policy-specific blkg which is usually a larger data structure with blkg as the first entry and intiailizes it. Note that initialization of blkg proper, including percpu stats, is responsibility of blk-cgroup proper.
Note that default config (weight, bps...) initialization is done from this method; otherwise, we end up violating locking order between blkcg and q locks via blkcg_get_CONF() functions.
* blkio_link_group_fn() is called under queue_lock and responsible for linking the blkg to the queue. blkcg side is handled by blk-cgroup proper.
* The common blkg creation function is named blkg_lookup_create() and blkiocg_lookup_group() is renamed to blkg_lookup() for consistency. Also, throtl / cfq related functions are similarly [re]named for consistency.
* Code to fill cfqg->blkg.dev is open coded in cfq_find_alloc_queue(). This is transitional and will be removed soon.
This simplifies blkcg policy implementations and enables further cleanup.
-v2: Vivek noticed that blkg_lookup_create() incorrectly tested
blk_queue_dead() instead of blk_queue_bypass() leading a user of the function ending up creating a new blkg on bypassing queue. This is a bug introduced while relocating bypass patches before this one. Fixed.
--- block/blk-cgroup.c | 115 +++++++++++++++++++++++++++++----------- block/blk-cgroup.h | 29 ++++------ block/blk-throttle.c | 144 ++++++++++++++------------------------------------- block/cfq-iosched.c | 134 ++++++++++++----------------------------------- block/cfq.h | 8 -- 5 files changed, 173 insertions(+), 257 deletions(-)
block/blk-cgroup.h +14 -15
@@ -204,6 +204,10 @@ extern unsigned int blkcg_get_read_iops(extern unsigned int blkcg_get_write_iops (struct blkio_cgroup *blkcg,dev_t dev); +typedef struct blkio_group *(blkio_alloc_group_fn )(struct request_queue *q, +struct blkio_cgroup *blkcg); +typedef void (blkio_link_group_fn )(struct request_queue *q, +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);
@@ -219,6 +223,8 @@ typedef void (blkio_update_group_write_i struct blkio_group *blkg, unsigned int write_iops);struct blkio_policy_ops { +blkio_alloc_group_fn *blkio_alloc_group_fn; +blkio_link_group_fn *blkio_link_group_fn;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;
@@ -308,14 +314,13 @@ static inline void blkiocg_set_start_empextern 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,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, -struct request_queue *q, -enum blkio_policy_id plid); +extern struct blkio_group *blkg_lookup (struct blkio_cgroup *blkcg, +struct request_queue *q, +enum blkio_policy_id plid); +struct blkio_group *blkg_lookup_create (struct blkio_cgroup *blkcg, +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);
@@ -336,17 +341,11 @@ cgroup_to_blkio_cgroup(struct cgroup *cgstatic inline struct blkio_cgroup *task_blkio_cgroup (struct task_struct *tsk) {return NULL ; } -static inline void blkiocg_add_blkio_group (struct blkio_cgroup *blkcg, -struct blkio_group *blkg,void *key,dev_t dev, -enum blkio_policy_id plid) {} - -static inline int blkio_alloc_blkg_stats (struct blkio_group *blkg) {return 0 ; } -static inline int blkiocg_del_blkio_group (struct blkio_group *blkg) {return 0 ; } -static inline struct blkio_group * -blkiocg_lookup_group (struct blkio_cgroup *blkcg,void *key) {return NULL ; } +static inline struct blkio_group *blkg_lookup (struct blkio_cgroup *blkcg, +void *key) {return NULL ; }static inline void blkiocg_update_timeslice_used (struct blkio_group *blkg,unsigned long time,unsigned long unaccounted_time)
block/blk-throttle.c +40 -104
228 lines not on this page · raw message
block/blk-cgroup.c +83 -32
137 lines not on this page · raw message
block/cfq-iosched.c +36 -98
205 lines not on this page · raw message
block/cfq.h +0 -8
@@ -67,12 +67,6 @@ static inline void cfq_blkiocg_update_co direction, sync); } -static inline void cfq_blkiocg_add_blkio_group (struct blkio_cgroup *blkcg, -struct blkio_group *blkg,struct request_queue *q,dev_t dev) -{ -blkiocg_add_blkio_group (blkcg, blkg, q, dev,BLKIO_POLICY_PROP ); -} -static inline int cfq_blkiocg_del_blkio_group (struct blkio_group *blkg) {return blkiocg_del_blkio_group (blkg);
@@ -105,8 +99,6 @@ static inline void cfq_blkiocg_update_di uint64_t bytes, bool direction, bool sync) {} static inline void cfq_blkiocg_update_completion_stats(struct blkio_group *blkg, uint64_t start_time, uint64_t io_start_time, bool direction, bool sync) {} -static inline void cfq_blkiocg_add_blkio_group(struct blkio_cgroup *blkcg, - struct blkio_group *blkg, struct request_queue *q, dev_t dev) {} static inline int cfq_blkiocg_del_blkio_group(struct blkio_group *blkg) {return 0 ;
Tejun Heo
maintainerPATCH 14/16 blkcg: don't allow or retain configuration of missing devices
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
blkcg is very peculiar in that it allows setting and remembering configurations for non-existent devices by maintaining separate data structures for configuration.
This behavior is completely out of the usual norms and outright confusing; furthermore, it uses dev_t number to match the configuration to devices, which is unpredictable to begin with and becomes completely unuseable if EXT_DEVT is fully used.
It is wholely unnecessary - we already have fully functional userland mechanism to program devices being hotplugged which has full access to device identification, connection topology and filesystem information.
Add a new struct blkio_group_conf which contains all blkcg configurations to blkio_group and let blkio_group, which can be created iff the associated device exists and is removed when the associated device goes away, carry all configurations.
Note that, after this patch, all newly created blkg's will always have the default configuration (unlimited for throttling and blkcg's weight for propio).
This patch makes blkio_policy_node meaningless but doesn't remove it. The next patch will.
--- block/blk-cgroup.c | 78 ++++++++++++++++++++++++++++++++++++-------------- block/blk-cgroup.h | 9 ++++++ block/blk-throttle.c | 8 ++-- block/cfq-iosched.c | 2 +- 4 files changed, 70 insertions(+), 27 deletions(-)
block/blk-cgroup.c +56 -22
@@ -820,9 +820,12 @@ static uint64_t blkio_get_stat(struct blkio_group *blkg, }static int blkio_policy_parse_and_set (char *buf, -struct blkio_policy_node *newpn,enum blkio_policy_id plid,int fileid) +struct blkio_policy_node *newpn, +enum blkio_policy_id plid,int fileid, +struct blkio_cgroup *blkcg) {struct gendisk *disk =NULL ; +struct blkio_group *blkg =NULL ;char *s[4 ], *p, *major_s =NULL , *minor_s =NULL ;unsigned long major, minor;int i =0 , ret = -EINVAL ;
@@ -868,12 +871,19 @@ static int blkio_policy_parse_and_set(char *buf, goto out;/* For rule removal, do not check for device presence. */ -if (temp) { - disk =get_gendisk (dev, &part); -if (!disk || part) { - ret = -ENODEV ; - goto out; - } + disk =get_gendisk (dev, &part); + +if ((!disk || part) && temp) { + ret = -ENODEV ; + goto out; + } + +rcu_read_lock (); + +if (disk && !part) { +spin_lock_irq (disk->queue->queue_lock); + blkg =blkg_lookup_create (blkcg, disk->queue, plid); +spin_unlock_irq (disk->queue->queue_lock); } newpn->dev = dev;
@@ -882,25 +892,46 @@ static int blkio_policy_parse_and_set(char *buf,case BLKIO_POLICY_PROP :if ((temp <BLKIO_WEIGHT_MIN && temp >0 ) || temp >BLKIO_WEIGHT_MAX ) - goto out; + goto out_unlock; newpn->plid = plid; newpn->fileid = fileid; newpn->val.weight = temp; +if (blkg) + blkg->conf.weight = temp;break ;case BLKIO_POLICY_THROTL :switch (fileid) {case BLKIO_THROTL_read_bps_device: +if (blkg) + blkg->conf.bps[READ ] = temp; + newpn->plid = plid; + newpn->fileid = fileid; + newpn->val.bps = temp; +break ;case BLKIO_THROTL_write_bps_device: +if (blkg) + blkg->conf.bps[WRITE ] = temp; newpn->plid = plid; newpn->fileid = fileid; newpn->val.bps = temp;break ;case BLKIO_THROTL_read_iops_device: +if (temp >THROTL_IOPS_MAX ) + goto out_unlock; + +if (blkg) + blkg->conf.iops[READ ] = temp; + newpn->plid = plid; + newpn->fileid = fileid; + newpn->val.iops = (unsigned int )temp; +break ;case BLKIO_THROTL_write_iops_device:if (temp >THROTL_IOPS_MAX ) - goto out; + goto out_unlock; +if (blkg) + blkg->conf.iops[WRITE ] = temp; newpn->plid = plid; newpn->fileid = fileid; newpn->val.iops = (unsigned int )temp;
@@ -911,6 +942,8 @@ static int blkio_policy_parse_and_set(char *buf,BUG (); } ret =0 ; +out_unlock: +rcu_read_unlock (); out:put_disk (disk);return ret;
@@ -1060,26 +1093,29 @@ static void blkio_update_policy_rule(struct blkio_policy_node *oldpn,static void blkio_update_blkg_policy (struct blkio_cgroup *blkcg,struct blkio_group *blkg,struct blkio_policy_node *pn) { -unsigned int weight, iops; -u64 bps; +struct blkio_group_conf *conf = &blkg->conf;switch (pn->plid) {case BLKIO_POLICY_PROP : - weight = pn->val.weight ? pn->val.weight : - blkcg->weight; -blkio_update_group_weight (blkg, weight); +blkio_update_group_weight (blkg, conf->weight ?: blkcg->weight);break ;case BLKIO_POLICY_THROTL :switch (pn->fileid) {case BLKIO_THROTL_read_bps_device: +blkio_update_group_bps (blkg, conf->bps[READ ] ?:-1 , + pn->fileid); +break ;case BLKIO_THROTL_write_bps_device: - bps = pn->val.bps ? pn->val.bps : (-1 ); -blkio_update_group_bps (blkg, bps, pn->fileid); +blkio_update_group_bps (blkg, conf->bps[WRITE ] ?:-1 , + pn->fileid);break ;case BLKIO_THROTL_read_iops_device: +blkio_update_group_iops (blkg, conf->iops[READ ] ?:-1 , + pn->fileid); +break ;case BLKIO_THROTL_write_iops_device: - iops = pn->val.iops ? pn->val.iops : (-1 ); -blkio_update_group_iops (blkg, iops, pn->fileid); +blkio_update_group_iops (blkg, conf->iops[WRITE ] ?:-1 , + pn->fileid);break ; }break ;
@@ -1117,7 +1153,7 @@ static int blkiocg_file_write(struct cgroup *cgrp, struct cftype *cft,int ret =0 ;char *buf;struct blkio_policy_node *newpn, *pn; -struct blkio_cgroup *blkcg; +struct blkio_cgroup *blkcg =cgroup_to_blkio_cgroup (cgrp);int keep_newpn =0 ;enum blkio_policy_id plid =BLKIOFILE_POLICY (cft->private);int fileid =BLKIOFILE_ATTR (cft->private);
@@ -1132,12 +1168,10 @@ static int blkiocg_file_write(struct cgroup *cgrp, struct cftype *cft, goto free_buf; } - ret =blkio_policy_parse_and_set (buf, newpn, plid, fileid); + ret =blkio_policy_parse_and_set (buf, newpn, plid, fileid, blkcg);if (ret) goto free_newpn; - blkcg =cgroup_to_blkio_cgroup (cgrp); -spin_lock_irq (&blkcg->lock); pn =blkio_policy_search_node (blkcg, newpn->dev, plid, fileid);
block/blk-cgroup.h +9 -0
@@ -154,6 +154,12 @@ struct blkio_group_stats_cpu {struct u64_stats_sync syncp; }; +struct blkio_group_conf { +unsigned int weight; +unsigned int iops[2 ]; +u64 bps[2 ]; +}; +struct blkio_group {/* Pointer to the associated request_queue, RCU protected */ struct request_queue __rcu *q;
@@ -166,6 +172,9 @@ struct blkio_group {/* policy which owns this blk group */ enum blkio_policy_id plid; +/* Configuration */ +struct blkio_group_conf conf; +/* Need to serialize the stats in the case of reset/update */ spinlock_t stats_lock;struct blkio_group_stats stats;
block/blk-throttle.c +4 -4
@@ -196,10 +196,10 @@ static struct blkio_group *throtl_alloc_blkio_group(struct request_queue *q,bio_list_init (&tg->bio_lists[1 ]); tg->limits_changed = false; - tg->bps[READ ] =blkcg_get_read_bps (blkcg, tg->blkg.dev); - tg->bps[WRITE ] =blkcg_get_write_bps (blkcg, tg->blkg.dev); - tg->iops[READ ] =blkcg_get_read_iops (blkcg, tg->blkg.dev); - tg->iops[WRITE ] =blkcg_get_write_iops (blkcg, tg->blkg.dev); + tg->bps[READ ] =-1 ; + tg->bps[WRITE ] =-1 ; + tg->iops[READ ] =-1 ; + tg->iops[WRITE ] =-1 ;/* * Take the initial reference that will be released on destroy
block/cfq-iosched.c +1 -1
@@ -1083,7 +1083,7 @@ static struct blkio_group *cfq_alloc_blkio_group(struct request_queue *q,return NULL ;cfq_init_cfqg_base (cfqg); - cfqg->weight =blkcg_get_weight (blkcg, cfqg->blkg.dev); + cfqg->weight = blkcg->weight;/* * Take the initial reference that will be released on destroy
Signature · 1 line
1.7.7.3
Re: [PATCH 14/16] blkcg: don't allow or retain configuration of missing devices
On Mon, Jan 23, 2012 at 03:09:51PM -0800, Tejun Heo wrote:
[..]
Quoted 24 lines
static int blkio_policy_parse_and_set(char *buf,
block/blk-cgroup.c +7
-7- struct blkio_policy_node *newpn, enum blkio_policy_id plid, int fileid) +struct blkio_policy_node *newpn, + enum blkio_policy_id plid, int fileid, + struct blkio_cgroup *blkcg) { struct gendisk *disk = NULL; +struct blkio_group *blkg =NULL ;char *s[4 ], *p, *major_s =NULL , *minor_s =NULL ;unsigned long major, minor;int i =0 , ret = -EINVAL ;@@ -868,12 +871,19 @@ static int blkio_policy_parse_and_set(char *buf, goto out;/* For rule removal, do not check for device presence. */ -if (temp) { - disk =get_gendisk (dev, &part); -if (!disk || part) { - ret = -ENODEV ; - goto out; - } + disk =get_gendisk (dev, &part); + +if ((!disk || part) && temp) {
block/blk-cgroup.c static int blkio_policy_parse_and_set(char *buf, · line 886
Quoted 10 lines
block/blk-cgroup.c +10
-0+ ret = -ENODEV ; + goto out; + } + +rcu_read_lock (); + +if (disk && !part) { +spin_lock_irq (disk->queue->queue_lock); + blkg =blkg_lookup_create (blkcg, disk->queue, plid); +spin_unlock_irq (disk->queue->queue_lock);
If queue is in bypass mode, is group creation and linking allowed when somebody is trying to set per device rules. (/me is thinking if there are any potential races between elevator switch and rule setting in cgroup).
Thanks
Vivek
Tejun Heo
maintainerRe: [PATCH 14/16] blkcg: don't allow or retain configuration of missing devices
Quoted 10 lines · Vivek Goyal
Quoted 9 lines · Tejun Heo
block/blk-cgroup.c +9
-0+ disk =get_gendisk (dev, &part); + +if ((!disk || part) && temp) { + ret = -ENODEV ; + goto out; + } + +rcu_read_lock (); +
Quoted 8 lines
block/blk-cgroup.c static int blkio_policy_parse_and_set(char *buf, · line 886
Quoted 4 lines
block/blk-cgroup.c +4
-0+if (disk && !part) { +spin_lock_irq (disk->queue->queue_lock); + blkg =blkg_lookup_create (blkcg, disk->queue, plid); +spin_unlock_irq (disk->queue->queue_lock);If queue is in bypass mode, is group creation and linking allowed when somebody is trying to set per device rules. (/me is thinking if there are any potential races between elevator switch and rule setting in cgroup).
Yeap, there is. Nice catch. blkg_lookup_create() should be testing blk_queue_bypass() instead of blk_queue_dead() and parse_and_set should probably retry after a bit. Will update.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 14/16] blkcg: don't allow or retain configuration of missing devices
Quoted 20 lines · Tejun Heo
Quoted 10 lines · Vivek Goyal
Quoted 9 lines · Tejun Heo
block/blk-cgroup.c +9
-0+ disk =get_gendisk (dev, &part); + +if ((!disk || part) && temp) { + ret = -ENODEV ; + goto out; + } + +rcu_read_lock (); +Quoted 8 lines
block/blk-cgroup.c static int blkio_policy_parse_and_set(char *buf, · line 886
Quoted 4 lines
block/blk-cgroup.c +4
-0+if (disk && !part) { +spin_lock_irq (disk->queue->queue_lock); + blkg =blkg_lookup_create (blkcg, disk->queue, plid); +spin_unlock_irq (disk->queue->queue_lock);If queue is in bypass mode, is group creation and linking allowed when somebody is trying to set per device rules. (/me is thinking if there are any potential races between elevator switch and rule setting in cgroup).
Quoted 3 lines
Yeap, there is. Nice catch.
blkg_lookup_create()should be testingblk_queue_bypass()instead ofblk_queue_dead()andparse_and_setshould probably retry after a bit. Will update.
So now group creation fails if queue is in bypass mode. So a per device rule setting will fail both for cfq and throttling logic while device is still around. So a user is supposed to retry? How would a user know that elevator switch is happening and he should retry and till when should he retry.
Thanks
Vivek
Tejun Heo
maintainerRe: [PATCH 14/16] blkcg: don't allow or retain configuration of missing devices
Quoted 9 lines · Vivek Goyal
Quoted 3 lines
Yeap, there is. Nice catch.
blkg_lookup_create()should be testingblk_queue_bypass()instead ofblk_queue_dead()andparse_and_setshould probably retry after a bit. Will update.So now group creation fails if queue is in bypass mode. So a per device rule setting will fail both for cfq and throttling logic while device is still around. So a user is supposed to retry? How would a user know that elevator switch is happening and he should retry and till when should he retry.
Ummm... I'm working on the second patch which makes parse_and_set do return restart_syscall().
Signature · 1 line
tejun
PATCH 14/16 blkcg: don't allow or retain configuration of missing devices
In reply to Tejun Heoblkcg is very peculiar in that it allows setting and remembering configurations for non-existent devices by maintaining separate data structures for configuration.
This behavior is completely out of the usual norms and outright confusing; furthermore, it uses dev_t number to match the configuration to devices, which is unpredictable to begin with and becomes completely unuseable if EXT_DEVT is fully used.
It is wholely unnecessary - we already have fully functional userland mechanism to program devices being hotplugged which has full access to device identification, connection topology and filesystem information.
Add a new struct blkio_group_conf which contains all blkcg configurations to blkio_group and let blkio_group, which can be created iff the associated device exists and is removed when the associated device goes away, carry all configurations.
Note that, after this patch, all newly created blkg's will always have the default configuration (unlimited for throttling and blkcg's weight for propio).
This patch makes blkio_policy_node meaningless but doesn't remove it. The next patch will.
-v2: Updated to retry after short sleep if blkg lookup/creation failed
due to the queue being temporarily bypassed as indicated by -EBUSY return. Pointed out by Vivek.
---
Alright, these three updates should solve the problem. Git branch updated accordingly. I'll repost the whole series once things settle down.
Thanks.
block/blk-cgroup.c | 94 +++++++++++++++++++++++++++++++++++++++------------ block/blk-cgroup.h | 9 ++++ block/blk-throttle.c | 8 ++-- block/cfq-iosched.c | 2 - 4 files changed, 87 insertions(+), 26 deletions(-)
block/blk-cgroup.c +73 -21
@@ -823,9 +823,12 @@ static uint64_t blkio_get_stat(struct bl }static int blkio_policy_parse_and_set (char *buf, -struct blkio_policy_node *newpn,enum blkio_policy_id plid,int fileid) +struct blkio_policy_node *newpn, +enum blkio_policy_id plid,int fileid, +struct blkio_cgroup *blkcg) {struct gendisk *disk =NULL ; +struct blkio_group *blkg =NULL ;char *s[4 ], *p, *major_s =NULL , *minor_s =NULL ;unsigned long major, minor;int i =0 , ret = -EINVAL ;
@@ -871,11 +874,25 @@ static int blkio_policy_parse_and_set(ch goto out;/* For rule removal, do not check for device presence. */ -if (temp) { - disk =get_gendisk (dev, &part); -if (!disk || part) { - ret = -ENODEV ; - goto out; + disk =get_gendisk (dev, &part); + +if ((!disk || part) && temp) { + ret = -ENODEV ; + goto out; + } + +rcu_read_lock (); + +if (disk && !part) { +spin_lock_irq (disk->queue->queue_lock); + blkg =blkg_lookup_create (blkcg, disk->queue, plid); +spin_unlock_irq (disk->queue->queue_lock); + +if (IS_ERR (blkg)) { + ret =PTR_ERR (blkg); +if (ret == -EBUSY ) + goto out_unlock; + blkg =NULL ; } }
@@ -885,25 +902,46 @@ static int blkio_policy_parse_and_set(chcase BLKIO_POLICY_PROP :if ((temp <BLKIO_WEIGHT_MIN && temp >0 ) || temp >BLKIO_WEIGHT_MAX ) - goto out; + goto out_unlock; newpn->plid = plid; newpn->fileid = fileid; newpn->val.weight = temp; +if (blkg) + blkg->conf.weight = temp;break ;case BLKIO_POLICY_THROTL :switch (fileid) {case BLKIO_THROTL_read_bps_device: +if (blkg) + blkg->conf.bps[READ ] = temp; + newpn->plid = plid; + newpn->fileid = fileid; + newpn->val.bps = temp; +break ;case BLKIO_THROTL_write_bps_device: +if (blkg) + blkg->conf.bps[WRITE ] = temp; newpn->plid = plid; newpn->fileid = fileid; newpn->val.bps = temp;break ;case BLKIO_THROTL_read_iops_device: +if (temp >THROTL_IOPS_MAX ) + goto out_unlock; + +if (blkg) + blkg->conf.iops[READ ] = temp; + newpn->plid = plid; + newpn->fileid = fileid; + newpn->val.iops = (unsigned int )temp; +break ;case BLKIO_THROTL_write_iops_device:if (temp >THROTL_IOPS_MAX ) - goto out; + goto out_unlock; +if (blkg) + blkg->conf.iops[WRITE ] = temp; newpn->plid = plid; newpn->fileid = fileid; newpn->val.iops = (unsigned int )temp;
@@ -914,8 +952,21 @@ static int blkio_policy_parse_and_set(chBUG (); } ret =0 ; +out_unlock: +rcu_read_unlock (); out:put_disk (disk); + +/* +* If queue was bypassing, we should retry. Do so after a short +* msleep(). It isn't strictly necessary but queue can be +* bypassing for some time and it's always nice to avoid busy +* looping. +*/ +if (ret == -EBUSY ) { +msleep (10 ); +return restart_syscall (); + }return ret; }
@@ -1063,26 +1114,29 @@ static void blkio_update_policy_rule(strstatic void blkio_update_blkg_policy (struct blkio_cgroup *blkcg,struct blkio_group *blkg,struct blkio_policy_node *pn) { -unsigned int weight, iops; -u64 bps; +struct blkio_group_conf *conf = &blkg->conf;switch (pn->plid) {case BLKIO_POLICY_PROP : - weight = pn->val.weight ? pn->val.weight : - blkcg->weight; -blkio_update_group_weight (blkg, weight); +blkio_update_group_weight (blkg, conf->weight ?: blkcg->weight);break ;case BLKIO_POLICY_THROTL :switch (pn->fileid) {case BLKIO_THROTL_read_bps_device: +blkio_update_group_bps (blkg, conf->bps[READ ] ?:-1 , + pn->fileid); +break ;case BLKIO_THROTL_write_bps_device: - bps = pn->val.bps ? pn->val.bps : (-1 ); -blkio_update_group_bps (blkg, bps, pn->fileid); +blkio_update_group_bps (blkg, conf->bps[WRITE ] ?:-1 , + pn->fileid);break ;case BLKIO_THROTL_read_iops_device: +blkio_update_group_iops (blkg, conf->iops[READ ] ?:-1 , + pn->fileid); +break ;case BLKIO_THROTL_write_iops_device: - iops = pn->val.iops ? pn->val.iops : (-1 ); -blkio_update_group_iops (blkg, iops, pn->fileid); +blkio_update_group_iops (blkg, conf->iops[WRITE ] ?:-1 , + pn->fileid);break ; }break ;
@@ -1120,7 +1174,7 @@ static int blkiocg_file_write(struct cgrint ret =0 ;char *buf;struct blkio_policy_node *newpn, *pn; -struct blkio_cgroup *blkcg; +struct blkio_cgroup *blkcg =cgroup_to_blkio_cgroup (cgrp);int keep_newpn =0 ;enum blkio_policy_id plid =BLKIOFILE_POLICY (cft->private);int fileid =BLKIOFILE_ATTR (cft->private);
@@ -1135,12 +1189,10 @@ static int blkiocg_file_write(struct cgr goto free_buf; } - ret =blkio_policy_parse_and_set (buf, newpn, plid, fileid); + ret =blkio_policy_parse_and_set (buf, newpn, plid, fileid, blkcg);if (ret) goto free_newpn; - blkcg =cgroup_to_blkio_cgroup (cgrp); -spin_lock_irq (&blkcg->lock); pn =blkio_policy_search_node (blkcg, newpn->dev, plid, fileid);
block/blk-cgroup.h +9 -0
@@ -154,6 +154,12 @@ struct blkio_group_stats_cpu {struct u64_stats_sync syncp; }; +struct blkio_group_conf { +unsigned int weight; +unsigned int iops[2 ]; +u64 bps[2 ]; +}; +struct blkio_group {/* Pointer to the associated request_queue, RCU protected */ struct request_queue __rcu *q;
@@ -166,6 +172,9 @@ struct blkio_group {/* policy which owns this blk group */ enum blkio_policy_id plid; +/* Configuration */ +struct blkio_group_conf conf; +/* Need to serialize the stats in the case of reset/update */ spinlock_t stats_lock;struct blkio_group_stats stats;
block/blk-throttle.c +4 -4
@@ -196,10 +196,10 @@ static struct blkio_group *throtl_alloc_bio_list_init (&tg->bio_lists[1 ]); tg->limits_changed = false; - tg->bps[READ ] =blkcg_get_read_bps (blkcg, tg->blkg.dev); - tg->bps[WRITE ] =blkcg_get_write_bps (blkcg, tg->blkg.dev); - tg->iops[READ ] =blkcg_get_read_iops (blkcg, tg->blkg.dev); - tg->iops[WRITE ] =blkcg_get_write_iops (blkcg, tg->blkg.dev); + tg->bps[READ ] =-1 ; + tg->bps[WRITE ] =-1 ; + tg->iops[READ ] =-1 ; + tg->iops[WRITE ] =-1 ;/* * Take the initial reference that will be released on destroy
block/cfq-iosched.c +1 -1
@@ -1083,7 +1083,7 @@ static struct blkio_group *cfq_alloc_blkreturn NULL ;cfq_init_cfqg_base (cfqg); - cfqg->weight =blkcg_get_weight (blkcg, cfqg->blkg.dev); + cfqg->weight = blkcg->weight;/* * Take the initial reference that will be released on destroy
Re: [PATCH UPDATED 14/16] blkcg: don't allow or retain configuration of missing devices
On Tue, Jan 24, 2012 at 11:30:58AM -0800, Tejun Heo wrote:
[..]
block/blk-cgroup.c static int blkio_policy_parse_and_set(ch · line 970
Quoted 12 lines
block/blk-cgroup.c +11
-0+ +/* +* If queue was bypassing, we should retry. Do so after a short +* msleep(). It isn't strictly necessary but queue can be +* bypassing for some time and it's always nice to avoid busy +* looping. +*/ +if (ret == -EBUSY ) { +msleep (10 ); +return restart_syscall (); + }return ret;
Thanks. This one looks better.
Vivek
Tejun Heo
maintainerPATCH 15/16 blkcg: kill blkio_policy_node
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
Now that blkcg configuration lives in blkg's, blkio_policy_node is no longer necessary. Kill it.
blkio_policy_parse_and_set() now fails if invoked for missing device and functions to print out configurations are updated to print from blkg's.
cftype_blkg_same_policy() is dropped along with other policy functions for consistency. Its one line is open coded in the only user - blkio_read_blkg_stats().
--- block/blk-cgroup.c | 430 +++++++--------------------------------------------- block/blk-cgroup.h | 32 ---- 2 files changed, 58 insertions(+), 404 deletions(-)
block/blk-cgroup.c +58 -372
@@ -59,54 +59,6 @@ struct cgroup_subsys blkio_subsys = { };EXPORT_SYMBOL_GPL (blkio_subsys); -static inline void blkio_policy_insert_node (struct blkio_cgroup *blkcg, -struct blkio_policy_node *pn) -{ -list_add (&pn->node, &blkcg->policy_list); -} - -static inline bool cftype_blkg_same_policy (struct cftype *cft, -struct blkio_group *blkg) -{ -enum blkio_policy_id plid =BLKIOFILE_POLICY (cft->private); - -if (blkg->plid == plid) -return 1 ; - -return 0 ; -} - -/* Determines if policy node matches cgroup file being accessed */ -static inline bool pn_matches_cftype (struct cftype *cft, -struct blkio_policy_node *pn) -{ -enum blkio_policy_id plid =BLKIOFILE_POLICY (cft->private); -int fileid =BLKIOFILE_ATTR (cft->private); - -return (plid == pn->plid && fileid == pn->fileid); -} - -/* Must be called with blkcg->lock held */ -static inline void blkio_policy_delete_node (struct blkio_policy_node *pn) -{ -list_del (&pn->node); -} - -/* Must be called with blkcg->lock held */ -static struct blkio_policy_node * -blkio_policy_search_node (const struct blkio_cgroup *blkcg,dev_t dev, -enum blkio_policy_id plid,int fileid) -{ -struct blkio_policy_node *pn; - -list_for_each_entry (pn, &blkcg->policy_list, node) { -if (pn->dev == dev && pn->plid == plid && pn->fileid == fileid) -return pn; - } - -return NULL ; -} -struct blkio_cgroup *cgroup_to_blkio_cgroup (struct cgroup *cgroup) { return container_of(cgroup_subsys_state(cgroup, blkio_subsys_id),
@@ -819,10 +771,8 @@ static uint64_t blkio_get_stat(struct blkio_group *blkg,return disk_total; } -static int blkio_policy_parse_and_set (char *buf, -struct blkio_policy_node *newpn, -enum blkio_policy_id plid,int fileid, -struct blkio_cgroup *blkcg) +static int blkio_policy_parse_and_set (char *buf,enum blkio_policy_id plid, +int fileid,struct blkio_cgroup *blkcg) {struct gendisk *disk =NULL ;struct blkio_group *blkg =NULL ;
@@ -870,23 +820,18 @@ static int blkio_policy_parse_and_set(char *buf,if (strict_strtoull (s[1 ],10 , &temp)) goto out; -/* For rule removal, do not check for device presence. */ disk =get_gendisk (dev, &part); - -if ((!disk || part) && temp) { - ret = -ENODEV ; +if (!disk || part) goto out; - }rcu_read_lock (); -if (disk && !part) { -spin_lock_irq (disk->queue->queue_lock); - blkg =blkg_lookup_create (blkcg, disk->queue, plid); -spin_unlock_irq (disk->queue->queue_lock); - } +spin_lock_irq (disk->queue->queue_lock); + blkg =blkg_lookup_create (blkcg, disk->queue, plid); +spin_unlock_irq (disk->queue->queue_lock); - newpn->dev = dev; +if (!blkg) + goto out_unlock;switch (plid) {case BLKIO_POLICY_PROP :
@@ -894,47 +839,30 @@ static int blkio_policy_parse_and_set(char *buf, temp > BLKIO_WEIGHT_MAX) goto out_unlock; - newpn->plid = plid; - newpn->fileid = fileid; - newpn->val.weight = temp; -if (blkg) - blkg->conf.weight = temp; + blkg->conf.weight = temp; +blkio_update_group_weight (blkg, temp ?: blkcg->weight);break ;case BLKIO_POLICY_THROTL :switch (fileid) {case BLKIO_THROTL_read_bps_device: -if (blkg) - blkg->conf.bps[READ ] = temp; - newpn->plid = plid; - newpn->fileid = fileid; - newpn->val.bps = temp; + blkg->conf.bps[READ ] = temp; +blkio_update_group_bps (blkg, temp ?:-1 , fileid);break ;case BLKIO_THROTL_write_bps_device: -if (blkg) - blkg->conf.bps[WRITE ] = temp; - newpn->plid = plid; - newpn->fileid = fileid; - newpn->val.bps = temp; + blkg->conf.bps[WRITE ] = temp; +blkio_update_group_bps (blkg, temp ?:-1 , fileid);break ;case BLKIO_THROTL_read_iops_device:if (temp >THROTL_IOPS_MAX ) goto out_unlock; - -if (blkg) - blkg->conf.iops[READ ] = temp; - newpn->plid = plid; - newpn->fileid = fileid; - newpn->val.iops = (unsigned int )temp; + blkg->conf.iops[READ ] = temp; +blkio_update_group_iops (blkg, temp ?:-1 , fileid);break ;case BLKIO_THROTL_write_iops_device:if (temp >THROTL_IOPS_MAX ) goto out_unlock; - -if (blkg) - blkg->conf.iops[WRITE ] = temp; - newpn->plid = plid; - newpn->fileid = fileid; - newpn->val.iops = (unsigned int )temp; + blkg->conf.iops[WRITE ] = temp; +blkio_update_group_iops (blkg, temp ?:-1 , fileid);break ; }break ;
@@ -949,212 +877,12 @@ out:return ret; } -unsigned int blkcg_get_weight (struct blkio_cgroup *blkcg, -dev_t dev) -{ -struct blkio_policy_node *pn; -unsigned long flags; -unsigned int weight; - -spin_lock_irqsave (&blkcg->lock, flags); - - pn =blkio_policy_search_node (blkcg, dev,BLKIO_POLICY_PROP , - BLKIO_PROP_weight_device); -if (pn) - weight = pn->val.weight; -else - weight = blkcg->weight; - -spin_unlock_irqrestore (&blkcg->lock, flags); - -return weight; -} -EXPORT_SYMBOL_GPL (blkcg_get_weight); - -uint64_t blkcg_get_read_bps (struct blkio_cgroup *blkcg,dev_t dev) -{ -struct blkio_policy_node *pn; -unsigned long flags; -uint64_t bps =-1 ; - -spin_lock_irqsave (&blkcg->lock, flags); - pn =blkio_policy_search_node (blkcg, dev,BLKIO_POLICY_THROTL , - BLKIO_THROTL_read_bps_device); -if (pn) - bps = pn->val.bps; -spin_unlock_irqrestore (&blkcg->lock, flags); - -return bps; -} - -uint64_t blkcg_get_write_bps (struct blkio_cgroup *blkcg,dev_t dev) -{ -struct blkio_policy_node *pn; -unsigned long flags; -uint64_t bps =-1 ; - -spin_lock_irqsave (&blkcg->lock, flags); - pn =blkio_policy_search_node (blkcg, dev,BLKIO_POLICY_THROTL , - BLKIO_THROTL_write_bps_device); -if (pn) - bps = pn->val.bps; -spin_unlock_irqrestore (&blkcg->lock, flags); - -return bps; -} - -unsigned int blkcg_get_read_iops (struct blkio_cgroup *blkcg,dev_t dev) -{ -struct blkio_policy_node *pn; -unsigned long flags; -unsigned int iops =-1 ; - -spin_lock_irqsave (&blkcg->lock, flags); - pn =blkio_policy_search_node (blkcg, dev,BLKIO_POLICY_THROTL , - BLKIO_THROTL_read_iops_device); -if (pn) - iops = pn->val.iops; -spin_unlock_irqrestore (&blkcg->lock, flags); - -return iops; -} - -unsigned int blkcg_get_write_iops (struct blkio_cgroup *blkcg,dev_t dev) -{ -struct blkio_policy_node *pn; -unsigned long flags; -unsigned int iops =-1 ; - -spin_lock_irqsave (&blkcg->lock, flags); - pn =blkio_policy_search_node (blkcg, dev,BLKIO_POLICY_THROTL , - BLKIO_THROTL_write_iops_device); -if (pn) - iops = pn->val.iops; -spin_unlock_irqrestore (&blkcg->lock, flags); - -return iops; -} - -/* Checks whether user asked for deleting a policy rule */ -static bool blkio_delete_rule_command (struct blkio_policy_node *pn) -{ -switch (pn->plid) { -case BLKIO_POLICY_PROP : -if (pn->val.weight ==0 ) -return 1 ; -break ; -case BLKIO_POLICY_THROTL : -switch (pn->fileid) { -case BLKIO_THROTL_read_bps_device: -case BLKIO_THROTL_write_bps_device: -if (pn->val.bps ==0 ) -return 1 ; -break ; -case BLKIO_THROTL_read_iops_device: -case BLKIO_THROTL_write_iops_device: -if (pn->val.iops ==0 ) -return 1 ; - } -break ; -default : -BUG (); - } - -return 0 ; -} - -static void blkio_update_policy_rule (struct blkio_policy_node *oldpn, -struct blkio_policy_node *newpn) -{ -switch (oldpn->plid) { -case BLKIO_POLICY_PROP : - oldpn->val.weight = newpn->val.weight; -break ; -case BLKIO_POLICY_THROTL : -switch (newpn->fileid) { -case BLKIO_THROTL_read_bps_device: -case BLKIO_THROTL_write_bps_device: - oldpn->val.bps = newpn->val.bps; -break ; -case BLKIO_THROTL_read_iops_device: -case BLKIO_THROTL_write_iops_device: - oldpn->val.iops = newpn->val.iops; - } -break ; -default : -BUG (); - } -} - -/* -* Some rules/values in blkg have changed. Propagate those to respective -* policies. -*/ -static void blkio_update_blkg_policy (struct blkio_cgroup *blkcg, -struct blkio_group *blkg,struct blkio_policy_node *pn) -{ -struct blkio_group_conf *conf = &blkg->conf; - -switch (pn->plid) { -case BLKIO_POLICY_PROP : -blkio_update_group_weight (blkg, conf->weight ?: blkcg->weight); -break ; -case BLKIO_POLICY_THROTL : -switch (pn->fileid) { -case BLKIO_THROTL_read_bps_device: -blkio_update_group_bps (blkg, conf->bps[READ ] ?:-1 , - pn->fileid); -break ; -case BLKIO_THROTL_write_bps_device: -blkio_update_group_bps (blkg, conf->bps[WRITE ] ?:-1 , - pn->fileid); -break ; -case BLKIO_THROTL_read_iops_device: -blkio_update_group_iops (blkg, conf->iops[READ ] ?:-1 , - pn->fileid); -break ; -case BLKIO_THROTL_write_iops_device: -blkio_update_group_iops (blkg, conf->iops[WRITE ] ?:-1 , - pn->fileid); -break ; - } -break ; -default : -BUG (); - } -} - -/* -* A policy node rule has been updated. Propagate this update to all the -* block groups which might be affected by this update. -*/ -static void blkio_update_policy_node_blkg (struct blkio_cgroup *blkcg, -struct blkio_policy_node *pn) -{ -struct blkio_group *blkg; -struct hlist_node *n; - -spin_lock (&blkio_list_lock); -spin_lock_irq (&blkcg->lock); - -hlist_for_each_entry (blkg, n, &blkcg->blkg_list, blkcg_node) { -if (pn->dev != blkg->dev || pn->plid != blkg->plid) -continue ; -blkio_update_blkg_policy (blkcg, blkg, pn); - } - -spin_unlock_irq (&blkcg->lock); -spin_unlock (&blkio_list_lock); -} -static int blkiocg_file_write (struct cgroup *cgrp,struct cftype *cft,const char *buffer) {int ret =0 ;char *buf; -struct blkio_policy_node *newpn, *pn;struct blkio_cgroup *blkcg =cgroup_to_blkio_cgroup (cgrp); -int keep_newpn =0 ;enum blkio_policy_id plid =BLKIOFILE_POLICY (cft->private);int fileid =BLKIOFILE_ATTR (cft->private);
@@ -1162,69 +890,42 @@ static int blkiocg_file_write(struct cgroup *cgrp, struct cftype *cft,if (!buf)return -ENOMEM ; - newpn =kzalloc (sizeof (*newpn),GFP_KERNEL ); -if (!newpn) { - ret = -ENOMEM ; - goto free_buf; - } - - ret =blkio_policy_parse_and_set (buf, newpn, plid, fileid, blkcg); -if (ret) - goto free_newpn; - -spin_lock_irq (&blkcg->lock); - - pn =blkio_policy_search_node (blkcg, newpn->dev, plid, fileid); -if (!pn) { -if (!blkio_delete_rule_command (newpn)) { -blkio_policy_insert_node (blkcg, newpn); - keep_newpn =1 ; - } -spin_unlock_irq (&blkcg->lock); - goto update_io_group; - } - -if (blkio_delete_rule_command (newpn)) { -blkio_policy_delete_node (pn); -kfree (pn); -spin_unlock_irq (&blkcg->lock); - goto update_io_group; - } -spin_unlock_irq (&blkcg->lock); - -blkio_update_policy_rule (pn, newpn); - -update_io_group: -blkio_update_policy_node_blkg (blkcg, newpn); - -free_newpn: -if (!keep_newpn) -kfree (newpn); -free_buf: + ret =blkio_policy_parse_and_set (buf, plid, fileid, blkcg);kfree (buf);return ret; } -static void -blkio_print_policy_node (struct seq_file *m,struct blkio_policy_node *pn) +static void blkio_print_group_conf (struct cftype *cft,struct blkio_group *blkg, +struct seq_file *m) { -switch (pn->plid) { +int fileid =BLKIOFILE_ATTR (cft->private); +int rw =WRITE ; + +switch (blkg->plid) {case BLKIO_POLICY_PROP : -if (pn->fileid == BLKIO_PROP_weight_device) -seq_printf (m,"%u:%u\t%u\n" ,MAJOR (pn->dev), -MINOR (pn->dev), pn->val.weight); +if (blkg->conf.weight) +seq_printf (m,"%u:%u\t%u\n" ,MAJOR (blkg->dev), +MINOR (blkg->dev), blkg->conf.weight);break ;case BLKIO_POLICY_THROTL : -switch (pn->fileid) { +switch (fileid) {case BLKIO_THROTL_read_bps_device: + rw =READ ;case BLKIO_THROTL_write_bps_device: -seq_printf (m,"%u:%u\t%llu\n" ,MAJOR (pn->dev), -MINOR (pn->dev), pn->val.bps); +if (blkg->conf.bps[rw]) +seq_printf (m,"%u:%u\t%llu\n" , +MAJOR (blkg->dev), +MINOR (blkg->dev), + blkg->conf.bps[rw]);break ;case BLKIO_THROTL_read_iops_device: + rw =READ ;case BLKIO_THROTL_write_iops_device: -seq_printf (m,"%u:%u\t%u\n" ,MAJOR (pn->dev), -MINOR (pn->dev), pn->val.iops); +if (blkg->conf.iops[rw]) +seq_printf (m,"%u:%u\t%u\n" , +MAJOR (blkg->dev), +MINOR (blkg->dev), + blkg->conf.iops[rw]);break ; }break ;
@@ -1234,20 +935,17 @@ blkio_print_policy_node(struct seq_file *m, struct blkio_policy_node *pn) }/* cgroup files which read their data from policy nodes end up here */ -static void blkio_read_policy_node_files (struct cftype *cft, -struct blkio_cgroup *blkcg,struct seq_file *m) +static void blkio_read_conf (struct cftype *cft,struct blkio_cgroup *blkcg, +struct seq_file *m) { -struct blkio_policy_node *pn; +struct blkio_group *blkg; +struct hlist_node *n; -if (!list_empty (&blkcg->policy_list)) { -spin_lock_irq (&blkcg->lock); -list_for_each_entry (pn, &blkcg->policy_list, node) { -if (!pn_matches_cftype (cft, pn)) -continue ; -blkio_print_policy_node (m, pn); - } -spin_unlock_irq (&blkcg->lock); - } +spin_lock_irq (&blkcg->lock); +hlist_for_each_entry (blkg, n, &blkcg->blkg_list, blkcg_node) +if (BLKIOFILE_POLICY (cft->private) == blkg->plid) +blkio_print_group_conf (cft, blkg, m); +spin_unlock_irq (&blkcg->lock); } static int blkiocg_file_read(struct cgroup *cgrp, struct cftype *cft,
@@ -1263,7 +961,7 @@ static int blkiocg_file_read(struct cgroup *cgrp, struct cftype *cft,case BLKIO_POLICY_PROP :switch (name) {case BLKIO_PROP_weight_device: -blkio_read_policy_node_files (cft, blkcg, m); +blkio_read_conf (cft, blkcg, m);return 0 ;default :BUG ();
@@ -1275,7 +973,7 @@ static int blkiocg_file_read(struct cgroup *cgrp, struct cftype *cft,case BLKIO_THROTL_write_bps_device:case BLKIO_THROTL_read_iops_device:case BLKIO_THROTL_write_iops_device: -blkio_read_policy_node_files (cft, blkcg, m); +blkio_read_conf (cft, blkcg, m);return 0 ;default :BUG ();
@@ -1299,7 +997,7 @@ static int blkio_read_blkg_stats(struct blkio_cgroup *blkcg,rcu_read_lock ();hlist_for_each_entry_rcu (blkg, n, &blkcg->blkg_list, blkcg_node) {if (blkg->dev) { -if (!cftype_blkg_same_policy (cft, blkg)) +if (BLKIOFILE_POLICY (cft->private) != blkg->plid)continue ;if (pcpu) cgroup_total += blkio_get_stat_cpu(blkg, cb,
@@ -1398,11 +1096,10 @@ static int blkiocg_file_read_map(struct cgroup *cgrp, struct cftype *cft,return 0 ; } -static int blkio_weight_write (struct blkio_cgroup *blkcg,u64 val) +static int blkio_weight_write (struct blkio_cgroup *blkcg,int plid,u64 val) {struct blkio_group *blkg;struct hlist_node *n; -struct blkio_policy_node *pn;if (val <BLKIO_WEIGHT_MIN || val >BLKIO_WEIGHT_MAX )return -EINVAL ;
@@ -1411,14 +1108,10 @@ static int blkio_weight_write(struct blkio_cgroup *blkcg, u64 val)spin_lock_irq (&blkcg->lock); blkcg->weight = (unsigned int )val; -hlist_for_each_entry (blkg, n, &blkcg->blkg_list, blkcg_node) { - pn =blkio_policy_search_node (blkcg, blkg->dev, -BLKIO_POLICY_PROP , BLKIO_PROP_weight_device); -if (pn) -continue ; +hlist_for_each_entry (blkg, n, &blkcg->blkg_list, blkcg_node) +if (blkg->plid == plid && !blkg->conf.weight) +blkio_update_group_weight (blkg, blkcg->weight); -blkio_update_group_weight (blkg, blkcg->weight); - }spin_unlock_irq (&blkcg->lock);spin_unlock (&blkio_list_lock);return 0 ;
@@ -1457,7 +1150,7 @@ blkiocg_file_write_u64(struct cgroup *cgrp, struct cftype *cft, u64 val)case BLKIO_POLICY_PROP :switch (name) {case BLKIO_PROP_weight: -return blkio_weight_write (blkcg, val); +return blkio_weight_write (blkcg, plid, val); }break ;default :
@@ -1638,7 +1331,6 @@ static void blkiocg_destroy(struct cgroup_subsys *subsys, struct cgroup *cgroup)struct blkio_group *blkg;struct request_queue *q;struct blkio_policy_type *blkiop; -struct blkio_policy_node *pn, *pntmp;rcu_read_lock (); do {
@@ -1670,11 +1362,6 @@ static void blkiocg_destroy(struct cgroup_subsys *subsys, struct cgroup *cgroup)spin_unlock (&blkio_list_lock); }while (1 ); -list_for_each_entry_safe (pn, pntmp, &blkcg->policy_list, node) { -blkio_policy_delete_node (pn); -kfree (pn); - } -free_css_id (&blkio_subsys, &blkcg->css);rcu_read_unlock ();if (blkcg != &blkio_root_cgroup)
@@ -1701,7 +1388,6 @@ done:spin_lock_init (&blkcg->lock);INIT_HLIST_HEAD (&blkcg->blkg_list); -INIT_LIST_HEAD (&blkcg->policy_list);return &blkcg->css; }
block/blk-cgroup.h +0 -32
@@ -112,7 +112,6 @@ struct blkio_cgroup {unsigned int weight;spinlock_t lock;struct hlist_head blkg_list; -struct list_head policy_list;/* list of blkio_policy_node */ };struct blkio_group_stats {
@@ -182,37 +181,6 @@ struct blkio_group {struct blkio_group_stats_cpu __percpu *stats_cpu; }; -struct blkio_policy_node { -struct list_head node; -dev_t dev; -/* This node belongs to max bw policy or porportional weight policy */ -enum blkio_policy_id plid; -/* cgroup file to which this rule belongs to */ -int fileid; - -union { -unsigned int weight; -/* -* Rate read/write in terms of bytes per second -* Whether this rate represents read or write is determined -* by file type "fileid". -*/ -u64 bps; -unsigned int iops; - } val; -}; - -extern unsigned int blkcg_get_weight (struct blkio_cgroup *blkcg, -dev_t dev); -extern uint64_t blkcg_get_read_bps (struct blkio_cgroup *blkcg, -dev_t dev); -extern uint64_t blkcg_get_write_bps (struct blkio_cgroup *blkcg, -dev_t dev); -extern unsigned int blkcg_get_read_iops (struct blkio_cgroup *blkcg, -dev_t dev); -extern unsigned int blkcg_get_write_iops (struct blkio_cgroup *blkcg, -dev_t dev); -typedef struct blkio_group *(blkio_alloc_group_fn )(struct request_queue *q,struct blkio_cgroup *blkcg); typedef void (blkio_link_group_fn)(struct request_queue *q,
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 16/16 blkcg: kill the mind-bending blkg->dev
In reply to Tejun HeoFrom: Tejun Heo <[email protected]>
blkg->dev is dev_t recording the device number of the block device for the associated request_queue. It is used to identify the associated block device when printing out configuration or stats.
This is redundant to begin with. A blkg is an association between a cgroup and a request_queue and it of course is possible to reach request_queue from blkg and synchronization conventions are in place for safe q dereferencing, so this shouldn't be necessary from the beginning. Furthermore, it's initialized by sscanf()ing the device name of backing_dev_info. The mind boggles.
Anyways, if blkg is visible under rcu lock, we *know* that the associated request_queue hasn't gone away yet and its bdi is registered and alive - blkg can't be created for request_queue which hasn't been fully initialized and it can't go away before blkg is removed.
Let stat and conf read functions get device name from blkg->q->backing_dev_info.dev and pass it down to printing functions and remove blkg->dev.
--- block/blk-cgroup.c | 86 ++++++++++++++++++++++++++------------------------ block/blk-cgroup.h | 2 - block/blk-throttle.c | 51 +---------------------------- block/cfq-iosched.c | 21 ------------ 4 files changed, 47 insertions(+), 113 deletions(-)
block/blk-cgroup.c +45 -41
@@ -627,10 +627,10 @@ blkiocg_reset_stats(struct cgroup *cgroup, struct cftype *cftype, u64 val)return 0 ; } -static void blkio_get_key_name (enum stat_sub_type type,dev_t dev,char *str, -int chars_left,bool diskname_only) +static void blkio_get_key_name (enum stat_sub_type type,const char *dname, +char *str,int chars_left,bool diskname_only) { -snprintf (str, chars_left,"%d:%d" ,MAJOR (dev),MINOR (dev)); +snprintf (str, chars_left,"%s" , dname); chars_left -=strlen (str);if (chars_left <=0 ) { printk(KERN_WARNING
@@ -661,9 +661,9 @@ static void blkio_get_key_name(enum stat_sub_type type, dev_t dev, char *str, }static uint64_t blkio_fill_stat (char *str,int chars_left,uint64_t val, -struct cgroup_map_cb *cb,dev_t dev) +struct cgroup_map_cb *cb,const char *dname) { -blkio_get_key_name (0 , dev, str, chars_left, true); +blkio_get_key_name (0 , dname, str, chars_left, true); cb->fill (cb, str, val);return val; }
@@ -695,7 +695,8 @@ static uint64_t blkio_read_stat_cpu(struct blkio_group *blkg, }static uint64_t blkio_get_stat_cpu (struct blkio_group *blkg, -struct cgroup_map_cb *cb,dev_t dev,enum stat_type_cpu type) +struct cgroup_map_cb *cb,const char *dname, +enum stat_type_cpu type) {uint64_t disk_total, val;char key_str[MAX_KEY_LEN ];
@@ -703,12 +704,14 @@ static uint64_t blkio_get_stat_cpu(struct blkio_group *blkg,if (type ==BLKIO_STAT_CPU_SECTORS ) { val =blkio_read_stat_cpu (blkg, type,0 ); -return blkio_fill_stat (key_str,MAX_KEY_LEN -1 , val, cb, dev); +return blkio_fill_stat (key_str,MAX_KEY_LEN -1 , val, cb, + dname); }for (sub_type =BLKIO_STAT_READ ; sub_type <BLKIO_STAT_TOTAL ; sub_type++) { -blkio_get_key_name (sub_type, dev, key_str,MAX_KEY_LEN , false); +blkio_get_key_name (sub_type, dname, key_str,MAX_KEY_LEN , + false); val =blkio_read_stat_cpu (blkg, type, sub_type); cb->fill (cb, key_str, val); }
@@ -716,14 +719,16 @@ static uint64_t blkio_get_stat_cpu(struct blkio_group *blkg, disk_total =blkio_read_stat_cpu (blkg, type,BLKIO_STAT_READ ) +blkio_read_stat_cpu (blkg, type,BLKIO_STAT_WRITE ); -blkio_get_key_name (BLKIO_STAT_TOTAL , dev, key_str,MAX_KEY_LEN , false); +blkio_get_key_name (BLKIO_STAT_TOTAL , dname, key_str,MAX_KEY_LEN , + false); cb->fill (cb, key_str, disk_total);return disk_total; }/* This should be called with blkg->stats_lock held */ static uint64_t blkio_get_stat (struct blkio_group *blkg, -struct cgroup_map_cb *cb,dev_t dev,enum stat_type type) +struct cgroup_map_cb *cb,const char *dname, +enum stat_type type) {uint64_t disk_total;char key_str[MAX_KEY_LEN ];
@@ -731,11 +736,11 @@ static uint64_t blkio_get_stat(struct blkio_group *blkg,if (type ==BLKIO_STAT_TIME )return blkio_fill_stat (key_str,MAX_KEY_LEN -1 , - blkg->stats.time, cb, dev); + blkg->stats.time, cb, dname);#ifdef CONFIG_DEBUG_BLK_CGROUP if (type ==BLKIO_STAT_UNACCOUNTED_TIME )return blkio_fill_stat (key_str,MAX_KEY_LEN -1 , - blkg->stats.unaccounted_time, cb, dev); + blkg->stats.unaccounted_time, cb, dname);if (type ==BLKIO_STAT_AVG_QUEUE_SIZE ) {uint64_t sum = blkg->stats.avg_queue_size_sum;uint64_t samples = blkg->stats.avg_queue_size_samples;
@@ -743,30 +748,33 @@ static uint64_t blkio_get_stat(struct blkio_group *blkg,do_div (sum, samples); else sum =0 ; -return blkio_fill_stat (key_str,MAX_KEY_LEN -1 , sum, cb, dev); +return blkio_fill_stat (key_str,MAX_KEY_LEN -1 , + sum, cb, dname); }if (type ==BLKIO_STAT_GROUP_WAIT_TIME )return blkio_fill_stat (key_str,MAX_KEY_LEN -1 , - blkg->stats.group_wait_time, cb, dev); + blkg->stats.group_wait_time, cb, dname);if (type ==BLKIO_STAT_IDLE_TIME )return blkio_fill_stat (key_str,MAX_KEY_LEN -1 , - blkg->stats.idle_time, cb, dev); + blkg->stats.idle_time, cb, dname);if (type ==BLKIO_STAT_EMPTY_TIME )return blkio_fill_stat (key_str,MAX_KEY_LEN -1 , - blkg->stats.empty_time, cb, dev); + blkg->stats.empty_time, cb, dname);if (type ==BLKIO_STAT_DEQUEUE )return blkio_fill_stat (key_str,MAX_KEY_LEN -1 , - blkg->stats.dequeue, cb, dev); + blkg->stats.dequeue, cb, dname);#endif for (sub_type =BLKIO_STAT_READ ; sub_type <BLKIO_STAT_TOTAL ; sub_type++) { -blkio_get_key_name (sub_type, dev, key_str,MAX_KEY_LEN , false); +blkio_get_key_name (sub_type, dname, key_str,MAX_KEY_LEN , + false); cb->fill (cb, key_str, blkg->stats.stat_arr[type][sub_type]); } disk_total = blkg->stats.stat_arr[type][BLKIO_STAT_READ ] + blkg->stats.stat_arr[type][BLKIO_STAT_WRITE ]; -blkio_get_key_name (BLKIO_STAT_TOTAL , dev, key_str,MAX_KEY_LEN , false); +blkio_get_key_name (BLKIO_STAT_TOTAL , dname, key_str,MAX_KEY_LEN , + false); cb->fill (cb, key_str, disk_total);return disk_total; }
@@ -898,14 +906,15 @@ static int blkiocg_file_write(struct cgroup *cgrp, struct cftype *cft,static void blkio_print_group_conf (struct cftype *cft,struct blkio_group *blkg,struct seq_file *m) { +const char *dname =dev_name (blkg->q->backing_dev_info.dev);int fileid =BLKIOFILE_ATTR (cft->private);int rw =WRITE ;switch (blkg->plid) {case BLKIO_POLICY_PROP :if (blkg->conf.weight) -seq_printf (m,"%u:%u\t%u\n" ,MAJOR (blkg->dev), -MINOR (blkg->dev), blkg->conf.weight); +seq_printf (m,"%s\t%u\n" , + dname, blkg->conf.weight);break ;case BLKIO_POLICY_THROTL :switch (fileid) {
@@ -913,19 +922,15 @@ static void blkio_print_group_conf(struct cftype *cft, struct blkio_group *blkg, rw =READ ;case BLKIO_THROTL_write_bps_device:if (blkg->conf.bps[rw]) -seq_printf (m,"%u:%u\t%llu\n" , -MAJOR (blkg->dev), -MINOR (blkg->dev), - blkg->conf.bps[rw]); +seq_printf (m,"%s\t%llu\n" , + dname, blkg->conf.bps[rw]);break ;case BLKIO_THROTL_read_iops_device: rw =READ ;case BLKIO_THROTL_write_iops_device:if (blkg->conf.iops[rw]) -seq_printf (m,"%u:%u\t%u\n" , -MAJOR (blkg->dev), -MINOR (blkg->dev), - blkg->conf.iops[rw]); +seq_printf (m,"%s\t%u\n" , + dname, blkg->conf.iops[rw]);break ; }break ;
@@ -996,18 +1001,17 @@ static int blkio_read_blkg_stats(struct blkio_cgroup *blkcg,rcu_read_lock ();hlist_for_each_entry_rcu (blkg, n, &blkcg->blkg_list, blkcg_node) { -if (blkg->dev) { -if (BLKIOFILE_POLICY (cft->private) != blkg->plid) -continue ; -if (pcpu) - cgroup_total +=blkio_get_stat_cpu (blkg, cb, - blkg->dev, type); -else { -spin_lock_irq (&blkg->stats_lock); - cgroup_total +=blkio_get_stat (blkg, cb, - blkg->dev, type); -spin_unlock_irq (&blkg->stats_lock); - } +const char *dname =dev_name (blkg->q->backing_dev_info.dev); + +if (BLKIOFILE_POLICY (cft->private) != blkg->plid) +continue ; +if (pcpu) + cgroup_total +=blkio_get_stat_cpu (blkg, cb, dname, + type); +else { +spin_lock_irq (&blkg->stats_lock); + cgroup_total +=blkio_get_stat (blkg, cb, dname, type); +spin_unlock_irq (&blkg->stats_lock); } }if (show_total)
block/blk-cgroup.h +0 -2
@@ -166,8 +166,6 @@ struct blkio_group {unsigned short blkcg_id;/* Store cgroup path */ char path[128 ]; -/* The device MKDEV(major, minor), this group has been created for */ -dev_t dev;/* policy which owns this blk group */ enum blkio_policy_id plid;
block/blk-throttle.c +2 -49
87 lines not on this page · raw message
block/cfq-iosched.c +0 -21
@@ -1052,20 +1052,7 @@ static void cfq_link_blkio_group(struct request_queue *q, struct blkio_group *blkg) {struct cfq_data *cfqd = q->elevator->elevator_data; -struct backing_dev_info *bdi = &q->backing_dev_info;struct cfq_group *cfqg =cfqg_of_blkg (blkg); -unsigned int major, minor; - -/* -* Add group onto cgroup list. It might happen that bdi->dev is -* not initialized yet. Initialize this new group without major -* and minor info and this info will be filled in once a new thread -* comes for IO. -*/ -if (bdi->dev) { -sscanf (dev_name (bdi->dev),"%u:%u" , &major, &minor); - blkg->dev =MKDEV (major, minor); - } cfqd->nr_blkcg_linked_grps++;
@@ -2808,7 +2795,6 @@ static struct cfq_queue * cfq_find_alloc_queue(struct cfq_data *cfqd, bool is_sync, struct io_context *ioc, gfp_t gfp_mask) { - struct backing_dev_info *bdi = &cfqd->queue->backing_dev_info; struct blkio_cgroup *blkcg;struct cfq_queue *cfqq, *new_cfqq =NULL ;struct cfq_io_cq *cic;
@@ -2825,13 +2811,6 @@ retry:else cfqg =cfq_lookup_create_cfqg (cfqd, blkcg); -if (cfqg && !cfqg->blkg.dev && bdi->dev &&dev_name (bdi->dev)) { -unsigned int major, minor; - -sscanf (dev_name (bdi->dev),"%u:%u" , &major, &minor); - cfqg->blkg.dev =MKDEV (major, minor); - } - cic =cfq_cic_lookup (cfqd, ioc);/* cic always exists here */ cfqq =cic_to_cfqq (cic, is_sync);
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 1/1 blkcg: make blkg_lookup_create() return ERR_PTR value on failure
In reply to Tejun HeoUpdate blkg_lookup_create() so that it indicates the cause of failure with ERR_PTR value. This is primarily to allow the caller to determine whether the failure is transitional due to queue being bypassed temporarily.
--- block/blk-cgroup.c | 15 +++++++++------ block/blk-throttle.c | 2 +- block/cfq-iosched.c | 5 ++++- 3 files changed, 14 insertions(+), 8 deletions(-)
block/blk-cgroup.c +9 -6
@@ -471,8 +471,7 @@ struct blkio_group *blkg_lookup_create(s __releases(q->queue_lock) __acquires(q->queue_lock) {struct blkio_policy_type *pol = blkio_policy[plid]; -struct blkio_group *blkg =NULL ; -struct blkio_group *new_blkg =NULL ; +struct blkio_group *blkg, *new_blkg =NULL ;WARN_ON_ONCE (!rcu_read_lock_held ());lockdep_assert_held (q->queue_lock);
@@ -484,14 +483,14 @@ struct blkio_group *blkg_lookup_create(s* fail on a bypassing queue. */ if (unlikely (blk_queue_bypass (q))) -return NULL ; +return ERR_PTR (blk_queue_dead (q) ? -EINVAL : -EBUSY ); blkg =blkg_lookup (blkcg, q, plid);if (blkg)return blkg;if (!css_tryget (&blkcg->css)) -return NULL ; +return ERR_PTR (-EINVAL );/* * Allocate and initialize.
@@ -522,8 +521,10 @@ struct blkio_group *blkg_lookup_create(scss_put (&blkcg->css);/* did bypass get turned on inbetween? */ -if (unlikely (blk_queue_bypass (q))) +if (unlikely (blk_queue_bypass (q))) { + blkg =ERR_PTR (blk_queue_dead (q) ? -EINVAL : -EBUSY ); goto out; + }/* did someone beat us to it? */ blkg =blkg_lookup (blkcg, q, plid);
@@ -531,8 +532,10 @@ struct blkio_group *blkg_lookup_create(s goto out;/* did alloc fail? */ -if (unlikely (!new_blkg || !new_blkg->stats_cpu)) +if (unlikely (!new_blkg || !new_blkg->stats_cpu)) { + blkg =ERR_PTR (-ENOMEM ); goto out; + }/* insert */ spin_lock (&blkcg->lock);
block/blk-throttle.c +1 -1
@@ -289,7 +289,7 @@ static struct throtl_grp *throtl_lookup_ blkg =blkg_lookup_create (blkcg, q,BLKIO_POLICY_THROTL );/* if %NULL and @q is alive, fall back to root_tg */ -if (blkg) +if (!IS_ERR (blkg)) tg =tg_of_blkg (blkg);else if (!blk_queue_dead (q)) tg = td->root_tg;
block/cfq-iosched.c +4 -1
@@ -1106,7 +1106,10 @@ static struct cfq_group *cfq_lookup_creastruct blkio_group *blkg; blkg =blkg_lookup_create (blkcg, cfqd->queue,BLKIO_POLICY_PROP ); -return cfqg_of_blkg (blkg); +if (!IS_ERR (blkg)) +return cfqg_of_blkg (blkg); +else +return NULL ; }static inline struct cfq_group *cfq_ref_get_cfqg (struct cfq_group *cfqg)