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

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

[Li Zefan]

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.

Should be applied after:
	cgroup-allow-subsys-to-set-default-mode-of-its-own-file.patch

[PATCH 1/4] cgroup debug: show correct file mode
[PATCH 2/4] cpuacct: show correct file mode
[PATCH 3/4] devcgroup: show correct file mode
[PATCH 4/4] cpuset: show correct file mode

[PATCH 3/4] devcgroup: show correct file mode

[Li Zefan]

devices.allow and devices.deny are write-only, and devices.list is read-only.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 security/device_cgroup.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/security/device_cgroup.c b/security/device_cgroup.c
index 3aacd0f..b13fbb8 100644
--- a/security/device_cgroup.c
+++ b/security/device_cgroup.c
@@ -439,16 +439,19 @@ static struct cftype dev_cgroup_files[] = {
 		.name = "allow",
 		.write_string  = devcgroup_access_write,
 		.private = DEVCG_ALLOW,
+		.mode = 0200,
 	},
 	{
 		.name = "deny",
 		.write_string = devcgroup_access_write,
 		.private = DEVCG_DENY,
+		.mode = 0200,
 	},
 	{
 		.name = "list",
 		.read_seq_string = devcgroup_seq_read,
 		.private = DEVCG_LIST,
+		.mode = 0444,
 	},
 };
 
-- 1.5.4.rc3 

Re: [PATCH 3/4] devcgroup: show correct file mode

[Serge E. Hallyn]

Quoting Li Zefan (lizf@cn.fujitsu.com):
> devices.allow and devices.deny are write-only, and devices.list is read-only.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

Yup that should be intuitive for people.

Acked-by: Serge Hallyn <serue@us.ibm.com>

thanks,
-serge

> ---
>  security/device_cgroup.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/security/device_cgroup.c b/security/device_cgroup.c
> index 3aacd0f..b13fbb8 100644
> --- a/security/device_cgroup.c
> +++ b/security/device_cgroup.c
> @@ -439,16 +439,19 @@ static struct cftype dev_cgroup_files[] = {
>  		.name = "allow",
>  		.write_string  = devcgroup_access_write,
>  		.private = DEVCG_ALLOW,
> +		.mode = 0200,
>  	},
>  	{
>  		.name = "deny",
>  		.write_string = devcgroup_access_write,
>  		.private = DEVCG_DENY,
> +		.mode = 0200,
>  	},
>  	{
>  		.name = "list",
>  		.read_seq_string = devcgroup_seq_read,
>  		.private = DEVCG_LIST,
> +		.mode = 0444,
>  	},
>  };
> 
> -- 1.5.4.rc3 

Re: [PATCH 3/4] devcgroup: show correct file mode

[Serge E. Hallyn]

Quoting Li Zefan (lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org):
> devices.allow and devices.deny are write-only, and devices.list is read-only.
> 
> Signed-off-by: Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>

Yup that should be intuitive for people.

Acked-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

thanks,
-serge

> ---
>  security/device_cgroup.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/security/device_cgroup.c b/security/device_cgroup.c
> index 3aacd0f..b13fbb8 100644
> --- a/security/device_cgroup.c
> +++ b/security/device_cgroup.c
> @@ -439,16 +439,19 @@ static struct cftype dev_cgroup_files[] = {
>  		.name = "allow",
>  		.write_string  = devcgroup_access_write,
>  		.private = DEVCG_ALLOW,
> +		.mode = 0200,
>  	},
>  	{
>  		.name = "deny",
>  		.write_string = devcgroup_access_write,
>  		.private = DEVCG_DENY,
> +		.mode = 0200,
>  	},
>  	{
>  		.name = "list",
>  		.read_seq_string = devcgroup_seq_read,
>  		.private = DEVCG_LIST,
> +		.mode = 0444,
>  	},
>  };
> 
> -- 1.5.4.rc3 

[PATCH 4/4] cpuset: show correct file mode

[Li Zefan]

