[PATCH 0/2] block, bfq: add weight symlink to the bfq.weight cgroup parameter

[PATCH 0/2] block, bfq: add weight symlink to the bfq.weight cgroup parameter

[Paolo Valente]

Many userspace tools and services use the proportional-share policy of
the blkio/io cgroups controller. The CFQ I/O scheduler implemented
this policy for the legacy block layer. To modify the weight of a
group in case CFQ was in charge, the 'weight' parameter of the group
must be modified. On the other hand, the BFQ I/O scheduler implements
the same policy in blk-mq, but, with BFQ, the parameter to modify has
a different name: bfq.weight (forced choice until legacy block was
present, because two different policies cannot share a common parameter
in cgroups).

Due to CFQ legacy, most if not all userspace configurations still use
the parameter 'weight', and for the moment do not seem likely to be
changed. But, when CFQ went away with legacy block, such a parameter
ceased to exist.

So, a simple workaround has been proposed by Johannes [1] to make all
configurations work: add a symlink, named weight, to bfq.weight. This
pair of patches adds:
1) the possibility to create a symlink to a cgroup file;
2) the above 'weight' symlink.

Thanks,
Paolo

[1] https://lkml.org/lkml/2019/4/8/555

Angelo Ruocco (2):
  cgroup: let a symlink too be created with a cftype file
  block, bfq: add weight symlink to the bfq.weight cgroup parameter

 block/bfq-cgroup.c          |  6 ++++--
 include/linux/cgroup-defs.h |  3 +++
 kernel/cgroup/cgroup.c      | 33 +++++++++++++++++++++++++++++----
 3 files changed, 36 insertions(+), 6 deletions(-)

--
2.20.1

[PATCH 1/2] cgroup: let a symlink too be created with a cftype file

[Paolo Valente]

From: Angelo Ruocco <angeloruocco90@gmail.com>

This commit enables a cftype to have a symlink (of any name) that
points to the file associated with the cftype.

