Hello,
Sorry about the delay.
On Tue, Jul 10, 2012 at 03:27:22PM -0400, Aristeu Rozanski wrote:
> Index: xattr/include/linux/cgroup.h
> ===================================================================
> --- xattr.orig/include/linux/cgroup.h 2012-07-03 15:43:43.404334484 -0400
> +++ xattr/include/linux/cgroup.h 2012-07-10 13:17:32.285119770 -0400
> @@ -306,6 +306,9 @@
> */
> size_t max_write_len;
>
> + /* The subsystem this cgroup file belongs to */
> + struct cgroup_subsys *subsys;
I couldn't find how this is used. How is it used?
> Index: xattr/kernel/cgroup.c
> ===================================================================
> --- xattr.orig/kernel/cgroup.c 2012-07-03 15:43:43.404334484 -0400
> +++ xattr/kernel/cgroup.c 2012-07-10 13:28:18.424657727 -0400
> @@ -825,6 +825,7 @@
> 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);
> static const struct inode_operations cgroup_dir_inode_operations;
> static const struct file_operations proc_cgroupstats_operations;
>
> @@ -973,12 +974,23 @@
> return -ENOENT;
> }
>
> -static void cgroup_clear_directory(struct dentry *dir)
> +static void cgroup_clear_directory(struct dentry *dir, bool remove_all,
> + unsigned long removed_bits)
Maybe @subsys_mask is better? Also, why is @remove_all a separate
argument? Can't we define, say, CGRP_ALL_SUBSYS and pass it in as
@subsys_mask?
> {
> struct cgroup *cgrp = __d_cgrp(dir);
> + struct cgroup_subsys *ss;
>
> - while (!list_empty(&cgrp->files))
> - cgroup_rm_file(cgrp, NULL);
> + for_each_subsys(cgrp->root, ss) {
> + struct cftype_set *set;
> + if (!remove_all && !test_bit(ss->subsys_id, &removed_bits))
> + continue;
> + list_for_each_entry(set, &ss->cftsets, node)
> + cgroup_rm_file(cgrp, set->cfts);
> + }
> + if (remove_all) {
> + while (!list_empty(&cgrp->files))
> + cgroup_rm_file(cgrp, NULL);
> + }
Ah, I see. We don't have a bit for the base files. I think it's a
bit confusing that there are two params controlling subsys file
removal. Maybe reserve a bit for NULL subsys or use
@remove_base_files argument instead?
> +static int cgroup_populate_dir(struct cgroup *cgrp)
> +{
> + int err;
> +
> + err = cgroup_addrm_files(cgrp, NULL, files, true);
> + if (err < 0)
> + return err;
> +
> + return __cgroup_populate_dir(cgrp, cgrp->root->subsys_bits);
> +}
I think it would be best to keep the clear and populate interface more
or less symmetrical so that both have @subsys_mask (and maybe
@base_files).
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
