* [PATCH 0/3] Simple cleanups for cgroups
@ 2011-12-11 14:45 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:45 UTC (permalink / raw)
To: linux-kernel; +Cc: jbottomley, cgroups, bsingharora, devel, kamezawa.hiroyu, tj
Hi,
While hacking on other stuff I found these spots that could
receive some simple cleanup in cgroup.c. Nothing revolutionary.
Patch 1 is rather trivial, the other 2 are more of a matter of
taste I'd say, but I believe we'd be better of this way.
Glauber Costa (3):
nitpick: make simple functions inline
make clone_children a flag
make 'none' field a flag
kernel/cgroup.c | 30 +++++++++++++++++-------------
1 files changed, 17 insertions(+), 13 deletions(-)
--
1.7.6.4
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 0/3] Simple cleanups for cgroups
@ 2011-12-11 14:45 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
tj-DgEjT+Ai2ygdnm+yROfE0A
Hi,
While hacking on other stuff I found these spots that could
receive some simple cleanup in cgroup.c. Nothing revolutionary.
Patch 1 is rather trivial, the other 2 are more of a matter of
taste I'd say, but I believe we'd be better of this way.
Glauber Costa (3):
nitpick: make simple functions inline
make clone_children a flag
make 'none' field a flag
kernel/cgroup.c | 30 +++++++++++++++++-------------
1 files changed, 17 insertions(+), 13 deletions(-)
--
1.7.6.4
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH] make clone_children a flag
@ 2011-12-11 14:45 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:45 UTC (permalink / raw)
To: linux-kernel
Cc: jbottomley, cgroups, bsingharora, devel, kamezawa.hiroyu, tj,
Glauber Costa
There is no reason to have a flags field, and then a separate
bool field just to indicate if the clone_children flag is set.
Make it a flag
Signed-off-by: Glauber Costa <glommer@parallels.com>
---
kernel/cgroup.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index e4b9d3c..fa405ee 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -231,6 +231,7 @@ inline int cgroup_is_removed(const struct cgroup *cgrp)
/* bits in struct cgroupfs_root flags field */
enum {
ROOT_NOPREFIX, /* mounted subsystems have no named prefix */
+ ROOT_CLONE_CHILDREN, /* mounted subsystems starts with clone_children */
};
static int cgroup_is_releasable(const struct cgroup *cgrp)
@@ -1062,7 +1063,6 @@ struct cgroup_sb_opts {
unsigned long subsys_bits;
unsigned long flags;
char *release_agent;
- bool clone_children;
char *name;
/* User explicitly requested empty subsystem */
bool none;
@@ -1113,7 +1113,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
continue;
}
if (!strcmp(token, "clone_children")) {
- opts->clone_children = true;
+ set_bit(ROOT_CLONE_CHILDREN, &opts->flags);
continue;
}
if (!strncmp(token, "release_agent=", 14)) {
@@ -1400,7 +1400,7 @@ static struct cgroupfs_root *cgroup_root_from_opts(struct cgroup_sb_opts *opts)
strcpy(root->release_agent_path, opts->release_agent);
if (opts->name)
strcpy(root->name, opts->name);
- if (opts->clone_children)
+ if (test_bit(ROOT_CLONE_CHILDREN, &opts->flags))
set_bit(CGRP_CLONE_CHILDREN, &root->top_cgroup.flags);
return root;
}
--
1.7.6.4
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH] make clone_children a flag
@ 2011-12-11 14:45 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
tj-DgEjT+Ai2ygdnm+yROfE0A, Glauber Costa
There is no reason to have a flags field, and then a separate
bool field just to indicate if the clone_children flag is set.
Make it a flag
Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
---
kernel/cgroup.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index e4b9d3c..fa405ee 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -231,6 +231,7 @@ inline int cgroup_is_removed(const struct cgroup *cgrp)
/* bits in struct cgroupfs_root flags field */
enum {
ROOT_NOPREFIX, /* mounted subsystems have no named prefix */
+ ROOT_CLONE_CHILDREN, /* mounted subsystems starts with clone_children */
};
static int cgroup_is_releasable(const struct cgroup *cgrp)
@@ -1062,7 +1063,6 @@ struct cgroup_sb_opts {
unsigned long subsys_bits;
unsigned long flags;
char *release_agent;
- bool clone_children;
char *name;
/* User explicitly requested empty subsystem */
bool none;
@@ -1113,7 +1113,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
continue;
}
if (!strcmp(token, "clone_children")) {
- opts->clone_children = true;
+ set_bit(ROOT_CLONE_CHILDREN, &opts->flags);
continue;
}
if (!strncmp(token, "release_agent=", 14)) {
@@ -1400,7 +1400,7 @@ static struct cgroupfs_root *cgroup_root_from_opts(struct cgroup_sb_opts *opts)
strcpy(root->release_agent_path, opts->release_agent);
if (opts->name)
strcpy(root->name, opts->name);
- if (opts->clone_children)
+ if (test_bit(ROOT_CLONE_CHILDREN, &opts->flags))
set_bit(CGRP_CLONE_CHILDREN, &root->top_cgroup.flags);
return root;
}
--
1.7.6.4
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 1/3] nitpick: make simple functions inline
@ 2011-12-11 14:45 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:45 UTC (permalink / raw)
To: linux-kernel
Cc: jbottomley, cgroups, bsingharora, devel, kamezawa.hiroyu, tj,
Glauber Costa
Those are quite simple bit-testing functions that are
only used within this file. Not reason for them not to
be inline.
Signed-off-by: Glauber Costa <glommer@parallels.com>
---
kernel/cgroup.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index d9d5648..e4b9d3c 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -241,12 +241,12 @@ static int cgroup_is_releasable(const struct cgroup *cgrp)
return (cgrp->flags & bits) == bits;
}
-static int notify_on_release(const struct cgroup *cgrp)
+static inline int notify_on_release(const struct cgroup *cgrp)
{
return test_bit(CGRP_NOTIFY_ON_RELEASE, &cgrp->flags);
}
-static int clone_children(const struct cgroup *cgrp)
+static inline int clone_children(const struct cgroup *cgrp)
{
return test_bit(CGRP_CLONE_CHILDREN, &cgrp->flags);
}
--
1.7.6.4
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 1/3] nitpick: make simple functions inline
@ 2011-12-11 14:45 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
tj-DgEjT+Ai2ygdnm+yROfE0A, Glauber Costa
Those are quite simple bit-testing functions that are
only used within this file. Not reason for them not to
be inline.
Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
---
kernel/cgroup.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index d9d5648..e4b9d3c 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -241,12 +241,12 @@ static int cgroup_is_releasable(const struct cgroup *cgrp)
return (cgrp->flags & bits) == bits;
}
-static int notify_on_release(const struct cgroup *cgrp)
+static inline int notify_on_release(const struct cgroup *cgrp)
{
return test_bit(CGRP_NOTIFY_ON_RELEASE, &cgrp->flags);
}
-static int clone_children(const struct cgroup *cgrp)
+static inline int clone_children(const struct cgroup *cgrp)
{
return test_bit(CGRP_CLONE_CHILDREN, &cgrp->flags);
}
--
1.7.6.4
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 2/3] make clone_children a flag
@ 2011-12-11 14:45 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:45 UTC (permalink / raw)
To: linux-kernel
Cc: jbottomley, cgroups, bsingharora, devel, kamezawa.hiroyu, tj,
Glauber Costa
There is no reason to have a flags field, and then a separate
bool field just to indicate if the clone_children flag is set.
Make it a flag
Signed-off-by: Glauber Costa <glommer@parallels.com>
---
kernel/cgroup.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index e4b9d3c..fa405ee 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -231,6 +231,7 @@ inline int cgroup_is_removed(const struct cgroup *cgrp)
/* bits in struct cgroupfs_root flags field */
enum {
ROOT_NOPREFIX, /* mounted subsystems have no named prefix */
+ ROOT_CLONE_CHILDREN, /* mounted subsystems starts with clone_children */
};
static int cgroup_is_releasable(const struct cgroup *cgrp)
@@ -1062,7 +1063,6 @@ struct cgroup_sb_opts {
unsigned long subsys_bits;
unsigned long flags;
char *release_agent;
- bool clone_children;
char *name;
/* User explicitly requested empty subsystem */
bool none;
@@ -1113,7 +1113,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
continue;
}
if (!strcmp(token, "clone_children")) {
- opts->clone_children = true;
+ set_bit(ROOT_CLONE_CHILDREN, &opts->flags);
continue;
}
if (!strncmp(token, "release_agent=", 14)) {
@@ -1400,7 +1400,7 @@ static struct cgroupfs_root *cgroup_root_from_opts(struct cgroup_sb_opts *opts)
strcpy(root->release_agent_path, opts->release_agent);
if (opts->name)
strcpy(root->name, opts->name);
- if (opts->clone_children)
+ if (test_bit(ROOT_CLONE_CHILDREN, &opts->flags))
set_bit(CGRP_CLONE_CHILDREN, &root->top_cgroup.flags);
return root;
}
--
1.7.6.4
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 2/3] make clone_children a flag
@ 2011-12-11 14:45 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
tj-DgEjT+Ai2ygdnm+yROfE0A, Glauber Costa
There is no reason to have a flags field, and then a separate
bool field just to indicate if the clone_children flag is set.
Make it a flag
Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
---
kernel/cgroup.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index e4b9d3c..fa405ee 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -231,6 +231,7 @@ inline int cgroup_is_removed(const struct cgroup *cgrp)
/* bits in struct cgroupfs_root flags field */
enum {
ROOT_NOPREFIX, /* mounted subsystems have no named prefix */
+ ROOT_CLONE_CHILDREN, /* mounted subsystems starts with clone_children */
};
static int cgroup_is_releasable(const struct cgroup *cgrp)
@@ -1062,7 +1063,6 @@ struct cgroup_sb_opts {
unsigned long subsys_bits;
unsigned long flags;
char *release_agent;
- bool clone_children;
char *name;
/* User explicitly requested empty subsystem */
bool none;
@@ -1113,7 +1113,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
continue;
}
if (!strcmp(token, "clone_children")) {
- opts->clone_children = true;
+ set_bit(ROOT_CLONE_CHILDREN, &opts->flags);
continue;
}
if (!strncmp(token, "release_agent=", 14)) {
@@ -1400,7 +1400,7 @@ static struct cgroupfs_root *cgroup_root_from_opts(struct cgroup_sb_opts *opts)
strcpy(root->release_agent_path, opts->release_agent);
if (opts->name)
strcpy(root->name, opts->name);
- if (opts->clone_children)
+ if (test_bit(ROOT_CLONE_CHILDREN, &opts->flags))
set_bit(CGRP_CLONE_CHILDREN, &root->top_cgroup.flags);
return root;
}
--
1.7.6.4
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 3/3] make 'none' field a flag
@ 2011-12-11 14:45 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:45 UTC (permalink / raw)
To: linux-kernel
Cc: jbottomley, cgroups, bsingharora, devel, kamezawa.hiroyu, tj,
Glauber Costa
There is no reason to have a flags field, and then a separate
bool field just to indicate if 'none' subsystem were explicitly
requested.
Make it a flag
Signed-off-by: Glauber Costa <glommer@parallels.com>
---
kernel/cgroup.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index fa405ee..e700abe 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -232,6 +232,7 @@ inline int cgroup_is_removed(const struct cgroup *cgrp)
enum {
ROOT_NOPREFIX, /* mounted subsystems have no named prefix */
ROOT_CLONE_CHILDREN, /* mounted subsystems starts with clone_children */
+ ROOT_NOSUBSYS, /* explicitly asked for 'none' subsystems */
};
static int cgroup_is_releasable(const struct cgroup *cgrp)
@@ -1064,13 +1065,16 @@ struct cgroup_sb_opts {
unsigned long flags;
char *release_agent;
char *name;
- /* User explicitly requested empty subsystem */
- bool none;
struct cgroupfs_root *new_root;
};
+static inline int opts_no_subsys(const struct cgroup_sb_opts *opts)
+{
+ return test_bit(ROOT_NOSUBSYS, &opts->flags);
+}
+
/*
* Convert a hierarchy specifier into a bitmask of subsystems and flags. Call
* with cgroup_mutex held to protect the subsys[] array. This function takes
@@ -1098,7 +1102,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
return -EINVAL;
if (!strcmp(token, "none")) {
/* Explicitly have no subsystems */
- opts->none = true;
+ set_bit(ROOT_NOSUBSYS, &opts->flags);
continue;
}
if (!strcmp(token, "all")) {
@@ -1178,7 +1182,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
* otherwise 'all, 'none' and a subsystem name options were not
* specified, let's default to 'all'
*/
- if (all_ss || (!all_ss && !one_ss && !opts->none)) {
+ if (all_ss || (!all_ss && !one_ss && !opts_no_subsys(opts))) {
for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
struct cgroup_subsys *ss = subsys[i];
if (ss == NULL)
@@ -1202,7 +1206,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
/* Can't specify "none" and some subsystems */
- if (opts->subsys_bits && opts->none)
+ if (opts->subsys_bits && opts_no_subsys(opts))
return -EINVAL;
/*
@@ -1370,7 +1374,7 @@ static int cgroup_test_super(struct super_block *sb, void *data)
* If we asked for subsystems (or explicitly for no
* subsystems) then they must match
*/
- if ((opts->subsys_bits || opts->none)
+ if ((opts->subsys_bits || opts_no_subsys(opts))
&& (opts->subsys_bits != root->subsys_bits))
return 0;
@@ -1381,7 +1385,7 @@ static struct cgroupfs_root *cgroup_root_from_opts(struct cgroup_sb_opts *opts)
{
struct cgroupfs_root *root;
- if (!opts->subsys_bits && !opts->none)
+ if (!opts->subsys_bits && !opts_no_subsys(opts))
return NULL;
root = kzalloc(sizeof(*root), GFP_KERNEL);
@@ -1426,7 +1430,7 @@ static int cgroup_set_super(struct super_block *sb, void *data)
if (!opts->new_root)
return -EINVAL;
- BUG_ON(!opts->subsys_bits && !opts->none);
+ BUG_ON(!opts->subsys_bits && !opts_no_subsys(opts));
ret = set_anon_super(sb, NULL);
if (ret)
--
1.7.6.4
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 3/3] make 'none' field a flag
@ 2011-12-11 14:45 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:45 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
tj-DgEjT+Ai2ygdnm+yROfE0A, Glauber Costa
There is no reason to have a flags field, and then a separate
bool field just to indicate if 'none' subsystem were explicitly
requested.
Make it a flag
Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
---
kernel/cgroup.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index fa405ee..e700abe 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -232,6 +232,7 @@ inline int cgroup_is_removed(const struct cgroup *cgrp)
enum {
ROOT_NOPREFIX, /* mounted subsystems have no named prefix */
ROOT_CLONE_CHILDREN, /* mounted subsystems starts with clone_children */
+ ROOT_NOSUBSYS, /* explicitly asked for 'none' subsystems */
};
static int cgroup_is_releasable(const struct cgroup *cgrp)
@@ -1064,13 +1065,16 @@ struct cgroup_sb_opts {
unsigned long flags;
char *release_agent;
char *name;
- /* User explicitly requested empty subsystem */
- bool none;
struct cgroupfs_root *new_root;
};
+static inline int opts_no_subsys(const struct cgroup_sb_opts *opts)
+{
+ return test_bit(ROOT_NOSUBSYS, &opts->flags);
+}
+
/*
* Convert a hierarchy specifier into a bitmask of subsystems and flags. Call
* with cgroup_mutex held to protect the subsys[] array. This function takes
@@ -1098,7 +1102,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
return -EINVAL;
if (!strcmp(token, "none")) {
/* Explicitly have no subsystems */
- opts->none = true;
+ set_bit(ROOT_NOSUBSYS, &opts->flags);
continue;
}
if (!strcmp(token, "all")) {
@@ -1178,7 +1182,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
* otherwise 'all, 'none' and a subsystem name options were not
* specified, let's default to 'all'
*/
- if (all_ss || (!all_ss && !one_ss && !opts->none)) {
+ if (all_ss || (!all_ss && !one_ss && !opts_no_subsys(opts))) {
for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
struct cgroup_subsys *ss = subsys[i];
if (ss == NULL)
@@ -1202,7 +1206,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
/* Can't specify "none" and some subsystems */
- if (opts->subsys_bits && opts->none)
+ if (opts->subsys_bits && opts_no_subsys(opts))
return -EINVAL;
/*
@@ -1370,7 +1374,7 @@ static int cgroup_test_super(struct super_block *sb, void *data)
* If we asked for subsystems (or explicitly for no
* subsystems) then they must match
*/
- if ((opts->subsys_bits || opts->none)
+ if ((opts->subsys_bits || opts_no_subsys(opts))
&& (opts->subsys_bits != root->subsys_bits))
return 0;
@@ -1381,7 +1385,7 @@ static struct cgroupfs_root *cgroup_root_from_opts(struct cgroup_sb_opts *opts)
{
struct cgroupfs_root *root;
- if (!opts->subsys_bits && !opts->none)
+ if (!opts->subsys_bits && !opts_no_subsys(opts))
return NULL;
root = kzalloc(sizeof(*root), GFP_KERNEL);
@@ -1426,7 +1430,7 @@ static int cgroup_set_super(struct super_block *sb, void *data)
if (!opts->new_root)
return -EINVAL;
- BUG_ON(!opts->subsys_bits && !opts->none);
+ BUG_ON(!opts->subsys_bits && !opts_no_subsys(opts));
ret = set_anon_super(sb, NULL);
if (ret)
--
1.7.6.4
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH] make clone_children a flag
2011-12-11 14:45 ` Glauber Costa
@ 2011-12-11 14:48 ` Glauber Costa
-1 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:48 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel, jbottomley, cgroups, bsingharora, devel,
kamezawa.hiroyu, tj
On 12/11/2011 03:45 PM, Glauber Costa wrote:
> There is no reason to have a flags field, and then a separate
> bool field just to indicate if the clone_children flag is set.
> Make it a flag
>
> Signed-off-by: Glauber Costa<glommer@parallels.com>
This one is repeated, I don't know how it went through. Sorry.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] make clone_children a flag
@ 2011-12-11 14:48 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 14:48 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel, jbottomley, cgroups, bsingharora, devel,
kamezawa.hiroyu, tj
On 12/11/2011 03:45 PM, Glauber Costa wrote:
> There is no reason to have a flags field, and then a separate
> bool field just to indicate if the clone_children flag is set.
> Make it a flag
>
> Signed-off-by: Glauber Costa<glommer@parallels.com>
This one is repeated, I don't know how it went through. Sorry.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] nitpick: make simple functions inline
@ 2011-12-11 18:55 ` KOSAKI Motohiro
0 siblings, 0 replies; 37+ messages in thread
From: KOSAKI Motohiro @ 2011-12-11 18:55 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel, jbottomley, cgroups, bsingharora, devel,
kamezawa.hiroyu, tj
> -static int notify_on_release(const struct cgroup *cgrp)
> +static inline int notify_on_release(const struct cgroup *cgrp)
> {
> return test_bit(CGRP_NOTIFY_ON_RELEASE,&cgrp->flags);
> }
>
> -static int clone_children(const struct cgroup *cgrp)
> +static inline int clone_children(const struct cgroup *cgrp)
> {
> return test_bit(CGRP_CLONE_CHILDREN,&cgrp->flags);
> }
Can you please tell us which compiler failed automatic inlining?
I suspect gcc is enough sane and we don't need this patch.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] nitpick: make simple functions inline
@ 2011-12-11 18:55 ` KOSAKI Motohiro
0 siblings, 0 replies; 37+ messages in thread
From: KOSAKI Motohiro @ 2011-12-11 18:55 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
tj-DgEjT+Ai2ygdnm+yROfE0A
> -static int notify_on_release(const struct cgroup *cgrp)
> +static inline int notify_on_release(const struct cgroup *cgrp)
> {
> return test_bit(CGRP_NOTIFY_ON_RELEASE,&cgrp->flags);
> }
>
> -static int clone_children(const struct cgroup *cgrp)
> +static inline int clone_children(const struct cgroup *cgrp)
> {
> return test_bit(CGRP_CLONE_CHILDREN,&cgrp->flags);
> }
Can you please tell us which compiler failed automatic inlining?
I suspect gcc is enough sane and we don't need this patch.
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
@ 2011-12-11 18:58 ` KOSAKI Motohiro
0 siblings, 0 replies; 37+ messages in thread
From: KOSAKI Motohiro @ 2011-12-11 18:58 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel, jbottomley, cgroups, bsingharora, devel,
kamezawa.hiroyu, tj
(12/11/11 9:45 AM), Glauber Costa wrote:
> There is no reason to have a flags field, and then a separate
> bool field just to indicate if the clone_children flag is set.
> Make it a flag
>
> Signed-off-by: Glauber Costa<glommer@parallels.com>
> ---
> kernel/cgroup.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
@ 2011-12-11 18:58 ` KOSAKI Motohiro
0 siblings, 0 replies; 37+ messages in thread
From: KOSAKI Motohiro @ 2011-12-11 18:58 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
tj-DgEjT+Ai2ygdnm+yROfE0A
(12/11/11 9:45 AM), Glauber Costa wrote:
> There is no reason to have a flags field, and then a separate
> bool field just to indicate if the clone_children flag is set.
> Make it a flag
>
> Signed-off-by: Glauber Costa<glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> ---
> kernel/cgroup.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: KOSAKI Motohiro <kosaki.motohiro-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 3/3] make 'none' field a flag
2011-12-11 14:45 ` Glauber Costa
(?)
@ 2011-12-11 18:59 ` KOSAKI Motohiro
-1 siblings, 0 replies; 37+ messages in thread
From: KOSAKI Motohiro @ 2011-12-11 18:59 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel, jbottomley, cgroups, bsingharora, devel,
kamezawa.hiroyu, tj
(12/11/11 9:45 AM), Glauber Costa wrote:
> There is no reason to have a flags field, and then a separate
> bool field just to indicate if 'none' subsystem were explicitly
> requested.
>
> Make it a flag
>
> Signed-off-by: Glauber Costa<glommer@parallels.com>
Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] nitpick: make simple functions inline
@ 2011-12-11 20:44 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 20:44 UTC (permalink / raw)
To: KOSAKI Motohiro
Cc: linux-kernel, jbottomley, cgroups, bsingharora, devel,
kamezawa.hiroyu, tj
On 12/11/2011 07:55 PM, KOSAKI Motohiro wrote:
>> -static int notify_on_release(const struct cgroup *cgrp)
>> +static inline int notify_on_release(const struct cgroup *cgrp)
>> {
>> return test_bit(CGRP_NOTIFY_ON_RELEASE,&cgrp->flags);
>> }
>>
>> -static int clone_children(const struct cgroup *cgrp)
>> +static inline int clone_children(const struct cgroup *cgrp)
>> {
>> return test_bit(CGRP_CLONE_CHILDREN,&cgrp->flags);
>> }
>
> Can you please tell us which compiler failed automatic inlining?
> I suspect gcc is enough sane and we don't need this patch.
Of course we don't need, that's the very definition of a "nitpick".
This patch is directed towards the reader, not the compiler. Maintainers
are free to take it or not, although I believe being explicit is better.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] nitpick: make simple functions inline
@ 2011-12-11 20:44 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-11 20:44 UTC (permalink / raw)
To: KOSAKI Motohiro
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
tj-DgEjT+Ai2ygdnm+yROfE0A
On 12/11/2011 07:55 PM, KOSAKI Motohiro wrote:
>> -static int notify_on_release(const struct cgroup *cgrp)
>> +static inline int notify_on_release(const struct cgroup *cgrp)
>> {
>> return test_bit(CGRP_NOTIFY_ON_RELEASE,&cgrp->flags);
>> }
>>
>> -static int clone_children(const struct cgroup *cgrp)
>> +static inline int clone_children(const struct cgroup *cgrp)
>> {
>> return test_bit(CGRP_CLONE_CHILDREN,&cgrp->flags);
>> }
>
> Can you please tell us which compiler failed automatic inlining?
> I suspect gcc is enough sane and we don't need this patch.
Of course we don't need, that's the very definition of a "nitpick".
This patch is directed towards the reader, not the compiler. Maintainers
are free to take it or not, although I believe being explicit is better.
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] nitpick: make simple functions inline
@ 2011-12-12 17:27 ` Tejun Heo
0 siblings, 0 replies; 37+ messages in thread
From: Tejun Heo @ 2011-12-12 17:27 UTC (permalink / raw)
To: Glauber Costa
Cc: KOSAKI Motohiro, linux-kernel, jbottomley, cgroups, bsingharora,
devel, kamezawa.hiroyu
Hello,
On Sun, Dec 11, 2011 at 09:44:54PM +0100, Glauber Costa wrote:
> On 12/11/2011 07:55 PM, KOSAKI Motohiro wrote:
> > Can you please tell us which compiler failed automatic inlining?
> > I suspect gcc is enough sane and we don't need this patch.
>
> Of course we don't need, that's the very definition of a "nitpick".
> This patch is directed towards the reader, not the compiler. Maintainers
> are free to take it or not, although I believe being explicit is better.
These days, I don't think adding inline buys us much (other than
explicit cases where always_inline or noinline is necessary). gcc
already does good enough job for inlining and 'inline' hint seems more
to hinder rather than help and I don't really see what it buys for
code readers either, so I won't be taking this one.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] nitpick: make simple functions inline
@ 2011-12-12 17:27 ` Tejun Heo
0 siblings, 0 replies; 37+ messages in thread
From: Tejun Heo @ 2011-12-12 17:27 UTC (permalink / raw)
To: Glauber Costa
Cc: KOSAKI Motohiro, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A
Hello,
On Sun, Dec 11, 2011 at 09:44:54PM +0100, Glauber Costa wrote:
> On 12/11/2011 07:55 PM, KOSAKI Motohiro wrote:
> > Can you please tell us which compiler failed automatic inlining?
> > I suspect gcc is enough sane and we don't need this patch.
>
> Of course we don't need, that's the very definition of a "nitpick".
> This patch is directed towards the reader, not the compiler. Maintainers
> are free to take it or not, although I believe being explicit is better.
These days, I don't think adding inline buys us much (other than
explicit cases where always_inline or noinline is necessary). gcc
already does good enough job for inlining and 'inline' hint seems more
to hinder rather than help and I don't really see what it buys for
code readers either, so I won't be taking this one.
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
@ 2011-12-13 15:39 ` Tejun Heo
0 siblings, 0 replies; 37+ messages in thread
From: Tejun Heo @ 2011-12-13 15:39 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel, jbottomley, cgroups, bsingharora, devel, kamezawa.hiroyu
On Sun, Dec 11, 2011 at 03:45:37PM +0100, Glauber Costa wrote:
> There is no reason to have a flags field, and then a separate
> bool field just to indicate if the clone_children flag is set.
> Make it a flag
>
> Signed-off-by: Glauber Costa <glommer@parallels.com>
Doesn't this change how remount conditions are checked?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
@ 2011-12-13 15:39 ` Tejun Heo
0 siblings, 0 replies; 37+ messages in thread
From: Tejun Heo @ 2011-12-13 15:39 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A
On Sun, Dec 11, 2011 at 03:45:37PM +0100, Glauber Costa wrote:
> There is no reason to have a flags field, and then a separate
> bool field just to indicate if the clone_children flag is set.
> Make it a flag
>
> Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Doesn't this change how remount conditions are checked?
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 3/3] make 'none' field a flag
@ 2011-12-13 15:41 ` Tejun Heo
0 siblings, 0 replies; 37+ messages in thread
From: Tejun Heo @ 2011-12-13 15:41 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel, jbottomley, cgroups, bsingharora, devel, kamezawa.hiroyu
On Sun, Dec 11, 2011 at 03:45:38PM +0100, Glauber Costa wrote:
> There is no reason to have a flags field, and then a separate
> bool field just to indicate if 'none' subsystem were explicitly
> requested.
>
> Make it a flag
>
> Signed-off-by: Glauber Costa <glommer@parallels.com>
Same as the previous patch. Doesn't this change how remount
conditions are checked?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 3/3] make 'none' field a flag
@ 2011-12-13 15:41 ` Tejun Heo
0 siblings, 0 replies; 37+ messages in thread
From: Tejun Heo @ 2011-12-13 15:41 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A
On Sun, Dec 11, 2011 at 03:45:38PM +0100, Glauber Costa wrote:
> There is no reason to have a flags field, and then a separate
> bool field just to indicate if 'none' subsystem were explicitly
> requested.
>
> Make it a flag
>
> Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Same as the previous patch. Doesn't this change how remount
conditions are checked?
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] nitpick: make simple functions inline
@ 2011-12-14 1:09 ` Li Zefan
0 siblings, 0 replies; 37+ messages in thread
From: Li Zefan @ 2011-12-14 1:09 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel, jbottomley, cgroups, bsingharora, devel,
kamezawa.hiroyu, tj
22:45, Glauber Costa wrote:
> Those are quite simple bit-testing functions that are
> only used within this file. Not reason for them not to
> be inline.
>
It's better to leave the optimization decision to gcc.
And I've confirmed they are inlined by gcc in my box.
(btw, please add "cgroup" prefix to the patch subject line)
> Signed-off-by: Glauber Costa <glommer@parallels.com>
> ---
> kernel/cgroup.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index d9d5648..e4b9d3c 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -241,12 +241,12 @@ static int cgroup_is_releasable(const struct cgroup *cgrp)
> return (cgrp->flags & bits) == bits;
> }
>
> -static int notify_on_release(const struct cgroup *cgrp)
> +static inline int notify_on_release(const struct cgroup *cgrp)
> {
> return test_bit(CGRP_NOTIFY_ON_RELEASE, &cgrp->flags);
> }
>
> -static int clone_children(const struct cgroup *cgrp)
> +static inline int clone_children(const struct cgroup *cgrp)
> {
> return test_bit(CGRP_CLONE_CHILDREN, &cgrp->flags);
> }
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/3] nitpick: make simple functions inline
@ 2011-12-14 1:09 ` Li Zefan
0 siblings, 0 replies; 37+ messages in thread
From: Li Zefan @ 2011-12-14 1:09 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
tj-DgEjT+Ai2ygdnm+yROfE0A
22:45, Glauber Costa wrote:
> Those are quite simple bit-testing functions that are
> only used within this file. Not reason for them not to
> be inline.
>
It's better to leave the optimization decision to gcc.
And I've confirmed they are inlined by gcc in my box.
(btw, please add "cgroup" prefix to the patch subject line)
> Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> ---
> kernel/cgroup.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index d9d5648..e4b9d3c 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -241,12 +241,12 @@ static int cgroup_is_releasable(const struct cgroup *cgrp)
> return (cgrp->flags & bits) == bits;
> }
>
> -static int notify_on_release(const struct cgroup *cgrp)
> +static inline int notify_on_release(const struct cgroup *cgrp)
> {
> return test_bit(CGRP_NOTIFY_ON_RELEASE, &cgrp->flags);
> }
>
> -static int clone_children(const struct cgroup *cgrp)
> +static inline int clone_children(const struct cgroup *cgrp)
> {
> return test_bit(CGRP_CLONE_CHILDREN, &cgrp->flags);
> }
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 3/3] make 'none' field a flag
@ 2011-12-14 2:09 ` Li Zefan
0 siblings, 0 replies; 37+ messages in thread
From: Li Zefan @ 2011-12-14 2:09 UTC (permalink / raw)
To: Tejun Heo
Cc: Glauber Costa, linux-kernel, jbottomley, cgroups, bsingharora,
devel, kamezawa.hiroyu
于 2011年12月13日 23:41, Tejun Heo 写道:
> On Sun, Dec 11, 2011 at 03:45:38PM +0100, Glauber Costa wrote:
>> There is no reason to have a flags field, and then a separate
>> bool field just to indicate if 'none' subsystem were explicitly
>> requested.
>>
>> Make it a flag
>>
>> Signed-off-by: Glauber Costa <glommer@parallels.com>
>
> Same as the previous patch. Doesn't this change how remount
> conditions are checked?
>
Right. The patch prevents us from doing:
# mount -t cgroup -o none,name=tmp xxx /mnt
# mount -o remount,cpuset xxx /mnt
(failed)
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 3/3] make 'none' field a flag
@ 2011-12-14 2:09 ` Li Zefan
0 siblings, 0 replies; 37+ messages in thread
From: Li Zefan @ 2011-12-14 2:09 UTC (permalink / raw)
To: Tejun Heo
Cc: Glauber Costa, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A
于 2011年12月13日 23:41, Tejun Heo 写道:
> On Sun, Dec 11, 2011 at 03:45:38PM +0100, Glauber Costa wrote:
>> There is no reason to have a flags field, and then a separate
>> bool field just to indicate if 'none' subsystem were explicitly
>> requested.
>>
>> Make it a flag
>>
>> Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>
> Same as the previous patch. Doesn't this change how remount
> conditions are checked?
>
Right. The patch prevents us from doing:
# mount -t cgroup -o none,name=tmp xxx /mnt
# mount -o remount,cpuset xxx /mnt
(failed)
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
@ 2011-12-14 2:29 ` Li Zefan
0 siblings, 0 replies; 37+ messages in thread
From: Li Zefan @ 2011-12-14 2:29 UTC (permalink / raw)
To: Tejun Heo
Cc: Glauber Costa, linux-kernel, jbottomley, cgroups, bsingharora,
devel, kamezawa.hiroyu
Tejun Heo wrote:
> On Sun, Dec 11, 2011 at 03:45:37PM +0100, Glauber Costa wrote:
>> There is no reason to have a flags field, and then a separate
>> bool field just to indicate if the clone_children flag is set.
>> Make it a flag
>>
>> Signed-off-by: Glauber Costa <glommer@parallels.com>
>
> Doesn't this change how remount conditions are checked?
>
Right. Currently we can do this:
# mount -t cgroup xxx /mnt
# mount -o remount,clone_children /mnt
with this patch, the above remount will fail.
But..the current bevaiour of remount is a bit confusing in that remount
with/without "clone_children" has no effect on anything:
# mount -t cgroup -o clone_children xxx /mnt
# cat /mnt/cgroup.clone_children
1
# mount -o remount xxx /mnt
# mount | grep cgroup
xxx on /mnt type cgroup (rw,clone_children)
# cat /mnt/cgroup.clone_children
1
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
@ 2011-12-14 2:29 ` Li Zefan
0 siblings, 0 replies; 37+ messages in thread
From: Li Zefan @ 2011-12-14 2:29 UTC (permalink / raw)
To: Tejun Heo
Cc: Glauber Costa, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A
Tejun Heo wrote:
> On Sun, Dec 11, 2011 at 03:45:37PM +0100, Glauber Costa wrote:
>> There is no reason to have a flags field, and then a separate
>> bool field just to indicate if the clone_children flag is set.
>> Make it a flag
>>
>> Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>
> Doesn't this change how remount conditions are checked?
>
Right. Currently we can do this:
# mount -t cgroup xxx /mnt
# mount -o remount,clone_children /mnt
with this patch, the above remount will fail.
But..the current bevaiour of remount is a bit confusing in that remount
with/without "clone_children" has no effect on anything:
# mount -t cgroup -o clone_children xxx /mnt
# cat /mnt/cgroup.clone_children
1
# mount -o remount xxx /mnt
# mount | grep cgroup
xxx on /mnt type cgroup (rw,clone_children)
# cat /mnt/cgroup.clone_children
1
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
@ 2011-12-14 7:09 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-14 7:09 UTC (permalink / raw)
To: Li Zefan
Cc: Tejun Heo, linux-kernel, jbottomley, cgroups, bsingharora, devel,
kamezawa.hiroyu
On 12/14/2011 06:29 AM, Li Zefan wrote:
> Tejun Heo wrote:
>> On Sun, Dec 11, 2011 at 03:45:37PM +0100, Glauber Costa wrote:
>>> There is no reason to have a flags field, and then a separate
>>> bool field just to indicate if the clone_children flag is set.
>>> Make it a flag
>>>
>>> Signed-off-by: Glauber Costa<glommer@parallels.com>
>>
>> Doesn't this change how remount conditions are checked?
>>
Well, I was thinking it wouldn't, because I patched all callers. But I
forget life is not always that simple: After you mentioned, I checked
and we do test for changes in the flag field explicitly on remount. So I
missed that, indeed.
> Right. Currently we can do this:
>
> # mount -t cgroup xxx /mnt
> # mount -o remount,clone_children /mnt
>
> with this patch, the above remount will fail.
>
> But..the current bevaiour of remount is a bit confusing in that remount
> with/without "clone_children" has no effect on anything:
>
> # mount -t cgroup -o clone_children xxx /mnt
> # cat /mnt/cgroup.clone_children
> 1
> # mount -o remount xxx /mnt
> # mount | grep cgroup
> xxx on /mnt type cgroup (rw,clone_children)
> # cat /mnt/cgroup.clone_children
> 1
That's indeed confusing, and it comes from the fact that we always
inherit clone_children from the parent - which is sane, IMHO. So this
flag only has any value in establishing the initial behaviour of the top
root cgroup. I wonder then if it wouldn't better to just be explicit and
fail in this case ?
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
@ 2011-12-14 7:09 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-14 7:09 UTC (permalink / raw)
To: Li Zefan
Cc: Tejun Heo, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A
On 12/14/2011 06:29 AM, Li Zefan wrote:
> Tejun Heo wrote:
>> On Sun, Dec 11, 2011 at 03:45:37PM +0100, Glauber Costa wrote:
>>> There is no reason to have a flags field, and then a separate
>>> bool field just to indicate if the clone_children flag is set.
>>> Make it a flag
>>>
>>> Signed-off-by: Glauber Costa<glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>
>> Doesn't this change how remount conditions are checked?
>>
Well, I was thinking it wouldn't, because I patched all callers. But I
forget life is not always that simple: After you mentioned, I checked
and we do test for changes in the flag field explicitly on remount. So I
missed that, indeed.
> Right. Currently we can do this:
>
> # mount -t cgroup xxx /mnt
> # mount -o remount,clone_children /mnt
>
> with this patch, the above remount will fail.
>
> But..the current bevaiour of remount is a bit confusing in that remount
> with/without "clone_children" has no effect on anything:
>
> # mount -t cgroup -o clone_children xxx /mnt
> # cat /mnt/cgroup.clone_children
> 1
> # mount -o remount xxx /mnt
> # mount | grep cgroup
> xxx on /mnt type cgroup (rw,clone_children)
> # cat /mnt/cgroup.clone_children
> 1
That's indeed confusing, and it comes from the fact that we always
inherit clone_children from the parent - which is sane, IMHO. So this
flag only has any value in establishing the initial behaviour of the top
root cgroup. I wonder then if it wouldn't better to just be explicit and
fail in this case ?
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
@ 2011-12-14 18:18 ` Tejun Heo
0 siblings, 0 replies; 37+ messages in thread
From: Tejun Heo @ 2011-12-14 18:18 UTC (permalink / raw)
To: Glauber Costa
Cc: Li Zefan, linux-kernel, jbottomley, cgroups, bsingharora, devel,
kamezawa.hiroyu
Hello,
On Wed, Dec 14, 2011 at 11:09:14AM +0400, Glauber Costa wrote:
> That's indeed confusing, and it comes from the fact that we always
> inherit clone_children from the parent - which is sane, IMHO. So
> this flag only has any value in establishing the initial behaviour
> of the top root cgroup. I wonder then if it wouldn't better to just
> be explicit and fail in this case ?
I don't think all current behaviors are sane and if not let's change
them, but those things have to be explicit with proper description and
rationale.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
@ 2011-12-14 18:18 ` Tejun Heo
0 siblings, 0 replies; 37+ messages in thread
From: Tejun Heo @ 2011-12-14 18:18 UTC (permalink / raw)
To: Glauber Costa
Cc: Li Zefan, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ,
cgroups-u79uwXL29TY76Z2rM5mHXA,
bsingharora-Re5JQEeQqe8AvxtiuMwx3w, devel-GEFAQzZX7r8dnm+yROfE0A,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A
Hello,
On Wed, Dec 14, 2011 at 11:09:14AM +0400, Glauber Costa wrote:
> That's indeed confusing, and it comes from the fact that we always
> inherit clone_children from the parent - which is sane, IMHO. So
> this flag only has any value in establishing the initial behaviour
> of the top root cgroup. I wonder then if it wouldn't better to just
> be explicit and fail in this case ?
I don't think all current behaviors are sane and if not let's change
them, but those things have to be explicit with proper description and
rationale.
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
2011-12-14 18:18 ` Tejun Heo
@ 2011-12-15 7:03 ` Glauber Costa
-1 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-15 7:03 UTC (permalink / raw)
To: Tejun Heo
Cc: Li Zefan, linux-kernel, jbottomley, cgroups, bsingharora, devel,
kamezawa.hiroyu
On 12/14/2011 10:18 PM, Tejun Heo wrote:
> Hello,
>
> On Wed, Dec 14, 2011 at 11:09:14AM +0400, Glauber Costa wrote:
>> That's indeed confusing, and it comes from the fact that we always
>> inherit clone_children from the parent - which is sane, IMHO. So
>> this flag only has any value in establishing the initial behaviour
>> of the top root cgroup. I wonder then if it wouldn't better to just
>> be explicit and fail in this case ?
>
> I don't think all current behaviors are sane and if not let's change
> them, but those things have to be explicit with proper description and
> rationale.
>
140 % agree to that. As I said, I wrongly believed it to be functionally
equivalent when I sent it, but missed the flags remount check.
If you believe the behavior we now get is saner, I can rewrite the
Changelog and resend it.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/3] make clone_children a flag
@ 2011-12-15 7:03 ` Glauber Costa
0 siblings, 0 replies; 37+ messages in thread
From: Glauber Costa @ 2011-12-15 7:03 UTC (permalink / raw)
To: Tejun Heo
Cc: Li Zefan, linux-kernel, jbottomley, cgroups, bsingharora, devel,
kamezawa.hiroyu
On 12/14/2011 10:18 PM, Tejun Heo wrote:
> Hello,
>
> On Wed, Dec 14, 2011 at 11:09:14AM +0400, Glauber Costa wrote:
>> That's indeed confusing, and it comes from the fact that we always
>> inherit clone_children from the parent - which is sane, IMHO. So
>> this flag only has any value in establishing the initial behaviour
>> of the top root cgroup. I wonder then if it wouldn't better to just
>> be explicit and fail in this case ?
>
> I don't think all current behaviors are sane and if not let's change
> them, but those things have to be explicit with proper description and
> rationale.
>
140 % agree to that. As I said, I wrongly believed it to be functionally
equivalent when I sent it, but missed the flags remount check.
If you believe the behavior we now get is saner, I can rewrite the
Changelog and resend it.
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2011-12-15 7:04 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-11 14:45 [PATCH 0/3] Simple cleanups for cgroups Glauber Costa
2011-12-11 14:45 ` Glauber Costa
2011-12-11 14:45 ` [PATCH] make clone_children a flag Glauber Costa
2011-12-11 14:45 ` Glauber Costa
2011-12-11 14:48 ` Glauber Costa
2011-12-11 14:48 ` Glauber Costa
2011-12-11 14:45 ` [PATCH 1/3] nitpick: make simple functions inline Glauber Costa
2011-12-11 14:45 ` Glauber Costa
2011-12-11 18:55 ` KOSAKI Motohiro
2011-12-11 18:55 ` KOSAKI Motohiro
2011-12-11 20:44 ` Glauber Costa
2011-12-11 20:44 ` Glauber Costa
2011-12-12 17:27 ` Tejun Heo
2011-12-12 17:27 ` Tejun Heo
2011-12-14 1:09 ` Li Zefan
2011-12-14 1:09 ` Li Zefan
2011-12-11 14:45 ` [PATCH 2/3] make clone_children a flag Glauber Costa
2011-12-11 14:45 ` Glauber Costa
2011-12-11 18:58 ` KOSAKI Motohiro
2011-12-11 18:58 ` KOSAKI Motohiro
2011-12-13 15:39 ` Tejun Heo
2011-12-13 15:39 ` Tejun Heo
2011-12-14 2:29 ` Li Zefan
2011-12-14 2:29 ` Li Zefan
2011-12-14 7:09 ` Glauber Costa
2011-12-14 7:09 ` Glauber Costa
2011-12-14 18:18 ` Tejun Heo
2011-12-14 18:18 ` Tejun Heo
2011-12-15 7:03 ` Glauber Costa
2011-12-15 7:03 ` Glauber Costa
2011-12-11 14:45 ` [PATCH 3/3] make 'none' field " Glauber Costa
2011-12-11 14:45 ` Glauber Costa
2011-12-11 18:59 ` KOSAKI Motohiro
2011-12-13 15:41 ` Tejun Heo
2011-12-13 15:41 ` Tejun Heo
2011-12-14 2:09 ` Li Zefan
2011-12-14 2:09 ` Li Zefan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.