Signed-off-by: Angelo Ruocco <angeloruocco90@gmail.com>
Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 include/linux/cgroup-defs.h |  3 +++
 kernel/cgroup/cgroup.c      | 33 +++++++++++++++++++++++++++++----
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 77258d276f93..2ad9f53ecefe 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -101,6 +101,8 @@ enum {
 	CFTYPE_WORLD_WRITABLE	= (1 << 4),	/* (DON'T USE FOR NEW FILES) S_IWUGO */
 	CFTYPE_DEBUG		= (1 << 5),	/* create when cgroup_debug */
 
+	CFTYPE_SYMLINKED	= (1 << 6),	/* pointed to by symlink too */
+
 	/* internal flags, do not use outside cgroup core proper */
 	__CFTYPE_ONLY_ON_DFL	= (1 << 16),	/* only on default hierarchy */
 	__CFTYPE_NOT_ON_DFL	= (1 << 17),	/* not on default hierarchy */
@@ -538,6 +540,7 @@ struct cftype {
 	 * end of cftype array.
 	 */
 	char name[MAX_CFTYPE_NAME];
+	char link_name[MAX_CFTYPE_NAME];
 	unsigned long private;
 
 	/*
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 217cec4e22c6..f77dee5eb82a 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1460,8 +1460,8 @@ struct cgroup *task_cgroup_from_root(struct task_struct *task,
 
 static struct kernfs_syscall_ops cgroup_kf_syscall_ops;
 
-static char *cgroup_file_name(struct cgroup *cgrp, const struct cftype *cft,
-			      char *buf)
+static char *cgroup_fill_name(struct cgroup *cgrp, const struct cftype *cft,
+			      char *buf, bool write_link_name)
 {
 	struct cgroup_subsys *ss = cft->ss;
 
@@ -1471,13 +1471,26 @@ static char *cgroup_file_name(struct cgroup *cgrp, const struct cftype *cft,
 
 		snprintf(buf, CGROUP_FILE_NAME_MAX, "%s%s.%s",
 			 dbg, cgroup_on_dfl(cgrp) ? ss->name : ss->legacy_name,
-			 cft->name);
+			 write_link_name ? cft->link_name : cft->name);
 	} else {
-		strscpy(buf, cft->name, CGROUP_FILE_NAME_MAX);
+		strscpy(buf, write_link_name ? cft->link_name : cft->name,
+			CGROUP_FILE_NAME_MAX);
 	}
 	return buf;
 }
 
+static char *cgroup_file_name(struct cgroup *cgrp, const struct cftype *cft,
+			      char *buf)
+{
+	return cgroup_fill_name(cgrp, cft, buf, false);
+}
+
+static char *cgroup_link_name(struct cgroup *cgrp, const struct cftype *cft,
+			      char *buf)
+{
+	return cgroup_fill_name(cgrp, cft, buf, true);
+}
+
 /**
  * cgroup_file_mode - deduce file mode of a control file
  * @cft: the control file in question
@@ -1636,6 +1649,9 @@ static void cgroup_rm_file(struct cgroup *cgrp, const struct cftype *cft)
 	}
 
 	kernfs_remove_by_name(cgrp->kn, cgroup_file_name(cgrp, cft, name));
+	if (cft->flags & CFTYPE_SYMLINKED)
+		kernfs_remove_by_name(cgrp->kn,
+				      cgroup_link_name(cgrp, cft, name));
 }
 
 /**
@@ -3809,6 +3825,7 @@ static int cgroup_add_file(struct cgroup_subsys_state *css, struct cgroup *cgrp,
 {
 	char name[CGROUP_FILE_NAME_MAX];
 	struct kernfs_node *kn;
+	struct kernfs_node *kn_link;
 	struct lock_class_key *key = NULL;
 	int ret;
 
@@ -3839,6 +3856,14 @@ static int cgroup_add_file(struct cgroup_subsys_state *css, struct cgroup *cgrp,
 		spin_unlock_irq(&cgroup_file_kn_lock);
 	}
 
+	if (cft->flags & CFTYPE_SYMLINKED) {
+		kn_link = kernfs_create_link(cgrp->kn,
+					     cgroup_link_name(cgrp, cft, name),
+					     kn);
+		if (IS_ERR(kn_link))
+			return PTR_ERR(kn_link);
+	}
+
 	return 0;
 }
 
-- 
2.20.1

[PATCH 2/2] block, bfq: add weight symlink to the bfq.weight cgroup parameter

[Paolo Valente]

From: Angelo Ruocco <angeloruocco90@gmail.com>

Many userspace tools and services use the proportional-share policy of
the blkio/io cgroups controller. The CFQ I/O scheduler implemented
this policy for the legacy block layer. To modify the weight of a
group in case CFQ was in charge, the 'weight' parameter of the group
must be modified. On the other hand, the BFQ I/O scheduler implements
the same policy in blk-mq, but, with BFQ, the parameter to modify has
a different name: bfq.weight (forced choice until legacy block was
present, because two different policies cannot share a common parameter
in cgroups).

Due to CFQ legacy, most if not all userspace configurations still use
the parameter 'weight', and for the moment do not seem likely to be
changed. But, when CFQ went away with legacy block, such a parameter
ceased to exist.

So, a simple workaround has been proposed [1] to make all
configurations work: add a symlink, named weight, to bfq.weight. This
commit adds such a symlink.

[1] https://lkml.org/lkml/2019/4/8/555

Suggested-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Angelo Ruocco <angeloruocco90@gmail.com>
Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 block/bfq-cgroup.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index b3796a40a61a..59f46904cb11 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -1046,7 +1046,8 @@ struct blkcg_policy blkcg_policy_bfq = {
 struct cftype bfq_blkcg_legacy_files[] = {
 	{
 		.name = "bfq.weight",
-		.flags = CFTYPE_NOT_ON_ROOT,
+		.link_name = "weight",
+		.flags = CFTYPE_NOT_ON_ROOT | CFTYPE_SYMLINKED,
 		.seq_show = bfq_io_show_weight,
 		.write_u64 = bfq_io_set_weight_legacy,
 	},
@@ -1166,7 +1167,8 @@ struct cftype bfq_blkcg_legacy_files[] = {
 struct cftype bfq_blkg_files[] = {
 	{
 		.name = "bfq.weight",
-		.flags = CFTYPE_NOT_ON_ROOT,
+		.link_name = "weight",
+		.flags = CFTYPE_NOT_ON_ROOT | CFTYPE_SYMLINKED,
 		.seq_show = bfq_io_show_weight,
 		.write = bfq_io_set_weight,
 	},
-- 
2.20.1

Re: [PATCH 0/2] block, bfq: add weight symlink to the bfq.weight cgroup parameter

[Paolo Valente]

[-- Attachment #1: Type: text/plain, Size: 1726 bytes --]

Hi Jens,
have you had time to look into this?

Thanks,
Paolo

> Il giorno 21 mag 2019, alle ore 10:01, Paolo Valente <paolo.valente@linaro.org> ha scritto:
> 
> Many userspace tools and services use the proportional-share policy of
> the blkio/io cgroups controller. The CFQ I/O scheduler implemented
> this policy for the legacy block layer. To modify the weight of a
> group in case CFQ was in charge, the 'weight' parameter of the group
> must be modified. On the other hand, the BFQ I/O scheduler implements
> the same policy in blk-mq, but, with BFQ, the parameter to modify has
> a different name: bfq.weight (forced choice until legacy block was
> present, because two different policies cannot share a common parameter
> in cgroups).
> 
> Due to CFQ legacy, most if not all userspace configurations still use
> the parameter 'weight', and for the moment do not seem likely to be
> changed. But, when CFQ went away with legacy block, such a parameter
> ceased to exist.
> 
> So, a simple workaround has been proposed by Johannes [1] to make all
> configurations work: add a symlink, named weight, to bfq.weight. This
> pair of patches adds:
> 1) the possibility to create a symlink to a cgroup file;
> 2) the above 'weight' symlink.
> 
> Thanks,
> Paolo
> 
> [1] https://lkml.org/lkml/2019/4/8/555
> 
> Angelo Ruocco (2):
>  cgroup: let a symlink too be created with a cftype file
>  block, bfq: add weight symlink to the bfq.weight cgroup parameter
> 
> block/bfq-cgroup.c          |  6 ++++--
> include/linux/cgroup-defs.h |  3 +++
> kernel/cgroup/cgroup.c      | 33 +++++++++++++++++++++++++++++----
> 3 files changed, 36 insertions(+), 6 deletions(-)
> 
> --
> 2.20.1


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/2] block, bfq: add weight symlink to the bfq.weight cgroup parameter

[Paolo Valente]

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

ping

> Il giorno 30 mag 2019, alle ore 17:02, Paolo Valente <paolo.valente@linaro.org> ha scritto:
> 
> Hi Jens,
> have you had time to look into this?
> 
> Thanks,
> Paolo
> 
>> Il giorno 21 mag 2019, alle ore 10:01, Paolo Valente <paolo.valente@linaro.org> ha scritto:
>> 
>> Many userspace tools and services use the proportional-share policy of
>> the blkio/io cgroups controller. The CFQ I/O scheduler implemented
>> this policy for the legacy block layer. To modify the weight of a
>> group in case CFQ was in charge, the 'weight' parameter of the group
>> must be modified. On the other hand, the BFQ I/O scheduler implements
>> the same policy in blk-mq, but, with BFQ, the parameter to modify has
>> a different name: bfq.weight (forced choice until legacy block was
>> present, because two different policies cannot share a common parameter
>> in cgroups).
>> 
>> Due to CFQ legacy, most if not all userspace configurations still use
>> the parameter 'weight', and for the moment do not seem likely to be
>> changed. But, when CFQ went away with legacy block, such a parameter
>> ceased to exist.
>> 
>> So, a simple workaround has been proposed by Johannes [1] to make all
>> configurations work: add a symlink, named weight, to bfq.weight. This
>> pair of patches adds:
>> 1) the possibility to create a symlink to a cgroup file;
>> 2) the above 'weight' symlink.
>> 
>> Thanks,
>> Paolo
>> 
>> [1] https://lkml.org/lkml/2019/4/8/555
>> 
>> Angelo Ruocco (2):
>> cgroup: let a symlink too be created with a cftype file
>> block, bfq: add weight symlink to the bfq.weight cgroup parameter
>> 
>> block/bfq-cgroup.c          |  6 ++++--
>> include/linux/cgroup-defs.h |  3 +++
>> kernel/cgroup/cgroup.c      | 33 +++++++++++++++++++++++++++++----
>> 3 files changed, 36 insertions(+), 6 deletions(-)
>> 
>> --
>> 2.20.1
> 


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/2] block, bfq: add weight symlink to the bfq.weight cgroup parameter

[Jens Axboe]

On 5/21/19 2:01 AM, Paolo Valente wrote:
> Many userspace tools and services use the proportional-share policy of
> the blkio/io cgroups controller. The CFQ I/O scheduler implemented
> this policy for the legacy block layer. To modify the weight of a
> group in case CFQ was in charge, the 'weight' parameter of the group
> must be modified. On the other hand, the BFQ I/O scheduler implements
> the same policy in blk-mq, but, with BFQ, the parameter to modify has
> a different name: bfq.weight (forced choice until legacy block was
> present, because two different policies cannot share a common parameter
> in cgroups).
> 
> Due to CFQ legacy, most if not all userspace configurations still use
> the parameter 'weight', and for the moment do not seem likely to be
> changed. But, when CFQ went away with legacy block, such a parameter
> ceased to exist.
> 
> So, a simple workaround has been proposed by Johannes [1] to make all
> configurations work: add a symlink, named weight, to bfq.weight. This
> pair of patches adds:
> 1) the possibility to create a symlink to a cgroup file;
> 2) the above 'weight' symlink.

Applied, thanks.

-- 
Jens Axboe