cgroup: misc fixes and cleanups
Tejun Heo
maintainerPATCH 1/1 cgroup: misc fixes and cleanups
Hello,
This patchset contains the following six patches. The first one is a fix but should be safe to route through for-4.6. The rest are misc cleanups and improvements which don't cause notable behavior changes.
0001-cgroup-fix-error-return-value-of-cgroup_addrm_files.patch 0002-Revert-cgroup-add-cgroup_subsys-css_e_css_changed.patch 0003-cgroup-s-child_subsys_mask-subtree_ss_mask.patch 0004-cgroup-convert-for_each_subsys_which-to-do-while-sty.patch 0005-cgroup-use-do_each_subsys_mask-where-applicable.patch 0006-cgroup-make-cgroup-subsystem-masks-u16.patch
The patchset based on top of cgroup/for-4.6 223ffb29f972 ("cgroup:
provide cgroup_nov1= to disable controllers in v1 mounts") and is available in the following git branch.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-misc
diffstat follows. I'll pull in the branch into cgroup/for-4.6 soonish.
include/linux/cgroup-defs.h | 12 +- kernel/cgroup.c | 217 +++++++++++++++++++------------------------- 2 files changed, 102 insertions(+), 127 deletions(-)
Thanks.
--
tejun
Tejun Heo
maintainerPATCH 1/6 cgroup: fix error return value of cgroup_addrm_files()
cgroup_addrm_files() incorrectly returned 0 after add failure. Fix it.
--- kernel/cgroup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/cgroup.c +2 -2
@@ -3369,7 +3369,7 @@ static int cgroup_addrm_files(struct cgroup_subsys_state *css, bool is_add) {struct cftype *cft, *cft_end =NULL ; -int ret; +int ret =0 ;lockdep_assert_held (&cgroup_mutex);
@@ -3398,7 +3398,7 @@ static int cgroup_addrm_files(struct cgroup_subsys_state *css,cgroup_rm_file (cgrp, cft); } } -return 0 ; +return ret; }static int cgroup_apply_cftypes (struct cftype *cfts,bool is_add)
Signature · 1 line
2.5.0
Tejun Heo
maintainerPATCH 2/6 Revert "cgroup: add cgroup_subsys->css_e_css_changed()"
In reply to Tejun HeoThis reverts commit 56c807ba4e91f0980567b6a69de239677879b17f.
cgroup_subsys->css_e_css_changed() was supposed to be used by cgroup writeback support; however, the change to per-inode cgroup association made it unnecessary and the callback doesn't have any user. Remove it.
--- include/linux/cgroup-defs.h | 1 - kernel/cgroup.c | 18 ------------------ 2 files changed, 19 deletions(-)
include/linux/cgroup-defs.h +0 -1
@@ -434,7 +434,6 @@ struct cgroup_subsys {void (*css_released)(struct cgroup_subsys_state *css);void (*css_free)(struct cgroup_subsys_state *css);void (*css_reset)(struct cgroup_subsys_state *css); -void (*css_e_css_changed)(struct cgroup_subsys_state *css);int (*can_attach)(struct cgroup_taskset *tset);void (*cancel_attach)(struct cgroup_taskset *tset);
kernel/cgroup.c +0 -18
@@ -3127,24 +3127,6 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of, } } -/* -* The effective csses of all the descendants (excluding @cgrp) may -* have changed. Subsystems can optionally subscribe to this event -* by implementing ->css_e_css_changed() which is invoked if any of -* the effective csses seen from the css's cgroup may have changed. -*/ -for_each_subsys (ss, ssid) { -struct cgroup_subsys_state *this_css =cgroup_css (cgrp, ss); -struct cgroup_subsys_state *css; - -if (!ss->css_e_css_changed || !this_css) -continue ; - -css_for_each_descendant_pre (css, this_css) -if (css != this_css) - ss->css_e_css_changed (css); - } -kernfs_activate (cgrp->kn); ret =0 ; out_unlock:
Signature · 1 line
2.5.0
Tejun Heo
maintainerPATCH 3/6 cgroup: s/child_subsys_mask/subtree_ss_mask/
In reply to Tejun HeoFor consistency with cgroup->subtree_control.
* cgroup->child_subsys_mask -> cgroup->subtree_ss_mask
* cgroup_calc_child_subsys_mask() -> cgroup_calc_subtree_ss_mask()
* cgroup_refresh_child_subsys_mask() -> cgroup_refresh_subtree_ss_mask()
No functional changes.
--- include/linux/cgroup-defs.h | 11 +++++------ kernel/cgroup.c | 48 ++++++++++++++++++++++----------------------- 2 files changed, 29 insertions(+), 30 deletions(-)
include/linux/cgroup-defs.h +5 -6
@@ -253,13 +253,12 @@ struct cgroup {/* * The bitmask of subsystems enabled on the child cgroups. * ->subtree_control is the one configured through -* "cgroup.subtree_control" while ->child_subsys_mask is the -* effective one which may have more subsystems enabled. -* Controller knobs are made available iff it's enabled in -* ->subtree_control. +* "cgroup.subtree_control" while ->child_ss_mask is the effective +* one which may have more subsystems enabled. Controller knobs +* are made available iff it's enabled in ->subtree_control. */ -unsigned int subtree_control; -unsigned int child_subsys_mask; +unsigned long subtree_control; +unsigned long subtree_ss_mask;/* Private pointers for each registered subsystem */ struct cgroup_subsys_state __rcu *subsys[CGROUP_SUBSYS_COUNT ];
kernel/cgroup.c +24 -24
148 lines not on this page · raw message
Signature · 1 line
2.5.0
Tejun Heo
maintainerPATCH 4/6 cgroup: convert for_each_subsys_which() to do-while style
In reply to Tejun Heofor_each_subsys_which() allows iterating subsystems specified in a subsystem bitmask; unfortunately, it requires the mask to be an unsigned long l-value which can be inconvenient and makes it awkward to use a smaller type for subsystem masks.
This patch converts for_each_subsy_which() to do-while style which allows it to drop the l-value requirement. The new iterator is named do_each_subsys_mask() / while_each_subsys_mask().
--- kernel/cgroup.c | 72 ++++++++++++++++++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 32 deletions(-)
kernel/cgroup.c +40 -32
@@ -514,22 +514,28 @@ static int notify_on_release(const struct cgroup *cgrp) (((ss) = cgroup_subsys[ssid]) || true); (ssid)++)/** -* for_each_subsys_which - filter for_each_subsys with a bitmask +* do_each_subsys_mask - filter for_each_subsys with a bitmask * @ss: the iteration cursor * @ssid: the index of @ss, CGROUP_SUBSYS_COUNT after reaching the end -* @ss_maskp: a pointer to the bitmask +* @ss_mask: the bitmask * * The block will only run for cases where the ssid-th bit (1 << ssid) of -* mask is set to 1. +* @ss_mask is set. */ -#define for_each_subsys_which (ss, ssid, ss_maskp) \ - if (!CGROUP_SUBSYS_COUNT)/* to avoid spurious gcc warning */ \ +#define do_each_subsys_mask (ss, ssid, ss_mask) do { \ + unsigned long __ss_mask = (ss_mask); \ + if (!CGROUP_SUBSYS_COUNT) {/* to avoid spurious gcc warning */ \ (ssid) =0 ; \ - else \ -for_each_set_bit (ssid ,ss_maskp ,CGROUP_SUBSYS_COUNT ) \ - if (((ss) = cgroup_subsys[ssid]) && false) \ - break; \ - else +break ; \ + } \ + for_each_set_bit(ssid, &__ss_mask, CGROUP_SUBSYS_COUNT) { \ + (ss) = cgroup_subsys[ssid]; \ + { + +#define while_each_subsys_mask () \ + } \ + } \ +} while (false)/* iterate across the hierarchies */ #define for_each_root (root) \
@@ -1284,8 +1290,9 @@ static unsigned long cgroup_calc_subtree_ss_mask(struct cgroup *cgrp,while (true) {unsigned long new_ss_mask = cur_ss_mask; -for_each_subsys_which (ss, ssid, &cur_ss_mask) + do_each_subsys_mask(ss, ssid, cur_ss_mask) { new_ss_mask |= ss->depends_on; + }while_each_subsys_mask ();/* * Mask out subsystems which aren't available. This can
@@ -1469,7 +1476,7 @@ static int rebind_subsystems(struct cgroup_root *dst_root,lockdep_assert_held (&cgroup_mutex); -for_each_subsys_which (ss, ssid, &ss_mask) { + do_each_subsys_mask(ss, ssid, ss_mask) {/* if @ss has non-root csses attached to it, can't move */ if (css_next_child (NULL ,cgroup_css (&ss->root->cgrp, ss)))return -EBUSY ;
@@ -1477,14 +1484,14 @@ static int rebind_subsystems(struct cgroup_root *dst_root,/* can't move between two non-dummy roots either */ if (ss->root != &cgrp_dfl_root && dst_root != &cgrp_dfl_root)return -EBUSY ; - } + }while_each_subsys_mask ();/* skip creating root files on dfl_root for inhibited subsystems */ tmp_ss_mask = ss_mask;if (dst_root == &cgrp_dfl_root) tmp_ss_mask &= ~cgrp_dfl_root_inhibit_ss_mask; -for_each_subsys_which (ss, ssid, &tmp_ss_mask) { + do_each_subsys_mask(ss, ssid, tmp_ss_mask) {struct cgroup *scgrp = &ss->root->cgrp;int tssid;
@@ -1507,19 +1514,19 @@ static int rebind_subsystems(struct cgroup_root *dst_root,continue ; } -for_each_subsys_which (ss, tssid, &tmp_ss_mask) { + do_each_subsys_mask(ss, tssid, tmp_ss_mask) {if (tssid == ssid)break ;css_clear_dir (cgroup_css (scgrp, ss), dcgrp); - } + }while_each_subsys_mask ();return ret; - } + }while_each_subsys_mask ();/* * Nothing can fail from this point on. Remove files for the * removed subsystems and rebind each subsystem. */ -for_each_subsys_which (ss, ssid, &ss_mask) { + do_each_subsys_mask(ss, ssid, ss_mask) {struct cgroup_root *src_root = ss->root;struct cgroup *scgrp = &src_root->cgrp;struct cgroup_subsys_state *css =cgroup_css (scgrp, ss);
@@ -1556,7 +1563,7 @@ static int rebind_subsystems(struct cgroup_root *dst_root,if (ss->bind) ss->bind (css); - } + }while_each_subsys_mask ();kernfs_activate (dcgrp->kn);return 0 ;
@@ -2838,12 +2845,12 @@ static void cgroup_print_ss_mask(struct seq_file *seq, unsigned long ss_mask)bool printed = false;int ssid; -for_each_subsys_which (ss, ssid, &ss_mask) { + do_each_subsys_mask(ss, ssid, ss_mask) {if (printed)seq_putc (seq,' ' );seq_printf (seq,"%s" , ss->name); printed = true; - } + }while_each_subsys_mask ();if (printed)seq_putc (seq,'\n' ); }
@@ -2956,11 +2963,9 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of,*/ buf =strstrip (buf);while ((tok =strsep (&buf," " ))) { -unsigned long tmp_ss_mask = ~cgrp_dfl_root_inhibit_ss_mask; -if (tok[0 ] =='\0' )continue ; -for_each_subsys_which (ss, ssid, &tmp_ss_mask) { + do_each_subsys_mask(ss, ssid, ~cgrp_dfl_root_inhibit_ss_mask) {if (!cgroup_ssid_enabled (ssid) ||strcmp (tok +1 , ss->name))continue ;
@@ -2975,7 +2980,7 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of,return -EINVAL ; }break ; - } + }while_each_subsys_mask ();if (ssid ==CGROUP_SUBSYS_COUNT )return -EINVAL ; }
@@ -3049,7 +3054,7 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of,* still around. In such cases, wait till it's gone using * offline_waitq. */ -for_each_subsys_which (ss, ssid, &css_enable) { + do_each_subsys_mask(ss, ssid, css_enable) {cgroup_for_each_live_child (child, cgrp) {DEFINE_WAIT (wait);
@@ -3066,7 +3071,7 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of,return restart_syscall (); } - } + }while_each_subsys_mask (); cgrp->subtree_control = new_sc; cgrp->subtree_ss_mask = new_ss;
@@ -5509,11 +5514,11 @@ int cgroup_can_fork(struct task_struct *child)struct cgroup_subsys *ss;int i, j, ret; -for_each_subsys_which (ss, i, &have_canfork_callback) { + do_each_subsys_mask(ss, i, have_canfork_callback) { ret = ss->can_fork (child);if (ret) goto out_revert; - } + }while_each_subsys_mask ();return 0 ;
@@ -5598,8 +5603,9 @@ void cgroup_post_fork(struct task_struct *child)* css_set; otherwise, @child might change state between ->fork() * and addition to css_set. */ -for_each_subsys_which (ss, i, &have_fork_callback) + do_each_subsys_mask(ss, i, have_fork_callback) { ss->fork (child); + }while_each_subsys_mask (); }/**
@@ -5642,8 +5648,9 @@ void cgroup_exit(struct task_struct *tsk) }/* see cgroup_post_fork() for details */ -for_each_subsys_which (ss, i, &have_exit_callback) + do_each_subsys_mask(ss, i, have_exit_callback) { ss->exit (tsk); + }while_each_subsys_mask (); }void cgroup_free (struct task_struct *task)
@@ -5652,8 +5659,9 @@ void cgroup_free(struct task_struct *task)struct cgroup_subsys *ss;int ssid; -for_each_subsys_which (ss, ssid, &have_free_callback) + do_each_subsys_mask(ss, ssid, have_free_callback) { ss->free (task); + }while_each_subsys_mask ();put_css_set (cset); }
Signature · 1 line
2.5.0
Tejun Heo
maintainerPATCH 5/6 cgroup: use do_each_subsys_mask() where applicable
In reply to Tejun HeoThere are several places in cgroup_subtree_control_write() which can use do_each_subsys_mask() instead of manual mask testing. Use it.
No functional changes.
--- kernel/cgroup.c | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-)
kernel/cgroup.c +12 -23
@@ -3082,10 +3082,7 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of,* dependency. An invisible css is made visible when the userland * explicitly enables it. */ -for_each_subsys (ss, ssid) { -if (!(enable & (1 << ssid))) -continue ; - + do_each_subsys_mask(ss, ssid, enable) {cgroup_for_each_live_child (child, cgrp) {if (css_enable & (1 << ssid)) ret = create_css(child, ss,
@@ -3096,7 +3093,7 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of,if (ret) goto err_undo_css; } - } + }while_each_subsys_mask ();/* * At this point, cgroup_e_css() results reflect the new csses
@@ -3115,10 +3112,7 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of,* state if it's made visible again later. Controllers which may * be depended upon should provide ->css_reset() for this purpose. */ -for_each_subsys (ss, ssid) { -if (!(disable & (1 << ssid))) -continue ; - + do_each_subsys_mask(ss, ssid, disable) {cgroup_for_each_live_child (child, cgrp) {struct cgroup_subsys_state *css =cgroup_css (child, ss);
@@ -3130,7 +3124,7 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of, ss->css_reset (css); } } - } + }while_each_subsys_mask ();kernfs_activate (cgrp->kn); ret =0 ;
@@ -3142,10 +3136,7 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of, cgrp->subtree_control = old_sc; cgrp->subtree_ss_mask = old_ss; -for_each_subsys (ss, ssid) { -if (!(enable & (1 << ssid))) -continue ; - + do_each_subsys_mask(ss, ssid, enable) {cgroup_for_each_live_child (child, cgrp) {struct cgroup_subsys_state *css =cgroup_css (child, ss);
@@ -3157,7 +3148,7 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of,else css_clear_dir (css,NULL ); } - } + }while_each_subsys_mask (); goto out_unlock; }
@@ -4973,14 +4964,12 @@ static int cgroup_mkdir(struct kernfs_node *parent_kn, const char *name, goto out_destroy;/* let's create and online css's */ -for_each_subsys (ss, ssid) { -if (parent->subtree_ss_mask & (1 << ssid)) { - ret =create_css (cgrp, ss, - parent->subtree_control & (1 << ssid)); -if (ret) - goto out_destroy; - } - } + do_each_subsys_mask(ss, ssid, parent->subtree_ss_mask) { + ret =create_css (cgrp, ss, + parent->subtree_control & (1 << ssid)); +if (ret) + goto out_destroy; + }while_each_subsys_mask ();/* * On the default hierarchy, a child doesn't automatically inherit
Signature · 1 line
2.5.0
Tejun Heo
maintainerPATCH 6/6 cgroup: make cgroup subsystem masks u16
In reply to Tejun HeoAfter the recent do_each_subsys_mask() conversion, there's no reason to use ulong for subsystem masks. We'll be adding more subsystem masks to persistent data structures, let's reduce its size to u16 which should be enough for now and the foreseeable future.
This doesn't create any noticeable behavior differences.
--- include/linux/cgroup-defs.h | 4 ++-- kernel/cgroup.c | 46 ++++++++++++++++++++++----------------------- 2 files changed, 24 insertions(+), 26 deletions(-)
include/linux/cgroup-defs.h +2 -2
@@ -257,8 +257,8 @@ struct cgroup {* one which may have more subsystems enabled. Controller knobs * are made available iff it's enabled in ->subtree_control. */ -unsigned long subtree_control; -unsigned long subtree_ss_mask; +u16 subtree_control; +u16 subtree_ss_mask;/* Private pointers for each registered subsystem */ struct cgroup_subsys_state __rcu *subsys[CGROUP_SUBSYS_COUNT ];
kernel/cgroup.c +22 -24
147 lines not on this page · raw message
Signature · 1 line
2.5.0
Johannes Weiner
maintainerRe: [PATCH 6/6] cgroup: make cgroup subsystem masks u16
kernel/cgroup.c static bool cgrp_dfl_root_visible; · line 184
Quoted 4 lines · Tejun Heo
kernel/cgroup.c +0
-0@@ -184,7 +184,7 @@ static bool cgrp_dfl_root_visible;static unsigned long cgroup_no_v1_mask;
^ This one can be turned into a u16 too.
Tejun Heo
maintainerPATCH v2 6/6 cgroup: make cgroup subsystem masks u16
In reply to Tejun HeoFrom 6e5c830770f9045a17b1b931c3e11fbd5591e630 Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Mon, 22 Feb 2016 22:25:47 -0500
After the recent do_each_subsys_mask() conversion, there's no reason to use ulong for subsystem masks. We'll be adding more subsystem masks to persistent data structures, let's reduce its size to u16 which should be enough for now and the foreseeable future.
This doesn't create any noticeable behavior differences.
v2: Johannes spotted that the initial patch missed cgroup_no_v1_mask.
Converted.
--- include/linux/cgroup-defs.h | 4 ++-- kernel/cgroup.c | 50 ++++++++++++++++++++++----------------------- 2 files changed, 26 insertions(+), 28 deletions(-)
include/linux/cgroup-defs.h +2 -2
@@ -257,8 +257,8 @@ struct cgroup {* one which may have more subsystems enabled. Controller knobs * are made available iff it's enabled in ->subtree_control. */ -unsigned long subtree_control; -unsigned long subtree_ss_mask; +u16 subtree_control; +u16 subtree_ss_mask;/* Private pointers for each registered subsystem */ struct cgroup_subsys_state __rcu *subsys[CGROUP_SUBSYS_COUNT ];
kernel/cgroup.c +24 -26
159 lines not on this page · raw message
Signature · 1 line
2.5.0
Quoted 3 lines · Tejun Heo
This patchset contains the following six patches. The first one is a fix but should be safe to route through for-4.6. The rest are misc cleanups and improvements which don't cause notable behavior changes.
These look good to me.
Tejun Heo
maintainerQuoted 6 lines · Tejun Heo
0001-cgroup-fix-error-return-value-of-cgroup_addrm_files.patch 0002-Revert-cgroup-add-cgroup_subsys-css_e_css_changed.patch 0003-cgroup-s-child_subsys_mask-subtree_ss_mask.patch 0004-cgroup-convert-for_each_subsys_which-to-do-while-sty.patch 0005-cgroup-use-do_each_subsys_mask-where-applicable.patch 0006-cgroup-make-cgroup-subsystem-masks-u16.patch
Applied 1-6 to cgroup/for-4.6.
Signature · 1 line
tejun
Quoted 17 lines · Tejun Heo
Hello,
This patchset contains the following six patches. The first one is a fix but should be safe to route through for-4.6. The rest are misc cleanups and improvements which don't cause notable behavior changes.
0001-cgroup-fix-error-return-value-of-cgroup_addrm_files.patch 0002-Revert-cgroup-add-cgroup_subsys-css_e_css_changed.patch 0003-cgroup-s-child_subsys_mask-subtree_ss_mask.patch 0004-cgroup-convert-for_each_subsys_which-to-do-while-sty.patch 0005-cgroup-use-do_each_subsys_mask-where-applicable.patch 0006-cgroup-make-cgroup-subsystem-masks-u16.patch
The patchset based on top of cgroup/for-4.6
223ffb29f972("cgroup:
provide cgroup_nov1= to disable controllers in v1 mounts") and is available in the following git branch.
Quoted 9 lines
git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.gitreview-miscdiffstat follows. I'll pull in the branch into cgroup/for-4.6 soonish.
include/linux/cgroup-defs.h | 12 +- kernel/cgroup.c | 217 +++++++++++++++++++------------------------- 2 files changed, 102 insertions(+), 127 deletions(-)