cgroup: revise how we re-populate root directory
PATCH 1/2 cgroup: revise how we re-populate root directory
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.
--- include/linux/cgroup.h | 11 +++--- kernel/cgroup.c | 92 +++++++++++++++++++++++++++++++---------------- 2 files changed, 67 insertions(+), 36 deletions(-)
include/linux/cgroup.h +6 -5
@@ -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);
kernel/cgroup.c +61 -31
186 lines not on this page · raw message
Signature · 1 line
1.7.3.1
PATCH 2/2 cgroup: add xattr support
This is one of the items in the plumber's wish list.
For use cases:
Quoted 17 lines
Quoted 1 line
What would the use case be for this?
Attaching meta information to services, in an easily discoverable way. For example, in systemd we create one cgroup for each service, and could then store data like the main pid of the specific service as an xattr on the cgroup itself. That way we'd have almost all service state in the cgroupfs, which would make it possible to terminate systemd and later restart it without losing any state information. But there's more:
for example, some very peculiar services cannot be terminated on shutdown (i.e. fakeraid DM stuff) and it would be really nice if the services in question could just mark that on their cgroup, by setting an xattr. On the more desktopy side of things there are other
possibilities: for example there are plans defining what an application is along the lines of a cgroup (i.e. an app being a collection of processes). With xattrs one could then attach an icon or human readable program name on the cgroup.
Quoted 8 lines
The key idea is that this would allow attaching runtime meta information to cgroups and everything they model (services, apps, vms), that doesn't need any complex userspace infrastructure, has good access control (i.e. because the file system enforces that anyway, and there's the "trusted." xattr namespace), notifications (inotify), and can easily be shared among applications.
Lennart
--- include/linux/cgroup.h | 15 +++ init/Kconfig | 12 ++ kernel/cgroup.c | 272 ++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 289 insertions(+), 10 deletions(-)
include/linux/cgroup.h +15 -0
@@ -16,6 +16,8 @@#include <linux/prio_heap.h> #include <linux/rwsem.h> #include <linux/idr.h> +#include <linux/rbtree.h> +#include <linux/spinlock.h> #ifdef CONFIG_CGROUPS
@@ -42,6 +44,13 @@ extern void cgroup_unload_subsys(struct cgroup_subsys *ss);extern const struct file_operations proc_cgroup_operations; +struct cgroup_xattr_root { +#ifdef CONFIG_CGROUP_XATTR +struct rb_root root; +spinlock_t lock; +#endif +}; +/* Define the enumeration of all builtin cgroup subsystems */ #define SUBSYS (_x) _x ## _subsys_id,enum cgroup_subsys_id {
@@ -243,6 +252,9 @@ struct cgroup {/* List of events which userspace want to receive */ struct list_head event_list;spinlock_t event_list_lock; + +/* directory xattrs */ +struct cgroup_xattr_root xattr_root; };/*
@@ -330,6 +342,9 @@ struct cftype {/* The subsystem this cgroup file belongs to */ struct cgroup_subsys *subsys; +/* file xattrs */ +struct cgroup_xattr_root xattr_root; +int (*open)(struct inode *inode,struct file *file); ssize_t (*read)(struct cgroup *cgrp, struct cftype *cft, struct file *file,
init/Kconfig +12 -0
@@ -587,6 +587,18 @@ menuconfig CGROUPS if CGROUPS +config CGROUP_XATTR + bool "Cgroup extended attributes" + default n + help + Extended attributes are name:value pairs associated with inodes by + the kernel or by users (see the attr(5) manual page, or visit + <http://acl.bestbits.at/> for details). + + Currently the system.* namespace is not supported. + + If unsure, say N. + config CGROUP_DEBUG bool "Example debug cgroup subsystem" default n
kernel/cgroup.c +262 -10
324 lines not on this page · raw message
Signature · 1 line
1.7.3.1
Tejun Heo
maintainerRe: [PATCH 2/2] cgroup: add xattr support
Hello,
Quoted 28 lines · Li Zefan
This is one of the items in the plumber's wish list.
For use cases:
Quoted 24 lines
Quoted 1 line
What would the use case be for this?
Attaching meta information to services, in an easily discoverable way. For example, in systemd we create one cgroup for each service, and could then store data like the main pid of the specific service as an xattr on the cgroup itself. That way we'd have almost all service state in the cgroupfs, which would make it possible to terminate systemd and later restart it without losing any state information. But there's more:
for example, some very peculiar services cannot be terminated on shutdown (i.e. fakeraid DM stuff) and it would be really nice if the services in question could just mark that on their cgroup, by setting an xattr. On the more desktopy side of things there are other
possibilities: for example there are plans defining what an application is along the lines of a cgroup (i.e. an app being a collection of processes). With xattrs one could then attach an icon or human readable program name on the cgroup.The key idea is that this would allow attaching runtime meta information to cgroups and everything they model (services, apps, vms), that doesn't need any complex userspace infrastructure, has good access control (i.e. because the file system enforces that anyway, and there's the "trusted." xattr namespace), notifications (inotify), and can easily be shared among applications.
Quoted 3 lines
Quoted 1 line
Lennart
Signed-off-by Li Zefan <[email protected]>
Ummm... I don't feel too good about this. Why can't those extra information be kept somewhere else? Overloading cgroupfs as storage for essentially opaque userland information doesn't seem like a good idea to me.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] cgroup: add xattr support
Quoted 31 lines · Tejun Heo
Hello,
Quoted 28 lines · Li Zefan
This is one of the items in the plumber's wish list.
For use cases:
Quoted 24 lines
Quoted 1 line
What would the use case be for this?
Attaching meta information to services, in an easily discoverable way. For example, in systemd we create one cgroup for each service, and could then store data like the main pid of the specific service as an xattr on the cgroup itself. That way we'd have almost all service state in the cgroupfs, which would make it possible to terminate systemd and later restart it without losing any state information. But there's more:
for example, some very peculiar services cannot be terminated on shutdown (i.e. fakeraid DM stuff) and it would be really nice if the services in question could just mark that on their cgroup, by setting an xattr. On the more desktopy side of things there are other
possibilities: for example there are plans defining what an application is along the lines of a cgroup (i.e. an app being a collection of processes). With xattrs one could then attach an icon or human readable program name on the cgroup.The key idea is that this would allow attaching runtime meta information to cgroups and everything they model (services, apps, vms), that doesn't need any complex userspace infrastructure, has good access control (i.e. because the file system enforces that anyway, and there's the "trusted." xattr namespace), notifications (inotify), and can easily be shared among applications.
Quoted 9 lines
Quoted 3 lines
Quoted 1 line
Lennart
Signed-off-by Li Zefan <[email protected]>Ummm... I don't feel too good about this. Why can't those extra information be kept somewhere else? Overloading cgroupfs as storage for essentially opaque userland information doesn't seem like a good idea to me.
Long ago Paul M toyed with a patch that adds a control file for userspace to read/write per-cgroup user information, but there were no use cases. This patchset has a similar purpose, but this interface is more flexable and easier to use, and we do have systemd as a use case this time.
I'll let Lennart answer if we can easily live without this.
Furthermore, I noticed tmpfs also implemented xattr support, and we should be able to share most of the code, which makes the cost for having this feature smaller.
Tejun Heo
maintainerRe: [PATCH 2/2] cgroup: add xattr support
Hello,
Quoted 3 lines · Li Zefan
Furthermore, I noticed tmpfs also implemented xattr support, and we should be able to share most of the code, which makes the cost for having this feature smaller.
Yes, maybe, but tmpfs doesn't have any special semantics attached to files. It's supposed to store opaque data from userland. Regardless of code complexity, I just don't think this is a good idea. What if we later want to attach certain config meaning to or export statistics via xattrs? That would be quite unusual and require strong rationale but at least fall in the similar usage as with the rest of the fs.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] cgroup: add xattr support
Quoted 8 lines · Tejun Heo
Hello,
Quoted 3 lines · Li Zefan
Furthermore, I noticed tmpfs also implemented xattr support, and we should be able to share most of the code, which makes the cost for having this feature smaller.
Quoted 6 lines
Yes, maybe, but tmpfs doesn't have any special semantics attached to files. It's supposed to store opaque data from userland. Regardless of code complexity, I just don't think this is a good idea. What if we later want to attach certain config meaning to or export statistics via xattrs? That would be quite unusual and require strong rationale but at least fall in the similar usage as with the rest of the fs.
If you plan to export kernel data to userspace via xattrs, then you should probably place that in the system.xxx xattr namespace, not in the user.xxx or trusted.xxx namespaces which are the only ones which can be manipulated by userspace.
See attr(5) for more information about the four available xattr namespaces.
In other words: kernel exported xattrs should never collide with xattrs set by userspace.
Lennart
Signature · 1 line
Lennart Poettering - Red Hat, Inc.
Re: [PATCH 2/2] cgroup: add xattr support
In reply to Tejun HeoQuoted 11 lines · Tejun Heo
Quoted 6 lines
Quoted 6 lines
The key idea is that this would allow attaching runtime meta information to cgroups and everything they model (services, apps, vms), that doesn't need any complex userspace infrastructure, has good access control (i.e. because the file system enforces that anyway, and there's the "trusted." xattr namespace), notifications (inotify), and can easily be shared among applications.
Ummm... I don't feel too good about this. Why can't those extra information be kept somewhere else? Overloading cgroupfs as storage for essentially opaque userland information doesn't seem like a good idea to me.
A bit of the background:
The /sys/fs/cgroup/systemd/ hierarchy represents something like a first class kernel-related system object, which we call an 'application' or 'service' in userspace. Every 'service' or 'application' has zero, one or many processes, and has some high-level metadata attached.
The general idea, not specifcally related to cgroups, is to try as hard as possible to stick metadata about objects to the objects themselves. It's a bit like the difference of lock files vs. locks attached to a process. In the later case, if the process dies, the lock is automatically cleaned up, in the former, we have a stale file.
The idea with the cgroup fs xattrs was to be able to attach some general useful attributes to the 'service container' itself, instead of keeping them in the memory of the managing process or store them on disk which can get out-of-sync much easier.
Kay
Tejun Heo
maintainerRe: [PATCH 2/2] cgroup: add xattr support
Hello,
Quoted 4 lines · Kay Sievers
The idea with the cgroup fs xattrs was to be able to attach some general useful attributes to the 'service container' itself, instead of keeping them in the memory of the managing process or store them on disk which can get out-of-sync much easier.
Hmmm.... I can see the attraction but there really is nothing which binds that information to cgroup. The same information might as well live in /proc/PID/userland_data or whatever. It may be convenient now but I'm pretty skeptical it's a good idea in the long run.
Given that cgroups themselves need to be explicitly created and destroyed, maintaining a parallel tmpfs hierarchy for metadata, if necessary, shouldn't be too bothersome, right?
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] cgroup: add xattr support
Quoted 9 lines · Tejun Heo
Hello,
Quoted 4 lines · Kay Sievers
The idea with the cgroup fs xattrs was to be able to attach some general useful attributes to the 'service container' itself, instead of keeping them in the memory of the managing process or store them on disk which can get out-of-sync much easier.
Quoted 8 lines
Hmmm.... I can see the attraction but there really is nothing which binds that information to cgroup. The same information might as well live in
/proc/PID/userland_dataor whatever. It may be convenient now but I'm pretty skeptical it's a good idea in the long run.Given that cgroups themselves need to be explicitly created and destroyed, maintaining a parallel tmpfs hierarchy for metadata, if necessary, shouldn't be too bothersome, right?
Well, the interesting bit here is that to make things robust we'd like to attach the meta information userspace needs to the object itself, so that it follows the same lifecycle, and we can detect changes to it with the usual APIs such as inotify, without any complex logic in userspace that tries to ensure that meta data stored elsewhere is always kept in sync with what cgroups are currently around.
I mean, certainly, it's possible to store this data elsewhere, and that's what all current cgroup client code does, including systemd for example. But ultimately that is very fragile and cumbersome, since it requires us to proxy all cgroup events to this meta information, emulating in userspace that we align the lifecycles of cgroups with the lifecycle of its metadata. It also makes it necessary to define new userspace APIs if we want different userspace components to share meta data on cgroups (and we do want to share metadata!), which is always difficult, and would look an awful lot like xattrs without actually being xattrs, but just suck as they'd be a userspace emulation of the real thing with all the complexities and problems it creates, for example the fact that they'd userspace emulated xattrs can only asynchronously follow the lifecycle of their cgroup, thus creating races one has to deal with and so on. Also, proper xattrs can distuingish "trusted.xxx" and "user.xxx" namespaces which influences access control on the xattr. Something like that is very useful but really hard to emulate without kernel support.
In summary: we want to be able to hook into the lifecycle of the cgroup right to make things robust. We want the xattrs to synchronously follow the lifecycle. We want a simple API that's already used for the same purpose on files. We don't want to to have to intrdouce fragile userspace code, that deals with races, and robustness, if we can trivially benefit from code that mostly already exists in the kernel.
Note that there already are a couple of attributes of cgroups userspace manipulates to encode meta data about services, for example via the mode/uid/gid of the cgroup dir and the tasks file. And people are actually already encoding information into those which better should be stored elsewhere, i.e. in my opinion the best place would be xattrs.
But yeah, to clarify this: this feature can be emulated in userspace, so this is not about making things possible that previously weren't. It's mostly about making things clean, safe, and robust which they previously weren't, without introducing non-trivial additional userspace components and interfaces.
Lennart
Signature · 1 line
Lennart Poettering - Red Hat, Inc.
Tejun Heo
maintainerRe: [PATCH 2/2] cgroup: add xattr support
Hello, Lennart, Li.
Two things.
* Probably I'm missing something but isn't the systemd cgroup hierarchy already managed by systemd? If so, I don't see how managing tmpfs on the side would noticeably make things more fragile. It would take a bit more care after, for example, restart but it shouldn't be too complex, no?
* FS attributes already being used for userland information seems like a good argument, but we shouldn't add separate specialized xattr implementation to different pseudo filesystems. For it to be acceptable, it should be a libfs thing easily applicable to any pseudo FS and definitely shouldn't be using kmem for storage.
Thanks.
Signature · 1 line
tejun
Tejun Heo
maintainerRe: [PATCH 2/2] cgroup: add xattr support
Quoted 10 lines · Tejun Heo
Hello, Lennart, Li.
Two things.
* Probably I'm missing something but isn't the systemd cgroup hierarchy already managed by systemd? If so, I don't see how managing tmpfs on the side would noticeably make things more fragile. It would take a bit more care after, for example, restart but it shouldn't be too complex, no?
Quoted 5 lines
* FS attributes already being used for userland information seems like a good argument, but we shouldn't add separate specialized xattr implementation to different pseudo filesystems. For it to be acceptable, it should be a libfs thing easily applicable to any pseudo FS and definitely shouldn't be using kmem for storage.
Also note that tmpfs also implies size limit. We definitely need some form of control over the amount of memory xattr may consume.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] cgroup: add xattr support
Quoted 12 lines · Tejun Heo
Quoted 10 lines · Tejun Heo
Hello, Lennart, Li.
Two things.
* Probably I'm missing something but isn't the systemd cgroup hierarchy already managed by systemd? If so, I don't see how managing tmpfs on the side would noticeably make things more fragile. It would take a bit more care after, for example, restart but it shouldn't be too complex, no?
Quoted 8 lines
Quoted 5 lines
* FS attributes already being used for userland information seems like a good argument, but we shouldn't add separate specialized xattr implementation to different pseudo filesystems. For it to be acceptable, it should be a libfs thing easily applicable to any pseudo FS and definitely shouldn't be using kmem for storage.
Also note that tmpfs also implies size limit. We definitely need some form of control over the amount of memory xattr may consume.
Good point. But then again we don't even have anything resembling for tmpfs either, where it would be much more important... :-(
Given that cgroupfs is probably mostly read-only for normal users, the requirement for quotas on it is probably much less important than for tmpfs though.
Lennart
Signature · 1 line
Lennart Poettering - Red Hat, Inc.
Re: [PATCH 2/2] cgroup: add xattr support
Quoted 8 lines · Lennart Poettering
Quoted 4 lines · Tejun Heo
On Wed, Jan 18, 2012 at 06:20:05PM -0800, Tejun Heo wrote:
Also note that tmpfs also implies size limit. We definitely need some form of control over the amount of memory xattr may consume.
Good point. But then again we don't even have anything resembling for tmpfs either, where it would be much more important... :-(
But although the tmpfs framework for xattrs is general, the validation function only permits XATTR_SECURITY_PREFIX and XATTR_TRUSTED_PREFIX.
Yes, it would be very necessary to impose size limits if tmpfs went beyond those to permit XATTR_USER_PREFIX - which the cgroupfs patch under discussion does permit.
Quoted 4 lines
Given that cgroupfs is probably mostly read-only for normal users, the requirement for quotas on it is probably much less important than for tmpfs though.
Hugh
Re: [PATCH 2/2] cgroup: add xattr support
In reply to Tejun HeoOn Wed, 18.01.12 18:20, Tejun Heo ([email protected]) wrote:
Heya,
Quoted 5 lines
* Probably I'm missing something but isn't the systemd cgroup hierarchy already managed by systemd? If so, I don't see how managing tmpfs on the side would noticeably make things more fragile. It would take a bit more care after, for example, restart but it shouldn't be too complex, no?
Well, it becomes more complex, for example, if we miss a cgroups event, or we go away and come back (i.e. systemd can reexecute itself for upgrades), and as soon as it isn't just systemd anymore which needs meta data attached to a cgroup, but we need something shared. For example, the logic described in
http://www.freedesktop.org/wiki/Software/systemd/PaxControlGroups
regarding "persistant" cgroups would be much nicer using xattrs (it currently misuses the sticky bit as flag instead).
It's just much nicer if meta data doesn't get stale just because the process owning it goes awol. And if process want to share meta data, too.
Quoted 5 lines
* FS attributes already being used for userland information seems like a good argument, but we shouldn't add separate specialized xattr implementation to different pseudo filesystems. For it to be acceptable, it should be a libfs thing easily applicable to any pseudo FS and definitely shouldn't be using kmem for storage.
I think the code that was recently added to tmpfs for the same purposes was supposed to be reusable on other virtual file systems. Might be worth looking into that.
Lennart
Signature · 1 line
Lennart Poettering - Red Hat, Inc.
Hi Zefan,
Quoted 139 lines · Li Zefan
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 <[email protected]>---
include/linux/cgroup.h | 11 +++---
kernel/cgroup.c | 92 +++++++++++++++++++++++++++++++----------------2 files changed, 67 insertions(+), 36 deletions(-)include/linux/cgroup.h +6
-5@@ -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);kernel/cgroup.c +27
-14@@ -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);
kernel/cgroup.c static int cgroup_remount(struct super_block *sb, int *flags, char *data) · line 1312
Quoted 6 lines
kernel/cgroup.c +3
-0@@ -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
Quoted 111 lines
kernel/cgroup.c +0
-0/* Don't allow flags or name to change at remount */if (opts.flags !=
root->flags|| (opts.name &&strcmp(opts.name, root->name))) {kernel/cgroup.c +31
-18@@ -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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 12 lines
Quoted 12 lines
kernel/cgroup.c +1
-0@@ -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;
Quoted 8 lines
kernel/cgroup.c static int cgroup_remount(struct super_block *sb, int *flags, char *data) · line 1312
Quoted 3 lines
kernel/cgroup.c +3
-0+ 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;
subsys_bits and actual_subsys_bits differ during cgroup_mount() only, and in other places they are replaceable with each other.
Thanks for looking into this.