Currently if there is a population error, we can end up with partially applied mask, which is not what we want, let's do a proper cleanup.
Split cgroup_apply_hidden() into two separate helpers for showing and hiding: cgroup_unhide() and cgroup_hide(). As cgroup_hide() never fails we can easily use it for both hiding on success path and cleanup on error path of cgroup_unhide() with restored old mask. While on it let's add small comments to those helpers. https://virtuozzo.atlassian.net/browse/VSTOR-119803 Fixes: fbc4461a215c ("cgroup-v2: Add a new API to hide cgroup files per controller") Co-developed-by: Konstantin Khorenko <[email protected]> Signed-off-by: Pavel Tikhomirov <[email protected]> Feature: ve: ve generic structures --- Note: There was also an option to use cgroup_apply_hidden() directly for cleanup in similar manner, but then there may be some tricky case where for some reason restored old mask was "incorrect" and that would interfere with cleanup, so let's use a more straight forward cleanup. --- kernel/cgroup/cgroup.c | 52 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index eb2b64f7da37..92abecb000b1 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -3896,7 +3896,11 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of, return ret ?: nbytes; } -static int cgroup_apply_hidden(struct cgroup *cgrp) +/* + * cgroup_unhide() - Apply cleared bits in hidden_ss_mask to the cgroup + * directory by populating files for visible controllers. + */ +static int cgroup_unhide(struct cgroup *cgrp) { struct cgroup_subsys *ss; int ssid; @@ -3916,14 +3920,36 @@ static int cgroup_apply_hidden(struct cgroup *cgrp) ret = css_populate_dir(css); if (ret) return ret; - } else { + } + } + + return 0; +} + +/* + * cgroup_hide() - Apply set bits in hidden_ss_mask to the cgroup + * directory by clearing files for hidden controllers. + */ +static void cgroup_hide(struct cgroup *cgrp) +{ + struct cgroup_subsys *ss; + int ssid; + + for_each_subsys(ss, ssid) { + struct cgroup_subsys_state *css = cgroup_css(cgrp, ss); + + if (!(cgroup_ss_mask(cgrp) & (1 << ss->id))) + continue; + + if (!css) + continue; + + if (!css_visible(css)) { css_clear_dir(css); if (ss->css_reset) ss->css_reset(css); } } - - return 0; } /* change the hidden controllers for a cgroup in the default hierarchy */ @@ -3933,6 +3959,7 @@ static ssize_t cgroup_controllers_hidden_write(struct kernfs_open_file *of, { struct cgroup_subsys *ss; u16 hide = 0, show = 0; + u16 old_hidden_ss_mask; struct cgroup *cgrp; int ssid, ret; char *tok; @@ -3988,10 +4015,23 @@ static ssize_t cgroup_controllers_hidden_write(struct kernfs_open_file *of, goto out_unlock; } - cgrp->hidden_ss_mask |= hide; + /* + * This is similar to cgroup_apply_control_enable(). We first try to + * populate dirs for controllers being shown, and on success, hide the + * controllers being hidden. On error we restore the old mask and hide + * any dirs that might have been populated in cgroup_unhide(). + */ + old_hidden_ss_mask = cgroup_hidden_ss_mask(cgrp); cgrp->hidden_ss_mask &= ~show; - ret = cgroup_apply_hidden(cgrp); + ret = cgroup_unhide(cgrp); + if (ret) + /* Restore old mask on failure */ + cgrp->hidden_ss_mask = old_hidden_ss_mask; + else + cgrp->hidden_ss_mask |= hide; + + cgroup_hide(cgrp); if (ret) goto out_unlock; -- 2.52.0 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
