Re: [PATCH 8/9] cgroup_freezer: add ->post_create() and ->pre_destroy() and track online state
Hello, On Thu, Nov 08, 2012 at 02:23:06PM +0100, Michal Hocko wrote: > On Sat 03-11-12 01:38:34, Tejun Heo wrote: > > A cgroup is online and visible to iteration between ->post_create() > > and ->pre_destroy(). This patch introduces CGROUP_FREEZER_ONLINE and > > toggles it from the newly added freezer_post_create() and > > freezer_pre_destroy() while holding freezer->lock such that a > > cgroup_freezer can be reilably distinguished to be online. This will > > be used by full hierarchy support. > > I am thinking whether freezer_pre_destroy is really needed. Once we > reach pre_destroy then there are no tasks nor any children in the group > so there is nobody to wake up if the group was frozen and the destroy > callback is called after synchronize_rcu so the traversing should be > safe. Yeah, it might be true, but I'd still like to keep the offlining in ->pre_destroy() so that it's symmetrical w/ ->post_create(). I'll rename and document the ops so that the roles of each are clearer. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 8/9] cgroup_freezer: add ->post_create() and ->pre_destroy() and track online state
Hello, Kamezawa. On Thu, Nov 08, 2012 at 01:48:25PM +0900, Kamezawa Hiroyuki wrote: > Could you explain what 'online' means here again, rather than changelog ? > BTW, 'online' is a shared concept, between post_create() and pre_destroy(), > among > developpers ? Is it new ? I'm prepping a patch to rename create, post_create, pre_destroy, destroy operations to allocate, online, offline, free, so yeah the terms and concepts will be used for all of cgroup. I'll update the docs in that patch. Thanks! -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 8/9] cgroup_freezer: add ->post_create() and ->pre_destroy() and track online state
On Sat 03-11-12 01:38:34, Tejun Heo wrote:
> A cgroup is online and visible to iteration between ->post_create()
> and ->pre_destroy(). This patch introduces CGROUP_FREEZER_ONLINE and
> toggles it from the newly added freezer_post_create() and
> freezer_pre_destroy() while holding freezer->lock such that a
> cgroup_freezer can be reilably distinguished to be online. This will
> be used by full hierarchy support.
I am thinking whether freezer_pre_destroy is really needed. Once we
reach pre_destroy then there are no tasks nor any children in the group
so there is nobody to wake up if the group was frozen and the destroy
callback is called after synchronize_rcu so the traversing should be
safe.
> ONLINE test is added to freezer_apply_state() but it currently doesn't
> make any difference as freezer_write() can only be called for an
> online cgroup.
>
> Adjusting system_freezing_cnt on destruction is moved from
> freezer_destroy() to the new freezer_pre_destroy() for consistency.
>
> This patch doesn't introduce any noticeable behavior change.
>
> Signed-off-by: Tejun Heo
Reviewed-by: Michal Hocko
> ---
> kernel/cgroup_freezer.c | 42 --
> 1 file changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
> index b8ad93c..4f12d31 100644
> --- a/kernel/cgroup_freezer.c
> +++ b/kernel/cgroup_freezer.c
> @@ -23,6 +23,7 @@
> #include
>
> enum freezer_state_flags {
> + CGROUP_FREEZER_ONLINE = (1 << 0), /* freezer is fully online */
> CGROUP_FREEZING_SELF= (1 << 1), /* this freezer is freezing */
> CGROUP_FREEZING_PARENT = (1 << 2), /* the parent freezer is freezing */
> CGROUP_FROZEN = (1 << 3), /* this and its descendants frozen
> */
> @@ -98,13 +99,45 @@ static struct cgroup_subsys_state *freezer_create(struct
> cgroup *cgroup)
> return >css;
> }
>
> -static void freezer_destroy(struct cgroup *cgroup)
> +/**
> + * freezer_post_create - commit creation of a freezer cgroup
> + * @cgroup: cgroup being created
> + *
> + * We're committing to creation of @cgroup. Mark it online.
> + */
> +static void freezer_post_create(struct cgroup *cgroup)
> {
> struct freezer *freezer = cgroup_freezer(cgroup);
>
> + spin_lock_irq(>lock);
> + freezer->state |= CGROUP_FREEZER_ONLINE;
> + spin_unlock_irq(>lock);
> +}
> +
> +/**
> + * freezer_pre_destroy - initiate destruction of @cgroup
> + * @cgroup: cgroup being destroyed
> + *
> + * @cgroup is going away. Mark it dead and decrement system_freezing_count
> + * if it was holding one.
> + */
> +static void freezer_pre_destroy(struct cgroup *cgroup)
> +{
> + struct freezer *freezer = cgroup_freezer(cgroup);
> +
> + spin_lock_irq(>lock);
> +
> if (freezer->state & CGROUP_FREEZING)
> atomic_dec(_freezing_cnt);
> - kfree(freezer);
> +
> + freezer->state = 0;
> +
> + spin_unlock_irq(>lock);
> +}
> +
> +static void freezer_destroy(struct cgroup *cgroup)
> +{
> + kfree(cgroup_freezer(cgroup));
> }
>
> /*
> @@ -260,6 +293,9 @@ static void freezer_apply_state(struct freezer *freezer,
> bool freeze,
> /* also synchronizes against task migration, see freezer_attach() */
> lockdep_assert_held(>lock);
>
> + if (!(freezer->state & CGROUP_FREEZER_ONLINE))
> + return;
> +
> if (freeze) {
> if (!(freezer->state & CGROUP_FREEZING))
> atomic_inc(_freezing_cnt);
> @@ -347,6 +383,8 @@ static struct cftype files[] = {
> struct cgroup_subsys freezer_subsys = {
> .name = "freezer",
> .create = freezer_create,
> + .post_create= freezer_post_create,
> + .pre_destroy= freezer_pre_destroy,
> .destroy= freezer_destroy,
> .subsys_id = freezer_subsys_id,
> .attach = freezer_attach,
> --
> 1.7.11.7
>
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 8/9] cgroup_freezer: add -post_create() and -pre_destroy() and track online state
On Sat 03-11-12 01:38:34, Tejun Heo wrote:
A cgroup is online and visible to iteration between -post_create()
and -pre_destroy(). This patch introduces CGROUP_FREEZER_ONLINE and
toggles it from the newly added freezer_post_create() and
freezer_pre_destroy() while holding freezer-lock such that a
cgroup_freezer can be reilably distinguished to be online. This will
be used by full hierarchy support.
I am thinking whether freezer_pre_destroy is really needed. Once we
reach pre_destroy then there are no tasks nor any children in the group
so there is nobody to wake up if the group was frozen and the destroy
callback is called after synchronize_rcu so the traversing should be
safe.
ONLINE test is added to freezer_apply_state() but it currently doesn't
make any difference as freezer_write() can only be called for an
online cgroup.
Adjusting system_freezing_cnt on destruction is moved from
freezer_destroy() to the new freezer_pre_destroy() for consistency.
This patch doesn't introduce any noticeable behavior change.
Signed-off-by: Tejun Heo t...@kernel.org
Reviewed-by: Michal Hocko mho...@suse.cz
---
kernel/cgroup_freezer.c | 42 --
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index b8ad93c..4f12d31 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -23,6 +23,7 @@
#include linux/seq_file.h
enum freezer_state_flags {
+ CGROUP_FREEZER_ONLINE = (1 0), /* freezer is fully online */
CGROUP_FREEZING_SELF= (1 1), /* this freezer is freezing */
CGROUP_FREEZING_PARENT = (1 2), /* the parent freezer is freezing */
CGROUP_FROZEN = (1 3), /* this and its descendants frozen
*/
@@ -98,13 +99,45 @@ static struct cgroup_subsys_state *freezer_create(struct
cgroup *cgroup)
return freezer-css;
}
-static void freezer_destroy(struct cgroup *cgroup)
+/**
+ * freezer_post_create - commit creation of a freezer cgroup
+ * @cgroup: cgroup being created
+ *
+ * We're committing to creation of @cgroup. Mark it online.
+ */
+static void freezer_post_create(struct cgroup *cgroup)
{
struct freezer *freezer = cgroup_freezer(cgroup);
+ spin_lock_irq(freezer-lock);
+ freezer-state |= CGROUP_FREEZER_ONLINE;
+ spin_unlock_irq(freezer-lock);
+}
+
+/**
+ * freezer_pre_destroy - initiate destruction of @cgroup
+ * @cgroup: cgroup being destroyed
+ *
+ * @cgroup is going away. Mark it dead and decrement system_freezing_count
+ * if it was holding one.
+ */
+static void freezer_pre_destroy(struct cgroup *cgroup)
+{
+ struct freezer *freezer = cgroup_freezer(cgroup);
+
+ spin_lock_irq(freezer-lock);
+
if (freezer-state CGROUP_FREEZING)
atomic_dec(system_freezing_cnt);
- kfree(freezer);
+
+ freezer-state = 0;
+
+ spin_unlock_irq(freezer-lock);
+}
+
+static void freezer_destroy(struct cgroup *cgroup)
+{
+ kfree(cgroup_freezer(cgroup));
}
/*
@@ -260,6 +293,9 @@ static void freezer_apply_state(struct freezer *freezer,
bool freeze,
/* also synchronizes against task migration, see freezer_attach() */
lockdep_assert_held(freezer-lock);
+ if (!(freezer-state CGROUP_FREEZER_ONLINE))
+ return;
+
if (freeze) {
if (!(freezer-state CGROUP_FREEZING))
atomic_inc(system_freezing_cnt);
@@ -347,6 +383,8 @@ static struct cftype files[] = {
struct cgroup_subsys freezer_subsys = {
.name = freezer,
.create = freezer_create,
+ .post_create= freezer_post_create,
+ .pre_destroy= freezer_pre_destroy,
.destroy= freezer_destroy,
.subsys_id = freezer_subsys_id,
.attach = freezer_attach,
--
1.7.11.7
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 8/9] cgroup_freezer: add -post_create() and -pre_destroy() and track online state
Hello, Kamezawa. On Thu, Nov 08, 2012 at 01:48:25PM +0900, Kamezawa Hiroyuki wrote: Could you explain what 'online' means here again, rather than changelog ? BTW, 'online' is a shared concept, between post_create() and pre_destroy(), among developpers ? Is it new ? I'm prepping a patch to rename create, post_create, pre_destroy, destroy operations to allocate, online, offline, free, so yeah the terms and concepts will be used for all of cgroup. I'll update the docs in that patch. Thanks! -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 8/9] cgroup_freezer: add -post_create() and -pre_destroy() and track online state
Hello, On Thu, Nov 08, 2012 at 02:23:06PM +0100, Michal Hocko wrote: On Sat 03-11-12 01:38:34, Tejun Heo wrote: A cgroup is online and visible to iteration between -post_create() and -pre_destroy(). This patch introduces CGROUP_FREEZER_ONLINE and toggles it from the newly added freezer_post_create() and freezer_pre_destroy() while holding freezer-lock such that a cgroup_freezer can be reilably distinguished to be online. This will be used by full hierarchy support. I am thinking whether freezer_pre_destroy is really needed. Once we reach pre_destroy then there are no tasks nor any children in the group so there is nobody to wake up if the group was frozen and the destroy callback is called after synchronize_rcu so the traversing should be safe. Yeah, it might be true, but I'd still like to keep the offlining in -pre_destroy() so that it's symmetrical w/ -post_create(). I'll rename and document the ops so that the roles of each are clearer. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 8/9] cgroup_freezer: add ->post_create() and ->pre_destroy() and track online state
(2012/11/03 17:38), Tejun Heo wrote:
> A cgroup is online and visible to iteration between ->post_create()
> and ->pre_destroy(). This patch introduces CGROUP_FREEZER_ONLINE and
> toggles it from the newly added freezer_post_create() and
> freezer_pre_destroy() while holding freezer->lock such that a
> cgroup_freezer can be reilably distinguished to be online. This will
> be used by full hierarchy support.
>
> ONLINE test is added to freezer_apply_state() but it currently doesn't
> make any difference as freezer_write() can only be called for an
> online cgroup.
>
> Adjusting system_freezing_cnt on destruction is moved from
> freezer_destroy() to the new freezer_pre_destroy() for consistency.
>
> This patch doesn't introduce any noticeable behavior change.
>
> Signed-off-by: Tejun Heo
> ---
> kernel/cgroup_freezer.c | 42 --
> 1 file changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
> index b8ad93c..4f12d31 100644
> --- a/kernel/cgroup_freezer.c
> +++ b/kernel/cgroup_freezer.c
> @@ -23,6 +23,7 @@
> #include
>
> enum freezer_state_flags {
> + CGROUP_FREEZER_ONLINE = (1 << 0), /* freezer is fully online */
Could you explain what 'online' means here again, rather than changelog ?
BTW, 'online' is a shared concept, between post_create() and pre_destroy(),
among
developpers ? Is it new ?
Anyway, the patch itself is simple.
Reviewed-by: KAMEZAWA Hiroyuki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 8/9] cgroup_freezer: add -post_create() and -pre_destroy() and track online state
(2012/11/03 17:38), Tejun Heo wrote:
A cgroup is online and visible to iteration between -post_create()
and -pre_destroy(). This patch introduces CGROUP_FREEZER_ONLINE and
toggles it from the newly added freezer_post_create() and
freezer_pre_destroy() while holding freezer-lock such that a
cgroup_freezer can be reilably distinguished to be online. This will
be used by full hierarchy support.
ONLINE test is added to freezer_apply_state() but it currently doesn't
make any difference as freezer_write() can only be called for an
online cgroup.
Adjusting system_freezing_cnt on destruction is moved from
freezer_destroy() to the new freezer_pre_destroy() for consistency.
This patch doesn't introduce any noticeable behavior change.
Signed-off-by: Tejun Heo t...@kernel.org
---
kernel/cgroup_freezer.c | 42 --
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index b8ad93c..4f12d31 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -23,6 +23,7 @@
#include linux/seq_file.h
enum freezer_state_flags {
+ CGROUP_FREEZER_ONLINE = (1 0), /* freezer is fully online */
Could you explain what 'online' means here again, rather than changelog ?
BTW, 'online' is a shared concept, between post_create() and pre_destroy(),
among
developpers ? Is it new ?
Anyway, the patch itself is simple.
Reviewed-by: KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
