Re: [PATCH 0/4] cgroups: show correct file mode

Li Zefan
2009-03-03T01:07:42Z

Paul Menage wrote:
> On Sun, Mar 1, 2009 at 6:13 PM, Li Zefan <lizf@cn.fujitsu.com> wrote:
>> Now a cgroup subsystem can set default file mode of its control files,
>> so here is a patchset to correct file mode of each subsystem's files.
>
> I really think that we should be defaulting this based on whether the
> control file has read or write handlers.
>
> Sure, there are special cases like "tasks" that we'd need to set a
> manual value for, but most of these patches would be unnecessary.
>

Those patches are small and trivial, but it's ok for me to do this
automatically. How about below patch.

Note cpuset.memory_pressure is read-only though it has read handler.
Since if the read handler is removed, it'll return EINVAL instead of
the current EACCESS, I think it's better to leave as it is.

=====================

cgroups: show correct file mode

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
include/linux/cgroup.h | 5 +++++
kernel/cgroup.c | 30 ++++++++++++++++++++++++++++++
kernel/cpuset.c | 1 +
3 files changed, 36 insertions(+)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 6ad1989..af3c10f 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -258,6 +258,11 @@ struct cftype {
*/
char name[MAX_CFTYPE_NAME];
int private;
+ /*
+ * If not 0, file mode is set to this value, otherwise it will
+ * be figured out automatically
+ */
+ int mode;

/*
* If non-zero, defines the maximum length of string that can
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 379baa3..0b19204 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1750,6 +1750,33 @@ static int cgroup_create_dir(struct cgroup *cgrp, struct dentry *dentry,
return error;
}

+/**
+ * cgroup_file_mode - deduce file mode of a control file
+ * @cft: the control file in question
+ *
+ * returns cftype->mode if ->mode is not 0
+ * returns 0644 if it has both a read and a write handler
+ * returns 0444 if it has only a read handler
+ * returns 0200 if it has only a write hander
+ */
+static int cgroup_file_mode(const struct cftype *cft)
+{
+ int mode = 0;
+
+ if (cft->mode)
+ return cft->mode;
+
+ if (cft->read || cft->read_u64 || cft->read_s64 ||
+ cft->read_map || cft->read_seq_string)
+ mode += 0444;
+
+ if (cft->write || cft->write_u64 || cft->write_s64 ||
+ cft->write_string || cft->trigger)
+ mode += 0200;
+
+ return mode;
+}
+
int cgroup_add_file(struct cgroup *cgrp,
struct cgroup_subsys *subsys,
const struct cftype *cft)
@@ -1757,6 +1784,7 @@ int cgroup_add_file(struct cgroup *cgrp,
struct dentry *dir = cgrp->dentry;
struct dentry *dentry;
int error;
+ int mode;

char name[MAX_CGROUP_TYPE_NAMELEN + MAX_CFTYPE_NAME + 2] = { 0 };
if (subsys && !test_bit(ROOT_NOPREFIX, &cgrp->root->flags)) {
@@ -1767,6 +1795,7 @@ int cgroup_add_file(struct cgroup *cgrp,
BUG_ON(!mutex_is_locked(&dir->d_inode->i_mutex));
dentry = lookup_one_len(name, dir, strlen(name));
if (!IS_ERR(dentry)) {
+ mode = cgroup_file_mode(cft);
error = cgroup_create_file(dentry, 0644 | S_IFREG,
cgrp->root->sb);
if (!error)
@@ -2349,6 +2378,7 @@ static struct cftype files[] = {
.write_u64 = cgroup_tasks_write,
.release = cgroup_tasks_release,
.private = FILE_TASKLIST,
+ .mode = 0644,
},

{
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index a46d693..31e28b3 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1678,6 +1678,7 @@ static struct cftype files[] = {
.read_u64 = cpuset_read_u64,
.write_u64 = cpuset_write_u64,
.private = FILE_MEMORY_PRESSURE,
+ .mode = 0444,
},

{