cpuset.memory_pressure is read-only.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/cpuset.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index a46d693..1ed507c 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1671,6 +1671,7 @@ static struct cftype files[] = {
 		.read_u64 = cpuset_read_u64,
 		.write_u64 = cpuset_write_u64,
 		.private = FILE_MEMORY_MIGRATE,
+		.mode = 0444,
 	},
 
 	{
-- 1.5.4.rc3 

[PATCH 1/4] cgroup debug: show correct file mode

[Li Zefan]

All control files in debug subsystem are read-only.

Signed-off-by: Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
 kernel/cgroup_debug.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/cgroup_debug.c b/kernel/cgroup_debug.c
index daca620..2ba54c6 100644
--- a/kernel/cgroup_debug.c
+++ b/kernel/cgroup_debug.c
@@ -71,25 +71,30 @@ static struct cftype files[] =  {
 	{
 		.name = "cgroup_refcount",
 		.read_u64 = cgroup_refcount_read,
+		.mode = 0444,
 	},
 	{
 		.name = "taskcount",
 		.read_u64 = taskcount_read,
+		.mode = 0444,
 	},
 
 	{
 		.name = "current_css_set",
 		.read_u64 = current_css_set_read,
+		.mode = 0444,
 	},
 
 	{
 		.name = "current_css_set_refcount",
 		.read_u64 = current_css_set_refcount_read,
+		.mode = 0444,
 	},
 
 	{
 		.name = "releasable",
 		.read_u64 = releasable_read,
+		.mode = 0444,
 	},
 };
 
-- 1.5.4.rc3 

[PATCH 1/4] cgroup debug: show correct file mode

[Li Zefan]

All control files in debug subsystem are read-only.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/cgroup_debug.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/cgroup_debug.c b/kernel/cgroup_debug.c
index daca620..2ba54c6 100644
--- a/kernel/cgroup_debug.c
+++ b/kernel/cgroup_debug.c
@@ -71,25 +71,30 @@ static struct cftype files[] =  {
 	{
 		.name = "cgroup_refcount",
 		.read_u64 = cgroup_refcount_read,
+		.mode = 0444,
 	},
 	{
 		.name = "taskcount",
 		.read_u64 = taskcount_read,
+		.mode = 0444,
 	},
 
 	{
 		.name = "current_css_set",
 		.read_u64 = current_css_set_read,
+		.mode = 0444,
 	},
 
 	{
 		.name = "current_css_set_refcount",
 		.read_u64 = current_css_set_refcount_read,
+		.mode = 0444,
 	},
 
 	{
 		.name = "releasable",
 		.read_u64 = releasable_read,
+		.mode = 0444,
 	},
 };
 
-- 1.5.4.rc3 

[PATCH 2/4] cpuacct: show correct file mode

[Li Zefan]

cpuacct.usage_percpu is read-only.

Signed-off-by: Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
 kernel/sched.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index e66f009..638aa4d 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -9734,6 +9734,7 @@ static struct cftype files[] = {
 	{
 		.name = "usage_percpu",
 		.read_seq_string = cpuacct_percpu_seq_read,
+		.mode = 0444,
 	},
 
 };
-- 1.5.4.rc3 

[PATCH 2/4] cpuacct: show correct file mode

[Li Zefan]

cpuacct.usage_percpu is read-only.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/sched.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index e66f009..638aa4d 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -9734,6 +9734,7 @@ static struct cftype files[] = {
 	{
 		.name = "usage_percpu",
 		.read_seq_string = cpuacct_percpu_seq_read,
+		.mode = 0444,
 	},
 
 };
-- 1.5.4.rc3 

[PATCH 3/4] devcgroup: show correct file mode

[Li Zefan]

devices.allow and devices.deny are write-only, and devices.list is read-only.

Signed-off-by: Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
 security/device_cgroup.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/security/device_cgroup.c b/security/device_cgroup.c
index 3aacd0f..b13fbb8 100644
--- a/security/device_cgroup.c
+++ b/security/device_cgroup.c
@@ -439,16 +439,19 @@ static struct cftype dev_cgroup_files[] = {
 		.name = "allow",
 		.write_string  = devcgroup_access_write,
 		.private = DEVCG_ALLOW,
+		.mode = 0200,
 	},
 	{
 		.name = "deny",
 		.write_string = devcgroup_access_write,
 		.private = DEVCG_DENY,
+		.mode = 0200,
 	},
 	{
 		.name = "list",
 		.read_seq_string = devcgroup_seq_read,
 		.private = DEVCG_LIST,
+		.mode = 0444,
 	},
 };
 
