Skip to contentSkip to conversation

blkcg: implement interface for the unified hierarchy

PATCH In review Tejun Heo 3 participants 21 messages Jul 24, 2015 – Aug 18, 2015

Subsystems (6)
Memory management Documentation Block layer cgroup +2 more subsystems
lore
Share & tools
lore

Tejun Heo

author maintainer
#0 lore raw

PATCH 1/1 blkcg: implement interface for the unified hierarchy

Hello,

blkcg interface grew to be the biggest of all controllers and unfortunately most inconsistent too. The interface files are inconsistent with a number of cloes duplicates. Some files have recursive variants while others don't. There's distinction between normal and leaf weights which isn't intuitive and there are a lot of stat knobs which don't make much sense outside of debugging and expose too much implementation details to userland.

In the unified hierarchy, everything is always hierarchical and internal nodes can't have tasks rendering the two structural issues twisting the current interface. The interface has to be updated in a significant anyway and this is a good chance to revamp it as a whole. This patchset implements blkcg interface for the unified hierarchy.

* blkcg is identified by "io" instead of "blkio" on the unified hierarchy. Given that the whole interface is updated anyway, the rename shouldn't carry noticeable conversion overhead.

* The original interface consisted of 27 files is replaced with the following three files.

blkio.stat : per-blkcg stats
blkio.weight : per-cgroup and per-cgroup-queue weight settings
blkio.max : per-cgroup-queue bps and iops max limits

For more details, please refer to
Documentation/cgroups/unified-hierarchy.txt.

This patchset contains the following 10 patches.

0001-cgroup-don-t-print-subsystems-for-the-default-hierar.patch 0002-cgroup-introduce-cgroup_subsys-legacy_name.patch 0003-blkcg-remove-unnecessary-NULL-checks-from-__cfqg_set.patch 0004-blkcg-refine-error-codes-returned-during-blkcg-confi.patch 0005-blkcg-rename-subsystem-name-from-blkio-to-io.patch 0006-blkcg-mark-existing-cftypes-as-legacy.patch 0007-blkcg-move-body-parsing-from-blkg_conf_prep-to-its-c.patch 0008-blkcg-separate-out-tg_conf_updated-from-tg_set_conf.patch 0009-blkcg-misc-preparations-for-unified-hierarchy-interf.patch 0010-blkcg-implement-interface-for-the-unified-hierarchy.patch

0001-0002 are cgroup prep patches. 0003-0004 are misc prep patches. 0005 renames blkio to io on the unified hierarchy. 0006-0010 implement the new interface.

This patchset is also available in the following git branch.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-blkcg-unified-hier

and is on top of

block/for-linus f3f5da624e0a ("block: Do a full clone when splitting discard bios")
+ [1] [PATCHSET block/for-4.3] writeback: cgroup writeback updates
+ [2] [PATCHSET v2 block/for-4.3] block, cgroup: make cfq charge async IOs to the appropriate blkcgs
+ [3] [PATCHSET v3 block/for-4.3] blkcg: blkcg policy methods and data handling cleanup
+ [4] [PATCHSET v2 block/for-4.3] blkcg: blkcg stats cleanup

diffstat follows. Thanks.

 Documentation/cgroups/unified-hierarchy.txt |   57 ++++++++-
 block/bio.c                                 |    2 
 block/blk-cgroup.c                          |  105 +++++++++++++---
 block/blk-throttle.c                        |  176 +++++++++++++++++++++++-----
 block/cfq-iosched.c                         |   91 +++++++++++---
 include/linux/backing-dev.h                 |    2 
 include/linux/blk-cgroup.h                  |   12 +
 include/linux/cgroup-defs.h                 |    3 
 include/linux/cgroup_subsys.h               |    2 
 kernel/cgroup.c                             |   41 ++++--
 mm/backing-dev.c                            |    4 
 11 files changed, 399 insertions(+), 96 deletions(-)

--
tejun

[L] http://lkml.kernel.org/g/1435268337-1738-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
[1] http://lkml.kernel.org/g/1436281823-1947-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
[2] http://lkml.kernel.org/g/1436283361-3889-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
[3] http://lkml.kernel.org/g/1436637654-28110-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
[4] http://lkml.kernel.org/g/1436724043-12986-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

author maintainer
#1 lore raw

PATCH 1/10 cgroup: don't print subsystems for the default hierarchy

It doesn't make sense to print subsystems on mount option or /proc/PID/cgroup for the default hierarchy.

* cgroup.controllers file at the root of the default hierarchy lists the currently attached controllers.

* The default hierarchy is catch-all for unmounted subsystems.

* The default hierarchy doesn't accept any mount options.

Suppress subsystem printing on mount options and /proc/PID/cgroup for the default hierarchy.

Signed-off-by Tejun Heo <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc [email protected]
---
 kernel/cgroup.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
