Re: [PATCH 1/2] cgroup: revise how we re-populate root directory

[Sha]

Hi Zefan,

On Mon, Jan 16, 2012 at 4:06 PM, Li Zefan <lizf@xxxxxxxxxxxxxx> wrote:
> When remounting cgroupfs with some subsystems added to it and some
> removed, cgroup will remove all the files in root directory and then
> re-popluate it.
>
> What I'm doing here is, only remove files which belong to subsystems that
> are to be unbound, and only create files for newly-added subsystems.
> The purpose is to have all other files untouched.
>
> This is a preparation for cgroup xattr support.
>
> Signed-off-by: Li Zefan <lizf@xxxxxxxxxxxxxx>
> ---
> Âinclude/linux/cgroup.h | Â 11 +++---
> Âkernel/cgroup.c    Â|  92 +++++++++++++++++++++++++++++++----------------
> Â2 files changed, 67 insertions(+), 36 deletions(-)
>
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index e9b6021..13db9e8 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -327,6 +327,9 @@ struct cftype {
> Â Â Â Â */
> Â Â Â Âsize_t max_write_len;
>
> + Â Â Â /* The subsystem this cgroup file belongs to */
> + Â Â Â struct cgroup_subsys *subsys;
> +
> Â Â Â Âint (*open)(struct inode *inode, struct file *file);
> Â Â Â Âssize_t (*read)(struct cgroup *cgrp, struct cftype *cft,
> Â Â Â Â Â Â Â Â Â Â Â Âstruct file *file,
> @@ -419,16 +422,14 @@ struct cgroup_scanner {
> Â* called by subsystems from within a populate() method
> Â*/
> Âint cgroup_add_file(struct cgroup *cgrp, struct cgroup_subsys *subsys,
> - Â Â Â Â Â Â Â Â Â Â Âconst struct cftype *cft);
> + Â Â Â Â Â Â Â Â Â struct cftype *cft);
>
> Â/*
> Â* Add a set of new files to the given cgroup directory. Should
> Â* only be called by subsystems from within a populate() method
> Â*/
> -int cgroup_add_files(struct cgroup *cgrp,
> - Â Â Â Â Â Â Â Â Â Â Â struct cgroup_subsys *subsys,
> - Â Â Â Â Â Â Â Â Â Â Â const struct cftype cft[],
> - Â Â Â Â Â Â Â Â Â Â Â int count);
> +int cgroup_add_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
> + Â Â Â Â Â Â Â Â Â Âstruct cftype cft[], int count);
>
> Âint cgroup_is_removed(const struct cgroup *cgrp);
>
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index a5d3b53..c4ed6fe 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -781,6 +781,8 @@ static int cgroup_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode);
> Âstatic struct dentry *cgroup_lookup(struct inode *, struct dentry *, struct nameidata *);
> Âstatic int cgroup_rmdir(struct inode *unused_dir, struct dentry *dentry);
> Âstatic int cgroup_populate_dir(struct cgroup *cgrp);
> +static int cgroup_repopulate_dir(struct cgroup *cgrp, unsigned long added_bits,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long removed_bits);
> Âstatic const struct inode_operations cgroup_dir_inode_operations;
> Âstatic const struct file_operations proc_cgroupstats_operations;
>
> @@ -882,34 +884,44 @@ static void remove_dir(struct dentry *d)
> Â Â Â Âdput(parent);
> Â}
>
> -static void cgroup_clear_directory(struct dentry *dentry)
> +static void cgroup_clear_directory(struct dentry *dentry, bool remove_all,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long removed_bits)
> Â{
> - Â Â Â struct list_head *node;
> + Â Â Â LIST_HEAD(head);
> + Â Â Â struct dentry *d, *node;
>
> Â Â Â ÂBUG_ON(!mutex_is_locked(&dentry->d_inode->i_mutex));
> +
> Â Â Â Âspin_lock(&dentry->d_lock);
> - Â Â Â node = dentry->d_subdirs.next;
> - Â Â Â while (node != &dentry->d_subdirs) {
> - Â Â Â Â Â Â Â struct dentry *d = list_entry(node, struct dentry, d_u.d_child);
> + Â Â Â list_for_each_entry_safe(d, node, &dentry->d_subdirs, d_u.d_child) {
> + Â Â Â Â Â Â Â struct cftype *cft = d->d_fsdata;
> +
> + Â Â Â Â Â Â Â if (!remove_all && cft->subsys &&
> + Â Â Â Â Â Â Â Â Â !test_bit(cft->subsys->subsys_id, &removed_bits))
> + Â Â Â Â Â Â Â Â Â Â Â continue;
>
> Â Â Â Â Â Â Â Âspin_lock_nested(&d->d_lock, DENTRY_D_LOCK_NESTED);
> - Â Â Â Â Â Â Â list_del_init(node);
> + Â Â Â Â Â Â Â list_move(&d->d_u.d_child, &head);
> Â Â Â Â Â Â Â Âif (d->d_inode) {
> Â Â Â Â Â Â Â Â Â Â Â Â/* This should never be called on a cgroup
> Â Â Â Â Â Â Â Â Â Â Â Â * directory with child cgroups */
> Â Â Â Â Â Â Â Â Â Â Â ÂBUG_ON(d->d_inode->i_mode & S_IFDIR);
> Â Â Â Â Â Â Â Â Â Â Â Âdget_dlock(d);
> - Â Â Â Â Â Â Â Â Â Â Â spin_unlock(&d->d_lock);
> - Â Â Â Â Â Â Â Â Â Â Â spin_unlock(&dentry->d_lock);
> + Â Â Â Â Â Â Â }
> + Â Â Â Â Â Â Â spin_unlock(&d->d_lock);
> + Â Â Â }
> + Â Â Â spin_unlock(&dentry->d_lock);
> +
> + Â Â Â list_for_each_entry_safe(d, node, &head, d_u.d_child) {
> + Â Â Â Â Â Â Â spin_lock(&d->d_lock);
> + Â Â Â Â Â Â Â list_del_init(&d->d_u.d_child);
> + Â Â Â Â Â Â Â spin_unlock(&d->d_lock);
> + Â Â Â Â Â Â Â if (d->d_inode) {
> Â Â Â Â Â Â Â Â Â Â Â Âd_delete(d);
> Â Â Â Â Â Â Â Â Â Â Â Âsimple_unlink(dentry->d_inode, d);
> Â Â Â Â Â Â Â Â Â Â Â Âdput(d);
> - Â Â Â Â Â Â Â Â Â Â Â spin_lock(&dentry->d_lock);
> - Â Â Â Â Â Â Â } else
> - Â Â Â Â Â Â Â Â Â Â Â spin_unlock(&d->d_lock);
> - Â Â Â Â Â Â Â node = dentry->d_subdirs.next;
> + Â Â Â Â Â Â Â }
> Â Â Â Â}
> - Â Â Â spin_unlock(&dentry->d_lock);
> Â}
>
> Â/*
> @@ -919,7 +931,7 @@ static void cgroup_d_remove_dir(struct dentry *dentry)
> Â{
> Â Â Â Âstruct dentry *parent;
>
> - Â Â Â cgroup_clear_directory(dentry);
> + Â Â Â cgroup_clear_directory(dentry, true, 0);
>
> Â Â Â Âparent = dentry->d_parent;
> Â Â Â Âspin_lock(&parent->d_lock);
> @@ -1284,6 +1296,7 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
> Â Â Â Âstruct cgroupfs_root *root = sb->s_fs_info;
> Â Â Â Âstruct cgroup *cgrp = &root->top_cgroup;
> Â Â Â Âstruct cgroup_sb_opts opts;
> + Â Â Â unsigned long added_bits, removed_bits;
>
> Â Â Â Âmutex_lock(&cgrp->dentry->d_inode->i_mutex);
> Â Â Â Âmutex_lock(&cgroup_mutex);
> @@ -1294,6 +1307,9 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
> Â Â Â Âif (ret)
> Â Â Â Â Â Â Â Âgoto out_unlock;
>
> + Â Â Â added_bits = opts.subsys_bits & ~root->subsys_bits;
> + Â Â Â removed_bits = root->subsys_bits & ~opts.subsys_bits;
> +

Should it be the followingïï
          added_bits = opts.subsys_bits & ~root->actual_subsys_bits;
          removed_bits = root->actual_subsys_bits & ~opts.subsys_bits;

Thanks,
Sha

> Â Â Â Â/* Don't allow flags or name to change at remount */
> Â Â Â Âif (opts.flags != root->flags ||
> Â Â Â Â Â Â(opts.name && strcmp(opts.name, root->name))) {
> @@ -1308,8 +1324,8 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
> Â Â Â Â Â Â Â Âgoto out_unlock;
> Â Â Â Â}
>
> - Â Â Â /* (re)populate subsystem files */
> - Â Â Â cgroup_populate_dir(cgrp);
> + Â Â Â /* re-populate subsystem files */
> + Â Â Â cgroup_repopulate_dir(cgrp, added_bits, removed_bits);
>
> Â Â Â Âif (opts.release_agent)
> Â Â Â Â Â Â Â Âstrcpy(root->release_agent_path, opts.release_agent);
> @@ -2710,16 +2726,17 @@ static umode_t cgroup_file_mode(const struct cftype *cft)
> Â Â Â Âreturn mode;
> Â}
>
> -int cgroup_add_file(struct cgroup *cgrp,
> - Â Â Â Â Â Â Â Â Â Â Âstruct cgroup_subsys *subsys,
> - Â Â Â Â Â Â Â Â Â Â Âconst struct cftype *cft)
> +int cgroup_add_file(struct cgroup *cgrp, struct cgroup_subsys *subsys,
> + Â Â Â Â Â Â Â Â Â struct cftype *cft)
> Â{
> Â Â Â Âstruct dentry *dir = cgrp->dentry;
> Â Â Â Âstruct dentry *dentry;
> Â Â Â Âint error;
> Â Â Â Âumode_t mode;
> -
> Â Â Â Âchar name[MAX_CGROUP_TYPE_NAMELEN + MAX_CFTYPE_NAME + 2] = { 0 };
> +
> + Â Â Â cft->subsys = subsys;
> +
> Â Â Â Âif (subsys && !test_bit(ROOT_NOPREFIX, &cgrp->root->flags)) {
> Â Â Â Â Â Â Â Âstrcpy(name, subsys->name);
> Â Â Â Â Â Â Â Âstrcat(name, ".");
> @@ -2740,10 +2757,8 @@ int cgroup_add_file(struct cgroup *cgrp,
> Â}
> ÂEXPORT_SYMBOL_GPL(cgroup_add_file);
>
> -int cgroup_add_files(struct cgroup *cgrp,
> - Â Â Â Â Â Â Â Â Â Â Â struct cgroup_subsys *subsys,
> - Â Â Â Â Â Â Â Â Â Â Â const struct cftype cft[],
> - Â Â Â Â Â Â Â Â Â Â Â int count)
> +int cgroup_add_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
> + Â Â Â Â Â Â Â Â Â Âstruct cftype cft[], int count)
> Â{
> Â Â Â Âint i, err;
> Â Â Â Âfor (i = 0; i < count; i++) {
> @@ -3703,26 +3718,27 @@ static struct cftype cft_release_agent = {
> Â Â Â Â.max_write_len = PATH_MAX,
> Â};
>
> -static int cgroup_populate_dir(struct cgroup *cgrp)
> +static int __cgroup_populate_dir(struct cgroup *cgrp, unsigned long added_bits)
> Â{
> Â Â Â Âint err;
> Â Â Â Âstruct cgroup_subsys *ss;
>
> - Â Â Â /* First clear out any existing files */
> - Â Â Â cgroup_clear_directory(cgrp->dentry);
> -
> Â Â Â Âerr = cgroup_add_files(cgrp, NULL, files, ARRAY_SIZE(files));
> Â Â Â Âif (err < 0)
> Â Â Â Â Â Â Â Âreturn err;
>
> Â Â Â Âif (cgrp == cgrp->top_cgroup) {
> - Â Â Â Â Â Â Â if ((err = cgroup_add_file(cgrp, NULL, &cft_release_agent)) < 0)
> + Â Â Â Â Â Â Â err = cgroup_add_file(cgrp, NULL, &cft_release_agent);
> + Â Â Â Â Â Â Â if (err < 0)
> Â Â Â Â Â Â Â Â Â Â Â Âreturn err;
> Â Â Â Â}
>
> Â Â Â Âfor_each_subsys(cgrp->root, ss) {
> - Â Â Â Â Â Â Â if (ss->populate && (err = ss->populate(ss, cgrp)) < 0)
> - Â Â Â Â Â Â Â Â Â Â Â return err;
> + Â Â Â Â Â Â Â if (test_bit(ss->subsys_id, &added_bits) && ss->populate) {
> + Â Â Â Â Â Â Â Â Â Â Â err = ss->populate(ss, cgrp);
> + Â Â Â Â Â Â Â Â Â Â Â if (err < 0)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return err;
> + Â Â Â Â Â Â Â }
> Â Â Â Â}
> Â Â Â Â/* This cgroup is ready now */
> Â Â Â Âfor_each_subsys(cgrp->root, ss) {
> @@ -3739,6 +3755,20 @@ static int cgroup_populate_dir(struct cgroup *cgrp)
> Â Â Â Âreturn 0;
> Â}
>
> +static int cgroup_populate_dir(struct cgroup *cgrp)
> +{
> + Â Â Â return __cgroup_populate_dir(cgrp, cgrp->root->subsys_bits);
> +}
> +
> +static int cgroup_repopulate_dir(struct cgroup *cgrp, unsigned long added_bits,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long removed_bits)
> +{
> + Â Â Â /* First clear out files */
> + Â Â Â cgroup_clear_directory(cgrp->dentry, false, removed_bits);
> +
> + Â Â Â return __cgroup_populate_dir(cgrp, added_bits);
> +}
> +
> Âstatic void init_cgroup_css(struct cgroup_subsys_state *css,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct cgroup_subsys *ss,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct cgroup *cgrp)
> --
> 1.7.3.1
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/