-- 1.5.4.rc3 

[PATCH 4/4] cpuset: show correct file mode

[Li Zefan]

cpuset.memory_pressure is read-only.

Signed-off-by: Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
 kernel/cpuset.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index a46d693..1ed507c 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1671,6 +1671,7 @@ static struct cftype files[] = {
 		.read_u64 = cpuset_read_u64,
 		.write_u64 = cpuset_write_u64,
 		.private = FILE_MEMORY_MIGRATE,
+		.mode = 0444,
 	},
 
 	{
-- 1.5.4.rc3 

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

[Paul Menage]

On Sun, Mar 1, 2009 at 6:13 PM, Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> 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.

Paul

>
> Should be applied after:
>        cgroup-allow-subsys-to-set-default-mode-of-its-own-file.patch
>
> [PATCH 1/4] cgroup debug: show correct file mode
> [PATCH 2/4] cpuacct: show correct file mode
> [PATCH 3/4] devcgroup: show correct file mode
> [PATCH 4/4] cpuset: show correct file mode
>
>

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

[Paul Menage]

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.

Paul

>
> Should be applied after:
>        cgroup-allow-subsys-to-set-default-mode-of-its-own-file.patch
>
> [PATCH 1/4] cgroup debug: show correct file mode
> [PATCH 2/4] cpuacct: show correct file mode
> [PATCH 3/4] devcgroup: show correct file mode
> [PATCH 4/4] cpuset: show correct file mode
>
>

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

[KAMEZAWA Hiroyuki]

On Mon, 2 Mar 2009 10:19:15 -0800
Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:

> On Sun, Mar 1, 2009 at 6:13 PM, Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> 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.
> 
like this ?
  
	int mode;
	if (cft->mode)
		mode = cft->mode;
	else if (cft->write_xxx || .....)
		mode = 0644;
	else
		mode = 0444;
Thanks,
-Kame

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

[Li Zefan]

Paul Menage wrote:
> On Sun, Mar 1, 2009 at 6:13 PM, Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> 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-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
 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,
 	},
 
 	{

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

[Li Zefan]

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,
 	},
 
 	{

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

[KAMEZAWA Hiroyuki]

On Tue, 03 Mar 2009 09:08:23 +0800
Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> wrote:

> Paul Menage wrote:
> > On Sun, Mar 1, 2009 at 6:13 PM, Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> 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-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
> ---
>  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;
> +

+= is not |=...

Thanks,
-Kame

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

[Paul Menage]

On Mon, Mar 2, 2009 at 5:08 PM, Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> wrote:
>
> 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.

Hmm. I find it hard to believe that anyone would have behaviour that
depends on cpuset.memory_pressure returning EACCES rather than EINVAL,
but I guess it's not a big deal.

On the += or |= issue, I think |= is slightly more familiar to most
readers given that you're doing bit options, although the end result
is the same in this case.

Acked-by: Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

>
> =====================
>
> cgroups: show correct file mode
>
> Signed-off-by: Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
> ---
>  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,
>        },
>
>        {
>
>

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

[Paul Menage]

On Mon, Mar 2, 2009 at 5:08 PM, Li Zefan <lizf@cn.fujitsu.com> wrote:
>
> 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.

Hmm. I find it hard to believe that anyone would have behaviour that
depends on cpuset.memory_pressure returning EACCES rather than EINVAL,
but I guess it's not a big deal.

On the += or |= issue, I think |= is slightly more familiar to most
readers given that you're doing bit options, although the end result
is the same in this case.

Acked-by: Paul Menage <menage@google.com>

>
> =====================
>
> 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,
>        },
>
>        {
>
>

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

[KAMEZAWA Hiroyuki]

On Tue, 03 Mar 2009 09:08:23 +0800
Li Zefan <lizf@cn.fujitsu.com> wrote:

> 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;
> +

+= is not |=...

Thanks,
-Kame

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

[Li Zefan]

>> +/**
>> + * 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;
>> +
> 
> += is not |=...
> 

Ah, yes, though both happen to result in 0644.

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

[KAMEZAWA Hiroyuki]

On Tue, 03 Mar 2009 09:15:42 +0800
Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> wrote:

> >> +/**
> >> + * 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;
> >> +
> > 
> > += is not |=...
> > 
> 
> Ah, yes, though both happen to result in 0644.
> 
yes but I like |= rather than += in bit ops.

Reviewed-by; KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>

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

[KAMEZAWA Hiroyuki]

On Tue, 03 Mar 2009 09:15:42 +0800
Li Zefan <lizf@cn.fujitsu.com> wrote:

> >> +/**
> >> + * 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;
> >> +
> > 
> > += is not |=...
> > 
> 
> Ah, yes, though both happen to result in 0644.
> 
yes but I like |= rather than += in bit ops.

Reviewed-by; KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

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

[Li Zefan]

>> +/**
>> + * 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;
>> +
> 
> += is not |=...
> 

Ah, yes, though both happen to result in 0644.

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

[KAMEZAWA Hiroyuki]

On Mon, 2 Mar 2009 10:19:15 -0800
Paul Menage <menage@google.com> 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.
> 
like this ?
  
	int mode;
	if (cft->mode)
		mode = cft->mode;
	else if (cft->write_xxx || .....)
		mode = 0644;
	else
		mode = 0444;
Thanks,
-Kame

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

[Paul Menage]

On Mon, Mar 2, 2009 at 4:09 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org> wrote:
>
>        int mode;
>        if (cft->mode)
>                mode = cft->mode;
>        else if (cft->write_xxx || .....)
>                mode = 0644;
>        else
>                mode = 0444;

Almost:

int mode;
if (cft->mode) {
  mode = cft->mode;
} else {
  if (cft->write_xxx || ....)
    mode |= 0600;
  if (cft->read_xxx || ...)
    mode |= 0444;
}

Paul

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

[Paul Menage]

On Mon, Mar 2, 2009 at 4:09 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>
>        int mode;
>        if (cft->mode)
>                mode = cft->mode;
>        else if (cft->write_xxx || .....)
>                mode = 0644;
>        else
>                mode = 0444;

Almost:

int mode;
if (cft->mode) {
  mode = cft->mode;
} else {
  if (cft->write_xxx || ....)
    mode |= 0600;
  if (cft->read_xxx || ...)
    mode |= 0444;
}

Paul

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

[KAMEZAWA Hiroyuki]

On Mon, 2 Mar 2009 16:15:51 -0800
Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:

> On Mon, Mar 2, 2009 at 4:09 PM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org> wrote:
> >
> >        int mode;
> >        if (cft->mode)
> >                mode = cft->mode;
> >        else if (cft->write_xxx || .....)
> >                mode = 0644;
> >        else
> >                mode = 0444;
> 
> Almost:
> 
> int mode;
> if (cft->mode) {
>   mode = cft->mode;
> } else {
>   if (cft->write_xxx || ....)
>     mode |= 0600;
>   if (cft->read_xxx || ...)
>     mode |= 0444;
> }
> 
Oh, I see. But int mode=0, at first ;)
I have no objections but please forgive subsys to set mode=0644 explicitly.

Thanks,
-Kame

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

[KAMEZAWA Hiroyuki]

On Mon, 2 Mar 2009 16:15:51 -0800
Paul Menage <menage@google.com> wrote:

> On Mon, Mar 2, 2009 at 4:09 PM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> >
> >        int mode;
> >        if (cft->mode)
> >                mode = cft->mode;
> >        else if (cft->write_xxx || .....)
> >                mode = 0644;
> >        else
> >                mode = 0444;
> 
> Almost:
> 
> int mode;
> if (cft->mode) {
>   mode = cft->mode;
> } else {
>   if (cft->write_xxx || ....)
>     mode |= 0600;
>   if (cft->read_xxx || ...)
>     mode |= 0444;
> }
> 
Oh, I see. But int mode=0, at first ;)
I have no objections but please forgive subsys to set mode=0644 explicitly.

Thanks,
-Kame

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

[Paul Menage]

On Mon, Mar 2, 2009 at 4:20 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>>
>> int mode;
>> if (cft->mode) {
>>   mode = cft->mode;
>> } else {
>>   if (cft->write_xxx || ....)
>>     mode |= 0600;
>>   if (cft->read_xxx || ...)
>>     mode |= 0444;
>> }
>>
> Oh, I see. But int mode=0, at first ;)

Oops, yes.

> I have no objections but please forgive subsys to set mode=0644 explicitly.

Do you mean "allow" rather than "forgive"? if so, then yes, a
subsystem should be able to override this to whatever it wants.

Paul

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

[Paul Menage]

On Mon, Mar 2, 2009 at 4:20 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org> wrote:
>>
>> int mode;
>> if (cft->mode) {
>>   mode = cft->mode;
>> } else {
>>   if (cft->write_xxx || ....)
>>     mode |= 0600;
>>   if (cft->read_xxx || ...)
>>     mode |= 0444;
>> }
>>
> Oh, I see. But int mode=0, at first ;)

Oops, yes.

> I have no objections but please forgive subsys to set mode=0644 explicitly.

Do you mean "allow" rather than "forgive"? if so, then yes, a
subsystem should be able to override this to whatever it wants.

Paul

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

[Mike Waychison]

KAMEZAWA Hiroyuki wrote:
> On Mon, 2 Mar 2009 16:15:51 -0800
> Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> 
>> On Mon, Mar 2, 2009 at 4:09 PM, KAMEZAWA Hiroyuki
>> <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org> wrote:
>>>        int mode;
>>>        if (cft->mode)
>>>                mode = cft->mode;
>>>        else if (cft->write_xxx || .....)
>>>                mode = 0644;
>>>        else
>>>                mode = 0444;
>> Almost:
>>
>> int mode;
>> if (cft->mode) {
>>   mode = cft->mode;
>> } else {
>>   if (cft->write_xxx || ....)
>>     mode |= 0600;

0200 ?

>>   if (cft->read_xxx || ...)
>>     mode |= 0444;
>> }
>>
> Oh, I see. But int mode=0, at first ;)
> I have no objections but please forgive subsys to set mode=0644 explicitly.
> 
> Thanks,
> -Kame
> 
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

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

[Mike Waychison]

KAMEZAWA Hiroyuki wrote:
> On Mon, 2 Mar 2009 16:15:51 -0800
> Paul Menage <menage@google.com> wrote:
> 
>> On Mon, Mar 2, 2009 at 4:09 PM, KAMEZAWA Hiroyuki
>> <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>>>        int mode;
>>>        if (cft->mode)
>>>                mode = cft->mode;
>>>        else if (cft->write_xxx || .....)
>>>                mode = 0644;
>>>        else
>>>                mode = 0444;
>> Almost:
>>
>> int mode;
>> if (cft->mode) {
>>   mode = cft->mode;
>> } else {
>>   if (cft->write_xxx || ....)
>>     mode |= 0600;

0200 ?

>>   if (cft->read_xxx || ...)
>>     mode |= 0444;
>> }
>>
> Oh, I see. But int mode=0, at first ;)
> I have no objections but please forgive subsys to set mode=0644 explicitly.
> 
> Thanks,
> -Kame
> 
> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

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

[Paul Menage]

On Mon, Mar 2, 2009 at 4:23 PM, Mike Waychison <mikew@google.com> wrote:
>>>  if (cft->write_xxx || ....)
>>>    mode |= 0600;
>
> 0200 ?

Oops (again). Yes, that's what I meant. Coding in email considered harmful :-)

Paul

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

[Paul Menage]

On Mon, Mar 2, 2009 at 4:23 PM, Mike Waychison <mikew-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>>  if (cft->write_xxx || ....)
>>>    mode |= 0600;
>
> 0200 ?

Oops (again). Yes, that's what I meant. Coding in email considered harmful :-)

Paul