kernel/cgroup.c +9 -6
index f89d929..6c85e6d 100644
@@ -1332,9 +1332,10 @@ static int cgroup_show_options(struct seq_file *seq,
 	struct cgroup_subsys *ss;
 	int ssid;
 
-	for_each_subsys(ss, ssid)
-		if (root->subsys_mask & (1 << ssid))
-			seq_printf(seq, ",%s", ss->name);
+	if (root != &cgrp_dfl_root)
+		for_each_subsys(ss, ssid)
+			if (root->subsys_mask & (1 << ssid))
+				seq_printf(seq, ",%s", ss->name);
 	if (root->flags & CGRP_ROOT_NOPREFIX)
 		seq_puts(seq, ",noprefix");
 	if (root->flags & CGRP_ROOT_XATTR)

@@ -5136,9 +5137,11 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 			continue;
 
 		seq_printf(m, "%d:", root->hierarchy_id);
-		for_each_subsys(ss, ssid)
-			if (root->subsys_mask & (1 << ssid))
-				seq_printf(m, "%s%s", count++ ? "," : "", ss->name);
+		if (root != &cgrp_dfl_root)
+			for_each_subsys(ss, ssid)
+				if (root->subsys_mask & (1 << ssid))
+					seq_printf(m, "%s%s", count++ ? "," : "",
+						   ss->name);
 		if (strlen(root->name))
 			seq_printf(m, "%sname=%s", count ? "," : "",
 				   root->name);
Signature · 1 line
2.4.3
#2 lore raw

Re: [PATCH 01/10] cgroup: don't print subsystems for the default hierarchy

Quoted 10 lines · Tejun Heo

It doesn't make sense to print subsystems on mount option or /proc/PID/cgroup for the default hierarchy.

* cgroup.controllers file at the root of the default hierarchy lists the currently attached controllers.

* The default hierarchy is catch-all for unmounted subsystems.

* The default hierarchy doesn't accept any mount options.

Quoted 7 lines

Suppress subsystem printing on mount options and /proc/PID/cgroup for the default hierarchy.

Signed-off-by Tejun Heo <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc [email protected]

Acked

author maintainer Applied
#3 lore raw

Re: [PATCH 01/10] cgroup: don't print subsystems for the default hierarchy

In reply to Tejun Heo
Quoted 10 lines · Tejun Heo

It doesn't make sense to print subsystems on mount option or /proc/PID/cgroup for the default hierarchy.

* cgroup.controllers file at the root of the default hierarchy lists the currently attached controllers.

* The default hierarchy is catch-all for unmounted subsystems.

* The default hierarchy doesn't accept any mount options.

Quoted 7 lines

Suppress subsystem printing on mount options and /proc/PID/cgroup for the default hierarchy.

Signed-off-by Tejun Heo <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc [email protected]

Applied to cgroup/for-4.3-unified-base.

Thanks.

Signature · 1 line
tejun
author maintainer
#4 lore raw

PATCH 2/10 cgroup: introduce cgroup_subsys->legacy_name

In reply to Tejun Heo

This allows cgroup subsystems to use a different name on the unified hierarchy. cgroup_subsys->name is used on the unified hierarchy, ->legacy_name elsewhere. If ->legacy_name is not explicitly set, it's automatically set to ->name and the userland visible behavior remains unchanged.

Signed-off-by Tejun Heo <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc [email protected]
---
 include/linux/cgroup-defs.h |  3 +++
 kernel/cgroup.c             | 30 +++++++++++++++++++-----------
 2 files changed, 22 insertions(+), 11 deletions(-)
include/linux/cgroup-defs.h +3 -0
index 8f5770a..7d0bb53 100644
@@ -434,6 +434,9 @@ struct cgroup_subsys {
 	int id;
 	const char *name;
 
+	/* optional, initialized automatically during boot if not set */
+	const char *legacy_name;
+
 	/* link to parent, protected by cgroup_lock() */
 	struct cgroup_root *root;
kernel/cgroup.c +19 -11
index 6c85e6d..0da2efa 100644
@@ -1027,10 +1027,13 @@ static const struct file_operations proc_cgroupstats_operations;
 static char *cgroup_file_name(struct cgroup *cgrp, const struct cftype *cft,
 			      char *buf)
 {
+	struct cgroup_subsys *ss = cft->ss;
+
 	if (cft->ss && !(cft->flags & CFTYPE_NO_PREFIX) &&
 	    !(cgrp->root->flags & CGRP_ROOT_NOPREFIX))
 		snprintf(buf, CGROUP_FILE_NAME_MAX, "%s.%s",
-			 cft->ss->name, cft->name);
+			 cgroup_on_dfl(cgrp) ? ss->name : ss->legacy_name,
+			 cft->name);
 	else
 		strncpy(buf, cft->name, CGROUP_FILE_NAME_MAX);
 	return buf;

@@ -1335,7 +1338,7 @@ static int cgroup_show_options(struct seq_file *seq,
 	if (root != &cgrp_dfl_root)
 		for_each_subsys(ss, ssid)
 			if (root->subsys_mask & (1 << ssid))
-				seq_printf(seq, ",%s", ss->name);
+				seq_printf(seq, ",%s", ss->legacy_name);
 	if (root->flags & CGRP_ROOT_NOPREFIX)
 		seq_puts(seq, ",noprefix");
 	if (root->flags & CGRP_ROOT_XATTR)

@@ -1448,7 +1451,8 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 		}
 
 		for_each_subsys(ss, i) {
-			if (strcmp(token, ss->name))
+			if (strcmp(token, ss->name) &&
+			    strcmp(token, ss->legacy_name))
 				continue;
 			if (ss->disabled)
 				continue;

@@ -4994,6 +4998,8 @@ int __init cgroup_init_early(void)
 
 		ss->id = i;
 		ss->name = cgroup_subsys_name[i];
+		if (!ss->legacy_name)
+			ss->legacy_name = cgroup_subsys_name[i];
 
 		if (ss->early_init)
 			cgroup_init_subsys(ss, true);

@@ -5141,7 +5147,7 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 			for_each_subsys(ss, ssid)
 				if (root->subsys_mask & (1 << ssid))
 					seq_printf(m, "%s%s", count++ ? "," : "",
-						   ss->name);
+						   ss->legacy_name);
 		if (strlen(root->name))
 			seq_printf(m, "%sname=%s", count ? "," : "",
 				   root->name);

@@ -5181,7 +5187,7 @@ static int proc_cgroupstats_show(struct seq_file *m, void *v)
 
 	for_each_subsys(ss, i)
 		seq_printf(m, "%s\t%d\t%d\t%d\n",
-			   ss->name, ss->root->hierarchy_id,
+			   ss->legacy_name, ss->root->hierarchy_id,
 			   atomic_read(&ss->root->nr_cgrps), !ss->disabled);
 
 	mutex_unlock(&cgroup_mutex);

@@ -5403,12 +5409,14 @@ static int __init cgroup_disable(char *str)
 			continue;
 
 		for_each_subsys(ss, i) {
-			if (!strcmp(token, ss->name)) {
-				ss->disabled = 1;
-				printk(KERN_INFO "Disabling %s control group"
-					" subsystem\n", ss->name);
-				break;
-			}
+			if (strcmp(token, ss->name) &&
+			    strcmp(token, ss->legacy_name))
+				continue;
+
+			ss->disabled = 1;
+			printk(KERN_INFO "Disabling %s control group subsystem\n",
+			       ss->name);
+			break;
 		}
 	}
 	return 1;
Signature · 1 line
2.4.3
#5 lore raw

Re: [PATCH 02/10] cgroup: introduce cgroup_subsys->legacy_name

kernel/cgroup.c static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts) · line 1456

Quoted 8 lines
kernel/cgroup.c +2 -1
@@ -1448,7 +1451,8 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 		}
 
 		for_each_subsys(ss, i) {
-			if (strcmp(token, ss->name))
+			if (strcmp(token, ss->name) &&
+			    strcmp(token, ss->legacy_name))
 				continue;

As mounting with specified subsystems is only allowed in legacy hierarchy, I think we should allow using leagcy name only?

Quoted 2 lines
kernel/cgroup.c +0 -0
 			if (ss->disabled)
 				continue;
author maintainer
#6 lore raw

Re: [PATCH 02/10] cgroup: introduce cgroup_subsys->legacy_name

Quoted 11 lines · Zefan Li

kernel/cgroup.c static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts) · line 1456

Quoted 8 lines
kernel/cgroup.c +2 -1
@@ -1448,7 +1451,8 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 		}
 
 		for_each_subsys(ss, i) {
-			if (strcmp(token, ss->name))
+			if (strcmp(token, ss->name) &&
+			    strcmp(token, ss->legacy_name))
 				continue;

As mounting with specified subsystems is only allowed in legacy hierarchy, I think we should allow using leagcy name only?

Yeah, good point. Will update the patch.

Thanks.

Signature · 1 line
tejun
author maintainer
#7 lore raw

PATCH v2 2/10 cgroup: introduce cgroup_subsys->legacy_name

In reply to Tejun Heo

This allows cgroup subsystems to use a different name on the unified hierarchy. cgroup_subsys->name is used on the unified hierarchy, ->legacy_name elsewhere. If ->legacy_name is not explicitly set, it's automatically set to ->name and the userland visible behavior remains unchanged.

v2: Make parse_cgroupfs_options() only consider ->legacy_name as mount
options are used only on legacy hierarchies. Suggested by Li Zefan.

Signed-off-by Tejun Heo <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc [email protected]

---
git branch updated accordingly. Thanks.

 include/linux/cgroup-defs.h |  3 +++
 kernel/cgroup.c             | 29 ++++++++++++++++++-----------
 2 files changed, 21 insertions(+), 11 deletions(-)
include/linux/cgroup-defs.h +3 -0
index 8f5770a..7d0bb53 100644
@@ -434,6 +434,9 @@ struct cgroup_subsys {
 	int id;
 	const char *name;
 
+	/* optional, initialized automatically during boot if not set */
+	const char *legacy_name;
+
 	/* link to parent, protected by cgroup_lock() */
 	struct cgroup_root *root;
kernel/cgroup.c +18 -11
index 6c85e6d..1276569 100644
@@ -1027,10 +1027,13 @@ static const struct file_operations proc_cgroupstats_operations;
 static char *cgroup_file_name(struct cgroup *cgrp, const struct cftype *cft,
 			      char *buf)
 {
+	struct cgroup_subsys *ss = cft->ss;
+
 	if (cft->ss && !(cft->flags & CFTYPE_NO_PREFIX) &&
 	    !(cgrp->root->flags & CGRP_ROOT_NOPREFIX))
 		snprintf(buf, CGROUP_FILE_NAME_MAX, "%s.%s",
-			 cft->ss->name, cft->name);
+			 cgroup_on_dfl(cgrp) ? ss->name : ss->legacy_name,
+			 cft->name);
 	else
 		strncpy(buf, cft->name, CGROUP_FILE_NAME_MAX);
 	return buf;

@@ -1335,7 +1338,7 @@ static int cgroup_show_options(struct seq_file *seq,
 	if (root != &cgrp_dfl_root)
 		for_each_subsys(ss, ssid)
 			if (root->subsys_mask & (1 << ssid))
-				seq_printf(seq, ",%s", ss->name);
+				seq_printf(seq, ",%s", ss->legacy_name);
 	if (root->flags & CGRP_ROOT_NOPREFIX)
 		seq_puts(seq, ",noprefix");
 	if (root->flags & CGRP_ROOT_XATTR)

@@ -1448,7 +1451,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 		}
 
 		for_each_subsys(ss, i) {
-			if (strcmp(token, ss->name))
+			if (strcmp(token, ss->legacy_name))
 				continue;
 			if (ss->disabled)
 				continue;

@@ -4994,6 +4997,8 @@ int __init cgroup_init_early(void)
 
 		ss->id = i;
 		ss->name = cgroup_subsys_name[i];
+		if (!ss->legacy_name)
+			ss->legacy_name = cgroup_subsys_name[i];
 
 		if (ss->early_init)
 			cgroup_init_subsys(ss, true);

@@ -5141,7 +5146,7 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 			for_each_subsys(ss, ssid)
 				if (root->subsys_mask & (1 << ssid))
 					seq_printf(m, "%s%s", count++ ? "," : "",
-						   ss->name);
+						   ss->legacy_name);
 		if (strlen(root->name))
 			seq_printf(m, "%sname=%s", count ? "," : "",
 				   root->name);

@@ -5181,7 +5186,7 @@ static int proc_cgroupstats_show(struct seq_file *m, void *v)
 
 	for_each_subsys(ss, i)
 		seq_printf(m, "%s\t%d\t%d\t%d\n",
-			   ss->name, ss->root->hierarchy_id,
+			   ss->legacy_name, ss->root->hierarchy_id,
 			   atomic_read(&ss->root->nr_cgrps), !ss->disabled);
 
 	mutex_unlock(&cgroup_mutex);

@@ -5403,12 +5408,14 @@ static int __init cgroup_disable(char *str)
 			continue;
 
 		for_each_subsys(ss, i) {
-			if (!strcmp(token, ss->name)) {
-				ss->disabled = 1;
-				printk(KERN_INFO "Disabling %s control group"
-					" subsystem\n", ss->name);
-				break;
-			}
+			if (strcmp(token, ss->name) &&
+			    strcmp(token, ss->legacy_name))
+				continue;
+
+			ss->disabled = 1;
+			printk(KERN_INFO "Disabling %s control group subsystem\n",
+			       ss->name);
+			break;
 		}
 	}
 	return 1;
Signature · 1 line
2.4.3
author maintainer Applied
#8 lore raw

Re: [PATCH v2 02/10] cgroup: introduce cgroup_subsys->legacy_name

Quoted 6 lines · Tejun Heo

This allows cgroup subsystems to use a different name on the unified hierarchy. cgroup_subsys->name is used on the unified hierarchy, ->legacy_name elsewhere. If ->legacy_name is not explicitly set, it's automatically set to ->name and the userland visible behavior remains unchanged.

Quoted 8 lines

v2: Make parse_cgroupfs_options() only consider ->legacy_name as mount
options are used only on legacy hierarchies. Suggested by Li Zefan.

Signed-off-by Tejun Heo <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc [email protected]

Applied to cgroup/for-4.3-unified-base.

Thanks.

Signature · 1 line
tejun
#9 lore raw

Re: [PATCH 02/10] cgroup: introduce cgroup_subsys->legacy_name

In reply to Tejun Heo
Quoted 10 lines · Tejun Heo

This allows cgroup subsystems to use a different name on the unified hierarchy. cgroup_subsys->name is used on the unified hierarchy, ->legacy_name elsewhere. If ->legacy_name is not explicitly set, it's automatically set to ->name and the userland visible behavior remains unchanged.

Signed-off-by Tejun Heo <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc [email protected]

Acked

author maintainer
#10 lore raw

PATCH 3/10 blkcg: remove unnecessary NULL checks from __cfqg_set_weight_device()

In reply to Tejun Heo

blkg_to_cfqg() and blkcg_to_cfqgd() on a valid blkg with the policy enabled are guaranteed to return non-NULL and the counterpart in blk-throttle doesn't have these checks either. Remove the spurious NULL checks.

Signed-off-by Tejun Heo <[email protected]>
---
 block/cfq-iosched.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
block/cfq-iosched.c +1 -4
index 395476a..bcf4026 100644
@@ -1752,12 +1752,10 @@ static ssize_t __cfqg_set_weight_device(struct kernfs_open_file *of,
 	if (ret)
 		return ret;
 
-	ret = -EINVAL;
 	cfqg = blkg_to_cfqg(ctx.blkg);
 	cfqgd = blkcg_to_cfqgd(blkcg);
-	if (!cfqg || !cfqgd)
-		goto err;
 
+	ret = -EINVAL;
 	if (!ctx.v || (ctx.v >= CFQ_WEIGHT_MIN && ctx.v <= CFQ_WEIGHT_MAX)) {
 		if (!is_leaf_weight) {
 			cfqg->dev_weight = ctx.v;

@@ -1769,7 +1767,6 @@ static ssize_t __cfqg_set_weight_device(struct kernfs_open_file *of,
 		ret = 0;
 	}
 
-err:
 	blkg_conf_finish(&ctx);
 	return ret ?: nbytes;
 }
Signature · 1 line
2.4.3
author maintainer
#11 lore raw

PATCH 4/10 blkcg: refine error codes returned during blkcg configuration

In reply to Tejun Heo

blkcg currently returns -EINVAL for most errors which can be pretty confusing given that the failure modes are quite varied. Update the error returns so that

* -EINVAL only for syntactic errors.
* -ERANGE if the value is out of range.
* -ENODEV if the target device can't be found.
* -EOPNOTSUPP if the policy is not enabled on the target device.

Signed-off-by Tejun Heo <[email protected]>
---
 block/blk-cgroup.c  | 12 ++++++------
 block/cfq-iosched.c |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)
block/blk-cgroup.c +6 -6
index 63c0914..a192f98 100644
@@ -179,7 +179,7 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg,
 
 	/* blkg holds a reference to blkcg */
 	if (!css_tryget_online(&blkcg->css)) {
-		ret = -EINVAL;
+		ret = -ENODEV;
 		goto err_free_blkg;
 	}
 

@@ -205,7 +205,7 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg,
 	if (blkcg_parent(blkcg)) {
 		blkg->parent = __blkg_lookup(blkcg_parent(blkcg), q, false);
 		if (WARN_ON_ONCE(!blkg->parent)) {
-			ret = -EINVAL;
+			ret = -ENODEV;
 			goto err_put_congested;
 		}
 		blkg_get(blkg->parent);

@@ -279,7 +279,7 @@ struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
 	 * we shouldn't allow anything to go through for a bypassing queue.
 	 */
 	if (unlikely(blk_queue_bypass(q)))
-		return ERR_PTR(blk_queue_dying(q) ? -EINVAL : -EBUSY);
+		return ERR_PTR(blk_queue_dying(q) ? -ENODEV : -EBUSY);
 
 	blkg = __blkg_lookup(blkcg, q, true);
 	if (blkg)

@@ -792,10 +792,10 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
 
 	disk = get_gendisk(MKDEV(major, minor), &part);
 	if (!disk)
-		return -EINVAL;
+		return -ENODEV;
 	if (part) {
 		put_disk(disk);
-		return -EINVAL;
+		return -ENODEV;
 	}
 
 	rcu_read_lock();

@@ -804,7 +804,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
 	if (blkcg_policy_enabled(disk->queue, pol))
 		blkg = blkg_lookup_create(blkcg, disk->queue);
 	else
-		blkg = ERR_PTR(-EINVAL);
+		blkg = ERR_PTR(-EOPNOTSUPP);
 
 	if (IS_ERR(blkg)) {
 		ret = PTR_ERR(blkg);
block/cfq-iosched.c +1 -1
index bcf4026..38277e3 100644
@@ -1755,7 +1755,7 @@ static ssize_t __cfqg_set_weight_device(struct kernfs_open_file *of,
 	cfqg = blkg_to_cfqg(ctx.blkg);
 	cfqgd = blkcg_to_cfqgd(blkcg);
 
-	ret = -EINVAL;
+	ret = -ERANGE;
 	if (!ctx.v || (ctx.v >= CFQ_WEIGHT_MIN && ctx.v <= CFQ_WEIGHT_MAX)) {
 		if (!is_leaf_weight) {
 			cfqg->dev_weight = ctx.v;
Signature · 1 line
2.4.3
author maintainer
#12 lore raw

PATCH 5/10 blkcg: rename subsystem name from blkio to io

In reply to Tejun Heo

blkio interface has become messy over time and is currently the largest. In addition to the inconsistent naming scheme, it has multiple stat files which report more or less the same thing, a number of debug stat files which expose internal details which shouldn't have been part of the public interface in the first place, recursive and non-recursive stats and leaf and non-leaf knobs.

Both recursive vs. non-recursive and leaf vs. non-leaf distinctions don't make any sense on the unified hierarchy as only leaf cgroups can contain processes. cgroups is going through a major interface revision with the unified hierarchy involving significant fundamental usage changes and given that a significant portion of the interface doesn't make sense anymore, it's a good time to reorganize the interface.

As the first step, this patch renames the external visible subsystem name from "blkio" to "io". This is more concise, matches the other two major subsystem names, "cpu" and "memory", and better suited as blkcg will be involved in anything writeback related too whether an actual block device is involved or not.

As the subsystem legacy_name is set to "blkio", the only userland visible change outside the unified hierarchy is that blkcg is reported as "io" instead of "blkio" in the subsystem initialized message during boot. On the unified hierarchy, blkcg now appears as "io".

Signed-off-by Tejun Heo <[email protected]>
Cc Li Zefan <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc [email protected]
---
 block/bio.c                   | 2 +-
 block/blk-cgroup.c            | 7 ++++---
 include/linux/backing-dev.h   | 2 +-
 include/linux/blk-cgroup.h    | 4 ++--
 include/linux/cgroup_subsys.h | 2 +-
 mm/backing-dev.c              | 4 ++--
 6 files changed, 11 insertions(+), 10 deletions(-)
block/bio.c +1 -1
index d6e5ba3..c52222c 100644
@@ -2046,7 +2046,7 @@ int bio_associate_current(struct bio *bio)
 
 	get_io_context_active(ioc);
 	bio->bi_ioc = ioc;
-	bio->bi_css = task_get_css(current, blkio_cgrp_id);
+	bio->bi_css = task_get_css(current, io_cgrp_id);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(bio_associate_current);
block/blk-cgroup.c +4 -3
index a192f98..c2fb867 100644
@@ -1088,12 +1088,13 @@ static int blkcg_can_attach(struct cgroup_subsys_state *css,
 	return ret;
 }
 
-struct cgroup_subsys blkio_cgrp_subsys = {
+struct cgroup_subsys io_cgrp_subsys = {
 	.css_alloc = blkcg_css_alloc,
 	.css_offline = blkcg_css_offline,
 	.css_free = blkcg_css_free,
 	.can_attach = blkcg_can_attach,
 	.legacy_cftypes = blkcg_files,
+	.legacy_name = "blkio",
 #ifdef CONFIG_MEMCG
 	/*
 	 * This ensures that, if available, memcg is automatically enabled

@@ -1103,7 +1104,7 @@ struct cgroup_subsys blkio_cgrp_subsys = {
 	.depends_on = 1 << memory_cgrp_id,
 #endif
 };
-EXPORT_SYMBOL_GPL(blkio_cgrp_subsys);
+EXPORT_SYMBOL_GPL(io_cgrp_subsys);
 
 /**
  * blkcg_activate_policy - activate a blkcg policy on a request_queue

@@ -1264,7 +1265,7 @@ int blkcg_policy_register(struct blkcg_policy *pol)
 
 	/* everything is in place, add intf files for the new policy */
 	if (pol->cftypes)
-		WARN_ON(cgroup_add_legacy_cftypes(&blkio_cgrp_subsys,
+		WARN_ON(cgroup_add_legacy_cftypes(&io_cgrp_subsys,
 						  pol->cftypes));
 	mutex_unlock(&blkcg_pol_register_mutex);
 	return 0;
include/linux/backing-dev.h +1 -1
index 23ebb94..5a5d79e 100644
@@ -286,7 +286,7 @@ static inline struct bdi_writeback *wb_find_current(struct backing_dev_info *bdi
 	 * %current's blkcg equals the effective blkcg of its memcg.  No
 	 * need to use the relatively expensive cgroup_get_e_css().
 	 */
-	if (likely(wb && wb->blkcg_css == task_css(current, blkio_cgrp_id)))
+	if (likely(wb && wb->blkcg_css == task_css(current, io_cgrp_id)))
 		return wb;
 	return NULL;
 }
include/linux/blk-cgroup.h +2 -2
index 286e1bd..db89acd 100644
@@ -221,7 +221,7 @@ static inline struct blkcg *css_to_blkcg(struct cgroup_subsys_state *css)
 
 static inline struct blkcg *task_blkcg(struct task_struct *tsk)
 {
-	return css_to_blkcg(task_css(tsk, blkio_cgrp_id));
+	return css_to_blkcg(task_css(tsk, io_cgrp_id));
 }
 
 static inline struct blkcg *bio_blkcg(struct bio *bio)

@@ -234,7 +234,7 @@ static inline struct blkcg *bio_blkcg(struct bio *bio)
 static inline struct cgroup_subsys_state *
 task_get_blkcg_css(struct task_struct *task)
 {
-	return task_get_css(task, blkio_cgrp_id);
+	return task_get_css(task, io_cgrp_id);
 }
 
 /**
include/linux/cgroup_subsys.h +1 -1
index e4a96fb..86b5056 100644
@@ -16,7 +16,7 @@ SUBSYS(cpuacct)
 #endif
 
 #if IS_ENABLED(CONFIG_BLK_CGROUP)
-SUBSYS(blkio)
+SUBSYS(io)
 #endif
 
 #if IS_ENABLED(CONFIG_MEMCG)
mm/backing-dev.c +2 -2
index dac5bf5..d0ee90e 100644
@@ -523,7 +523,7 @@ static int cgwb_create(struct backing_dev_info *bdi,
 	int ret = 0;
 
 	memcg = mem_cgroup_from_css(memcg_css);
-	blkcg_css = cgroup_get_e_css(memcg_css->cgroup, &blkio_cgrp_subsys);
+	blkcg_css = cgroup_get_e_css(memcg_css->cgroup, &io_cgrp_subsys);
 	blkcg = css_to_blkcg(blkcg_css);
 	memcg_cgwb_list = mem_cgroup_cgwb_list(memcg);
 	blkcg_cgwb_list = &blkcg->cgwb_list;

@@ -645,7 +645,7 @@ struct bdi_writeback *wb_get_create(struct backing_dev_info *bdi,
 
 			/* see whether the blkcg association has changed */
 			blkcg_css = cgroup_get_e_css(memcg_css->cgroup,
-						     &blkio_cgrp_subsys);
+						     &io_cgrp_subsys);
 			if (unlikely(wb->blkcg_css != blkcg_css ||
 				     !wb_tryget(wb)))
 				wb = NULL;
Signature · 1 line
2.4.3
author maintainer
#13 lore raw

PATCH 6/10 blkcg: mark existing cftypes as legacy

In reply to Tejun Heo

blkcg is about to grow interface for the unified hierarchy. Add legacy to existing cftypes.

* blkcg_policy->cftypes -> blkcg_policy->legacy_cftypes
* blk-cgroup.c:blkcg_files -> blkcg_legacy_files
* cfq-iosched.c:cfq_blkcg_files -> cfq_blkcg_legacy_files
* blk-throttle.c:throtl_files -> throtl_legacy_files

Pure renames. No functional change.

Signed-off-by Tejun Heo <[email protected]>
---
 block/blk-cgroup.c         | 12 ++++++------
 block/blk-throttle.c       |  4 ++--
 block/cfq-iosched.c        |  4 ++--
 include/linux/blk-cgroup.h |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)
block/blk-cgroup.c +6 -6
index c2fb867..db7b3cc 100644
@@ -847,7 +847,7 @@ void blkg_conf_finish(struct blkg_conf_ctx *ctx)
 }
 EXPORT_SYMBOL_GPL(blkg_conf_finish);
 
-struct cftype blkcg_files[] = {
+struct cftype blkcg_legacy_files[] = {
 	{
 		.name = "reset_stats",
 		.write_u64 = blkcg_reset_stats,

@@ -1093,7 +1093,7 @@ struct cgroup_subsys io_cgrp_subsys = {
 	.css_offline = blkcg_css_offline,
 	.css_free = blkcg_css_free,
 	.can_attach = blkcg_can_attach,
-	.legacy_cftypes = blkcg_files,
+	.legacy_cftypes = blkcg_legacy_files,
 	.legacy_name = "blkio",
 #ifdef CONFIG_MEMCG
 	/*

@@ -1264,9 +1264,9 @@ int blkcg_policy_register(struct blkcg_policy *pol)
 	mutex_unlock(&blkcg_pol_mutex);
 
 	/* everything is in place, add intf files for the new policy */
-	if (pol->cftypes)
+	if (pol->legacy_cftypes)
 		WARN_ON(cgroup_add_legacy_cftypes(&io_cgrp_subsys,
-						  pol->cftypes));
+						  pol->legacy_cftypes));
 	mutex_unlock(&blkcg_pol_register_mutex);
 	return 0;
 

@@ -1303,8 +1303,8 @@ void blkcg_policy_unregister(struct blkcg_policy *pol)
 		goto out_unlock;
 
 	/* kill the intf files first */
-	if (pol->cftypes)
-		cgroup_rm_cftypes(pol->cftypes);
+	if (pol->legacy_cftypes)
+		cgroup_rm_cftypes(pol->legacy_cftypes);
 
 	/* remove cpds and unregister */
 	mutex_lock(&blkcg_pol_mutex);
block/blk-throttle.c +2 -2
index bd3e4b2..8b4f6b8 100644
@@ -1217,7 +1217,7 @@ static ssize_t tg_set_conf_uint(struct kernfs_open_file *of,
 	return tg_set_conf(of, buf, nbytes, off, false);
 }
 
-static struct cftype throtl_files[] = {
+static struct cftype throtl_legacy_files[] = {
 	{
 		.name = "throttle.read_bps_device",
 		.private = offsetof(struct throtl_grp, bps[READ]),

@@ -1263,7 +1263,7 @@ static void throtl_shutdown_wq(struct request_queue *q)
 }
 
 static struct blkcg_policy blkcg_policy_throtl = {
-	.cftypes		= throtl_files,
+	.legacy_cftypes		= throtl_legacy_files,
 
 	.pd_alloc_fn		= throtl_pd_alloc,
 	.pd_init_fn		= throtl_pd_init,
block/cfq-iosched.c +2 -2
index 38277e3..baa8459 100644
@@ -1944,7 +1944,7 @@ static int cfqg_print_avg_queue_size(struct seq_file *sf, void *v)
 }
 #endif	/* CONFIG_DEBUG_BLK_CGROUP */
 
-static struct cftype cfq_blkcg_files[] = {
+static struct cftype cfq_blkcg_legacy_files[] = {
 	/* on root, weight is mapped to leaf_weight */
 	{
 		.name = "weight_device",

@@ -4654,7 +4654,7 @@ static struct elevator_type iosched_cfq = {
 
 #ifdef CONFIG_CFQ_GROUP_IOSCHED
 static struct blkcg_policy blkcg_policy_cfq = {
-	.cftypes		= cfq_blkcg_files,
+	.legacy_cftypes		= cfq_blkcg_legacy_files,
 
 	.cpd_alloc_fn		= cfq_cpd_alloc,
 	.cpd_init_fn		= cfq_cpd_init,
include/linux/blk-cgroup.h +1 -1
index db89acd..6e016e6 100644
@@ -148,7 +148,7 @@ typedef void (blkcg_pol_reset_pd_stats_fn)(struct blkg_policy_data *pd);
 struct blkcg_policy {
 	int				plid;
 	/* cgroup files for the policy */
-	struct cftype			*cftypes;
+	struct cftype			*legacy_cftypes;
 
 	/* operations */
 	blkcg_pol_alloc_cpd_fn		*cpd_alloc_fn;
Signature · 1 line
2.4.3
author maintainer
#14 lore raw

PATCH 7/10 blkcg: move body parsing from blkg_conf_prep() to its callers

In reply to Tejun Heo

Currently, blkg_conf_prep() expects input to be of the following form

MAJ:MIN NUM

and reads the NUM part into blkg_conf_ctx->v. This is quite restrictive and gets in the way in implementing blkcg interface for the unified hierarchy. This patch updates blkg_conf_prep() so that it expects

MAJ:MIN BODY_STR

where BODY_STR is an arbitrary string. blkg_conf_ctx->v is replaced with ->body which is a char pointer pointing to the start of BODY_STR. Parsing of the body is moved to blkg_conf_prep()'s callers.

To allow using, for example, strsep() on blkg_conf_ctx->val, it is a non-const pointer and to accommodate that const is dropped from @input too.

This doesn't cause any behavior changes.

Signed-off-by Tejun Heo <[email protected]>
---
 block/blk-cgroup.c         | 22 ++++++++++++++--------
 block/blk-throttle.c       | 18 ++++++++++++------
 block/cfq-iosched.c        | 17 +++++++++++------
 include/linux/blk-cgroup.h |  4 ++--
 4 files changed, 39 insertions(+), 22 deletions(-)
block/blk-cgroup.c +14 -8
index db7b3cc..2ea3a2a 100644
@@ -24,6 +24,7 @@
 #include <linux/genhd.h>
 #include <linux/delay.h>
 #include <linux/atomic.h>
+#include <linux/ctype.h>
 #include <linux/blk-cgroup.h>
 #include "blk.h"
 

@@ -773,23 +774,28 @@ EXPORT_SYMBOL_GPL(blkg_rwstat_recursive_sum);
  * @ctx: blkg_conf_ctx to be filled
  *
  * Parse per-blkg config update from @input and initialize @ctx with the
- * result.  @ctx->blkg points to the blkg to be updated and @ctx->v the new
- * value.  This function returns with RCU read lock and queue lock held and
- * must be paired with blkg_conf_finish().
+ * result.  @ctx->blkg points to the blkg to be updated and @ctx->body the
+ * part of @input following MAJ:MIN.  This function returns with RCU read
+ * lock and queue lock held and must be paired with blkg_conf_finish().
  */
 int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
-		   const char *input, struct blkg_conf_ctx *ctx)
+		   char *input, struct blkg_conf_ctx *ctx)
 	__acquires(rcu) __acquires(disk->queue->queue_lock)
 {
 	struct gendisk *disk;
 	struct blkcg_gq *blkg;
 	unsigned int major, minor;
-	unsigned long long v;
-	int part, ret;
+	int key_len, part, ret;
+	char *body;
 
-	if (sscanf(input, "%u:%u %llu", &major, &minor, &v) != 3)
+	if (sscanf(input, "%u:%u%n", &major, &minor, &key_len) != 2)
 		return -EINVAL;
 
+	body = input + key_len;
+	if (!isspace(*body))
+		return -EINVAL;
+	body = skip_spaces(body);
+
 	disk = get_gendisk(MKDEV(major, minor), &part);
 	if (!disk)
 		return -ENODEV;

@@ -826,7 +832,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
 
 	ctx->disk = disk;
 	ctx->blkg = blkg;
-	ctx->v = v;
+	ctx->body = body;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(blkg_conf_prep);
block/blk-throttle.c +12 -6
index 8b4f6b8..0e17c8f 100644
@@ -1154,21 +1154,25 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of,
 	struct blkcg_gq *blkg;
 	struct cgroup_subsys_state *pos_css;
 	int ret;
+	u64 v;
 
 	ret = blkg_conf_prep(blkcg, &blkcg_policy_throtl, buf, &ctx);
 	if (ret)
 		return ret;
 
+	ret = -EINVAL;
+	if (sscanf(ctx.body, "%llu", &v) != 1)
+		goto out_finish;
+	if (!v)
+		v = -1;
+
 	tg = blkg_to_tg(ctx.blkg);
 	sq = &tg->service_queue;
 
-	if (!ctx.v)
-		ctx.v = -1;
-
 	if (is_u64)
-		*(u64 *)((void *)tg + of_cft(of)->private) = ctx.v;
+		*(u64 *)((void *)tg + of_cft(of)->private) = v;
 	else
-		*(unsigned int *)((void *)tg + of_cft(of)->private) = ctx.v;
+		*(unsigned int *)((void *)tg + of_cft(of)->private) = v;
 
 	throtl_log(&tg->service_queue,
 		   "limit change rbps=%llu wbps=%llu riops=%u wiops=%u",

@@ -1201,8 +1205,10 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of,
 		throtl_schedule_next_dispatch(sq->parent_sq, true);
 	}
 
+	ret = 0;
+out_finish:
 	blkg_conf_finish(&ctx);
-	return nbytes;
+	return ret ?: nbytes;
 }
 
 static ssize_t tg_set_conf_u64(struct kernfs_open_file *of,
block/cfq-iosched.c +11 -6
index baa8459..ea88d89 100644
@@ -1747,26 +1747,31 @@ static ssize_t __cfqg_set_weight_device(struct kernfs_open_file *of,
 	struct cfq_group *cfqg;
 	struct cfq_group_data *cfqgd;
 	int ret;
+	u64 v;
 
 	ret = blkg_conf_prep(blkcg, &blkcg_policy_cfq, buf, &ctx);
 	if (ret)
 		return ret;
 
+	ret = -EINVAL;
+	if (sscanf(ctx.body, "%llu", &v) != 1)
+		goto out_finish;
+
 	cfqg = blkg_to_cfqg(ctx.blkg);
 	cfqgd = blkcg_to_cfqgd(blkcg);
 
 	ret = -ERANGE;
-	if (!ctx.v || (ctx.v >= CFQ_WEIGHT_MIN && ctx.v <= CFQ_WEIGHT_MAX)) {
+	if (!v || (v >= CFQ_WEIGHT_MIN && v <= CFQ_WEIGHT_MAX)) {
 		if (!is_leaf_weight) {
-			cfqg->dev_weight = ctx.v;
-			cfqg->new_weight = ctx.v ?: cfqgd->weight;
+			cfqg->dev_weight = v;
+			cfqg->new_weight = v ?: cfqgd->weight;
 		} else {
-			cfqg->dev_leaf_weight = ctx.v;
-			cfqg->new_leaf_weight = ctx.v ?: cfqgd->leaf_weight;
+			cfqg->dev_leaf_weight = v;
+			cfqg->new_leaf_weight = v ?: cfqgd->leaf_weight;
 		}
 		ret = 0;
 	}
-
+out_finish:
 	blkg_conf_finish(&ctx);
 	return ret ?: nbytes;
 }
include/linux/blk-cgroup.h +2 -2
index 6e016e6..85a4d98 100644
@@ -206,11 +206,11 @@ struct blkg_rwstat blkg_rwstat_recursive_sum(struct blkcg_gq *blkg,
 struct blkg_conf_ctx {
 	struct gendisk			*disk;
 	struct blkcg_gq			*blkg;
-	u64				v;
+	char				*body;
 };
 
 int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
-		   const char *input, struct blkg_conf_ctx *ctx);
+		   char *input, struct blkg_conf_ctx *ctx);
 void blkg_conf_finish(struct blkg_conf_ctx *ctx);
Signature · 1 line
2.4.3
author maintainer
#15 lore raw

PATCH 8/10 blkcg: separate out tg_conf_updated() from tg_set_conf()

In reply to Tejun Heo

tg_set_conf() is largely consisted of parsing and setting the new config and the follow-up application and propagation. This patch separates out the latter part into tg_conf_updated(). This will be used to implement interface for the unified hierarchy.

Signed-off-by Tejun Heo <[email protected]>
---
 block/blk-throttle.c | 60 ++++++++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 28 deletions(-)
block/blk-throttle.c +32 -28
index 0e17c8f..a8bb2fd 100644
@@ -1144,35 +1144,11 @@ static int tg_print_conf_uint(struct seq_file *sf, void *v)
 	return 0;
 }
 
-static ssize_t tg_set_conf(struct kernfs_open_file *of,
-			   char *buf, size_t nbytes, loff_t off, bool is_u64)
+static void tg_conf_updated(struct throtl_grp *tg)
 {
-	struct blkcg *blkcg = css_to_blkcg(of_css(of));
-	struct blkg_conf_ctx ctx;
-	struct throtl_grp *tg;
-	struct throtl_service_queue *sq;
-	struct blkcg_gq *blkg;
+	struct throtl_service_queue *sq = &tg->service_queue;
 	struct cgroup_subsys_state *pos_css;
-	int ret;
-	u64 v;
-
-	ret = blkg_conf_prep(blkcg, &blkcg_policy_throtl, buf, &ctx);
-	if (ret)
-		return ret;
-
-	ret = -EINVAL;
-	if (sscanf(ctx.body, "%llu", &v) != 1)
-		goto out_finish;
-	if (!v)
-		v = -1;
-
-	tg = blkg_to_tg(ctx.blkg);
-	sq = &tg->service_queue;
-
-	if (is_u64)
-		*(u64 *)((void *)tg + of_cft(of)->private) = v;
-	else
-		*(unsigned int *)((void *)tg + of_cft(of)->private) = v;
+	struct blkcg_gq *blkg;
 
 	throtl_log(&tg->service_queue,
 		   "limit change rbps=%llu wbps=%llu riops=%u wiops=%u",

@@ -1186,7 +1162,7 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of,
 	 * restrictions in the whole hierarchy and allows them to bypass
 	 * blk-throttle.
 	 */
-	blkg_for_each_descendant_pre(blkg, pos_css, ctx.blkg)
+	blkg_for_each_descendant_pre(blkg, pos_css, tg_to_blkg(tg))
 		tg_update_has_rules(blkg_to_tg(blkg));
 
 	/*

@@ -1204,7 +1180,35 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of,
 		tg_update_disptime(tg);
 		throtl_schedule_next_dispatch(sq->parent_sq, true);
 	}
+}
+
+static ssize_t tg_set_conf(struct kernfs_open_file *of,
+			   char *buf, size_t nbytes, loff_t off, bool is_u64)
+{
+	struct blkcg *blkcg = css_to_blkcg(of_css(of));
+	struct blkg_conf_ctx ctx;
+	struct throtl_grp *tg;
+	int ret;
+	u64 v;
+
+	ret = blkg_conf_prep(blkcg, &blkcg_policy_throtl, buf, &ctx);
+	if (ret)
+		return ret;
+
+	ret = -EINVAL;
+	if (sscanf(ctx.body, "%llu", &v) != 1)
+		goto out_finish;
+	if (!v)
+		v = -1;
+
+	tg = blkg_to_tg(ctx.blkg);
+
+	if (is_u64)
+		*(u64 *)((void *)tg + of_cft(of)->private) = v;
+	else
+		*(unsigned int *)((void *)tg + of_cft(of)->private) = v;
 
+	tg_conf_updated(tg);
 	ret = 0;
 out_finish:
 	blkg_conf_finish(&ctx);
Signature · 1 line
2.4.3
author maintainer
#16 lore raw

PATCH 9/10 blkcg: misc preparations for unified hierarchy interface

In reply to Tejun Heo

* Export blkg_dev_name()

* Drop unnecessary @cft from __cfq_set_weight().

Signed-off-by Tejun Heo <[email protected]>
---
 block/blk-cgroup.c         | 3 ++-
 block/cfq-iosched.c        | 8 ++++----
 include/linux/blk-cgroup.h | 1 +
 3 files changed, 7 insertions(+), 5 deletions(-)
block/blk-cgroup.c +2 -1
index 2ea3a2a..b9d511b 100644
@@ -462,13 +462,14 @@ static int blkcg_reset_stats(struct cgroup_subsys_state *css,
 	return 0;
 }
 
-static const char *blkg_dev_name(struct blkcg_gq *blkg)
+const char *blkg_dev_name(struct blkcg_gq *blkg)
 {
 	/* some drivers (floppy) instantiate a queue w/o disk registered */
 	if (blkg->q->backing_dev_info.dev)
 		return dev_name(blkg->q->backing_dev_info.dev);
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(blkg_dev_name);
 
 /**
  * blkcg_print_blkgs - helper for printing per-blkg data
block/cfq-iosched.c +4 -4
index ea88d89..7a72301 100644
@@ -1788,8 +1788,8 @@ static ssize_t cfqg_set_leaf_weight_device(struct kernfs_open_file *of,
 	return __cfqg_set_weight_device(of, buf, nbytes, off, true);
 }
 
-static int __cfq_set_weight(struct cgroup_subsys_state *css, struct cftype *cft,
-			    u64 val, bool is_leaf_weight)
+static int __cfq_set_weight(struct cgroup_subsys_state *css, u64 val,
+			    bool is_leaf_weight)
 {
 	struct blkcg *blkcg = css_to_blkcg(css);
 	struct blkcg_gq *blkg;

@@ -1834,13 +1834,13 @@ static int __cfq_set_weight(struct cgroup_subsys_state *css, struct cftype *cft,
 static int cfq_set_weight(struct cgroup_subsys_state *css, struct cftype *cft,
 			  u64 val)
 {
-	return __cfq_set_weight(css, cft, val, false);
+	return __cfq_set_weight(css, val, false);
 }
 
 static int cfq_set_leaf_weight(struct cgroup_subsys_state *css,
 			       struct cftype *cft, u64 val)
 {
-	return __cfq_set_weight(css, cft, val, true);
+	return __cfq_set_weight(css, val, true);
 }
 
 static int cfqg_print_stat(struct seq_file *sf, void *v)
include/linux/blk-cgroup.h +1 -0
index 85a4d98..b270aef 100644
@@ -182,6 +182,7 @@ int blkcg_activate_policy(struct request_queue *q,
 void blkcg_deactivate_policy(struct request_queue *q,
 			     const struct blkcg_policy *pol);
 
+const char *blkg_dev_name(struct blkcg_gq *blkg);
 void blkcg_print_blkgs(struct seq_file *sf, struct blkcg *blkcg,
 		       u64 (*prfill)(struct seq_file *,
 				     struct blkg_policy_data *, int),
Signature · 1 line
2.4.3
author maintainer
#17 lore raw

PATCH 10/10 blkcg: implement interface for the unified hierarchy

In reply to Tejun Heo

blkcg interface grew to be the biggest of all controllers and unfortunately most inconsistent too. The interface files are inconsistent with a number of cloes duplicates. Some files have recursive variants while others don't. There's distinction between normal and leaf weights which isn't intuitive and there are a lot of stat knobs which don't make much sense outside of debugging and expose too much implementation details to userland.

In the unified hierarchy, everything is always hierarchical and internal nodes can't have tasks rendering the two structural issues twisting the current interface. The interface has to be updated in a significant anyway and this is a good chance to revamp it as a whole. This patch implements blkcg interface for the unified hierarchy.

* (from a previous patch) blkcg is identified by "io" instead of "blkio" on the unified hierarchy. Given that the whole interface is updated anyway, the rename shouldn't carry noticeable conversion overhead.

* The original interface consisted of 27 files is replaced with the following three files.

blkio.stat : per-blkcg stats
blkio.weight : per-cgroup and per-cgroup-queue weight settings
blkio.max : per-cgroup-queue bps and iops max limits

Documentation/cgroups/unified-hierarchy.txt updated accordingly.

Signed-off-by Tejun Heo <[email protected]>
---
 Documentation/cgroups/unified-hierarchy.txt |  57 +++++++++++++-
 block/blk-cgroup.c                          |  51 +++++++++++++
 block/blk-throttle.c                        | 112 ++++++++++++++++++++++++++++
 block/cfq-iosched.c                         |  61 +++++++++++++--
 include/linux/blk-cgroup.h                  |   1 +
 5 files changed, 275 insertions(+), 7 deletions(-)
Documentation/cgroups/unified-hierarchy.txt +55 -2
index 86847a7..4e23d4c 100644
@@ -374,9 +374,62 @@ supported and the interface files "release_agent" and
 
 5-3. Per-Controller Changes
 
-5-3-1. blkio
+5-3-1. io
 
-- blk-throttle becomes properly hierarchical.
+- blkio is renamed to io.  The interface is overhauled anyway.  The
+  new name is more in line with the other two major controllers, cpu
+  and memory, and better suited given that it may be used for cgroup
+  writeback without involving block layer.
+
+- Everything including stat is always hierarchical making separate
+  recursive stat files pointless and, as no internal node can have
+  tasks, leaf weights are meaningless.  The operation model is
+  simplified and the interface is overhauled accordingly.
+
+  io.stat
+
+	The stat file.  The reported stats are from the point where
+	bio's are issued to request_queue.  The stats are counted
+	independent of which policies are enabled.  Each line in the
+	file follows the following format.  More fields may later be
+	added at the end.
+
+	  $MAJ:$MIN rbytes=$RBYTES wbytes=$WBYTES rios=$RIOS wrios=$WIOS
+
+  io.weight
+
+	The weight setting, currently only available and effective if
+	cfq-iosched is in use for the target device.  The weight is
+	between 10 and 1000 and defaults to 500.  The first line
+	always contains the default weight in the following format to
+	use when per-device setting is missing.
+
+	  default $WEIGHT
+
+	Subsequent lines list per-device weights of the following
+	format.
+
+	  $MAJ:$MIN $WEIGHT
+
+	Writing "$WEIGHT" or "default $WEIGHT" changes the default
+	setting.  Writing "$MAJ:$MIN $WEIGHT" sets per-device weight
+	while "$MAJ:$MIN default" clears it.
+
+	This file is available only on non-root cgroups.
+
+  io.max
+
+	The maximum bandwidth and/or iops setting, only available if
+	blk-throttle is enabled.  The file is of the following format.
+
+	  $MAJ:$MIN rbps=$RBPS wbps=$WBPS riops=$RIOPS wiops=$WIOPS
+
+	${R|W}BPS are read/write bytes per second and ${R|W}IOPS are
+	read/write IOs per second.  "max" indicates no limit.  Writing
+	to the file follows the same format but the individual
+	settings may be ommitted or specified in any order.
+
+	This file is available only on non-root cgroups.
 
 
 5-3-2. cpuset
block/blk-cgroup.c +51 -0

69 lines not on this page · raw message

block/blk-throttle.c +112 -0

124 lines not on this page · raw message

block/cfq-iosched.c +56 -5

97 lines not on this page · raw message

include/linux/blk-cgroup.h +1 -0
index b270aef..9a7c4bd 100644
@@ -148,6 +148,7 @@ typedef void (blkcg_pol_reset_pd_stats_fn)(struct blkg_policy_data *pd);
 struct blkcg_policy {
 	int				plid;
 	/* cgroup files for the policy */
+	struct cftype			*dfl_cftypes;
 	struct cftype			*legacy_cftypes;
 
 	/* operations */
Signature · 1 line
2.4.3
author maintainer
#18 lore raw

PATCH v2 10/10 blkcg: implement interface for the unified hierarchy

From 1618deedf2fb0788ae11ac544f45671ceb6e43ec Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Thu, 30 Jul 2015 18:51:53 -0400

blkcg interface grew to be the biggest of all controllers and unfortunately most inconsistent too. The interface files are inconsistent with a number of cloes duplicates. Some files have recursive variants while others don't. There's distinction between normal and leaf weights which isn't intuitive and there are a lot of stat knobs which don't make much sense outside of debugging and expose too much implementation details to userland.

In the unified hierarchy, everything is always hierarchical and internal nodes can't have tasks rendering the two structural issues twisting the current interface. The interface has to be updated in a significant anyway and this is a good chance to revamp it as a whole. This patch implements blkcg interface for the unified hierarchy.

* (from a previous patch) blkcg is identified by "io" instead of "blkio" on the unified hierarchy. Given that the whole interface is updated anyway, the rename shouldn't carry noticeable conversion overhead.

* The original interface consisted of 27 files is replaced with the following three files.

blkio.stat : per-blkcg stats
blkio.weight : per-cgroup and per-cgroup-queue weight settings
blkio.max : per-cgroup-queue bps and iops max limits

Documentation/cgroups/unified-hierarchy.txt updated accordingly.

v2: blkcg_policy->dfl_cftypes wasn't removed on
blkcg_policy_unregister() corrupting the cftypes list. Fixed.

Signed-off-by Tejun Heo <[email protected]>

---
git branch updated accordingly.

Thanks.

 Documentation/cgroups/unified-hierarchy.txt |  57 +++++++++++++-
 block/blk-cgroup.c                          |  53 +++++++++++++
 block/blk-throttle.c                        | 112 ++++++++++++++++++++++++++++
 block/cfq-iosched.c                         |  61 +++++++++++++--
 include/linux/blk-cgroup.h                  |   1 +
 5 files changed, 277 insertions(+), 7 deletions(-)
Documentation/cgroups/unified-hierarchy.txt +55 -2
index 86847a7..4e23d4c 100644
@@ -374,9 +374,62 @@ supported and the interface files "release_agent" and
 
 5-3. Per-Controller Changes
 
-5-3-1. blkio
+5-3-1. io
 
-- blk-throttle becomes properly hierarchical.
+- blkio is renamed to io.  The interface is overhauled anyway.  The
+  new name is more in line with the other two major controllers, cpu
+  and memory, and better suited given that it may be used for cgroup
+  writeback without involving block layer.
+
+- Everything including stat is always hierarchical making separate
+  recursive stat files pointless and, as no internal node can have
+  tasks, leaf weights are meaningless.  The operation model is
+  simplified and the interface is overhauled accordingly.
+
+  io.stat
+
+	The stat file.  The reported stats are from the point where
+	bio's are issued to request_queue.  The stats are counted
+	independent of which policies are enabled.  Each line in the
+	file follows the following format.  More fields may later be
+	added at the end.
+
+	  $MAJ:$MIN rbytes=$RBYTES wbytes=$WBYTES rios=$RIOS wrios=$WIOS
+
+  io.weight
+
+	The weight setting, currently only available and effective if
+	cfq-iosched is in use for the target device.  The weight is
+	between 10 and 1000 and defaults to 500.  The first line
+	always contains the default weight in the following format to
+	use when per-device setting is missing.
+
+	  default $WEIGHT
+
+	Subsequent lines list per-device weights of the following
+	format.
+
+	  $MAJ:$MIN $WEIGHT
+
+	Writing "$WEIGHT" or "default $WEIGHT" changes the default
+	setting.  Writing "$MAJ:$MIN $WEIGHT" sets per-device weight
+	while "$MAJ:$MIN default" clears it.
+
+	This file is available only on non-root cgroups.
+
+  io.max
+
+	The maximum bandwidth and/or iops setting, only available if
+	blk-throttle is enabled.  The file is of the following format.
+
+	  $MAJ:$MIN rbps=$RBPS wbps=$WBPS riops=$RIOPS wiops=$WIOPS
+
+	${R|W}BPS are read/write bytes per second and ${R|W}IOPS are
+	read/write IOs per second.  "max" indicates no limit.  Writing
+	to the file follows the same format but the individual
+	settings may be ommitted or specified in any order.
+
+	This file is available only on non-root cgroups.
 
 
 5-3-2. cpuset
block/blk-cgroup.c +53 -0

77 lines not on this page · raw message

block/blk-throttle.c +112 -0

124 lines not on this page · raw message

block/cfq-iosched.c +56 -5

97 lines not on this page · raw message

include/linux/blk-cgroup.h +1 -0
index b270aef..9a7c4bd 100644
@@ -148,6 +148,7 @@ typedef void (blkcg_pol_reset_pd_stats_fn)(struct blkg_policy_data *pd);
 struct blkcg_policy {
 	int				plid;
 	/* cgroup files for the policy */
+	struct cftype			*dfl_cftypes;
 	struct cftype			*legacy_cftypes;
 
 	/* operations */
Signature · 1 line
2.4.3
#19 lore raw
In reply to Tejun Heo
Quoted 20 lines · Tejun Heo

Hello,

blkcg interface grew to be the biggest of all controllers and unfortunately most inconsistent too. The interface files are inconsistent with a number of cloes duplicates. Some files have recursive variants while others don't. There's distinction between normal and leaf weights which isn't intuitive and there are a lot of stat knobs which don't make much sense outside of debugging and expose too much implementation details to userland.

In the unified hierarchy, everything is always hierarchical and internal nodes can't have tasks rendering the two structural issues twisting the current interface. The interface has to be updated in a significant anyway and this is a good chance to revamp it as a whole. This patchset implements blkcg interface for the unified hierarchy.

* blkcg is identified by "io" instead of "blkio" on the unified hierarchy. Given that the whole interface is updated anyway, the rename shouldn't carry noticeable conversion overhead.

Quoted 7 lines

* The original interface consisted of 27 files is replaced with the following three files.

blkio.stat : per-blkcg stats
blkio.weight : per-cgroup and per-cgroup-queue weight settings
blkio.max : per-cgroup-queue bps and iops max limits

Hi Tejun,

I browsed though the details of above knobs and it sounds great. It is clean and much less number of knobs and files. You got rid of all the debug CFQ knobs which is good. I was not happy with these either. Glad to see that all the magic about leaf weight is gone. That was really mind bending. Knob for reset stats is gone and instead of mutiple files for configuration now we are using single file for R/W BPS/IOPS configuration.

I will do some basic testing and see if something pops up.

Userspace will need to understand these new files but that's the understanding anyway that unified hierarchy is different and needs to be handled differently.

Thanks
Vivek

author maintainer
#20 lore raw

Hello,

Quoted 3 lines · Vivek Goyal

I browsed though the details of above knobs and it sounds great. It is clean and much less number of knobs and files. You got rid of all the debug CFQ knobs which is good. I was not happy with these either. Glad

Yeah, especially given that CFQ may be replaced by BFQ.

Quoted 10 lines

to see that all the magic about leaf weight is gone. That was really mind bending. Knob for reset stats is gone and instead of mutiple files for configuration now we are using single file for R/W BPS/IOPS configuration.

I will do some basic testing and see if something pops up.

Userspace will need to understand these new files but that's the understanding anyway that unified hierarchy is different and needs to be handled differently.

For blkcg (and memory too), the usage model changes so drastically that I don't think blkcg interface changes would matter much. Everything is changed anyway.

Thanks.

Signature · 1 line
tejun