Re: [PATCH 6/9] cgroup_freezer: make freezer->state mask of flags

2012-11-08 Thread Michal Hocko
On Thu 08-11-12 06:39:52, Tejun Heo wrote:
> Hello, Michal.
> 
> On Thu, Nov 08, 2012 at 11:39:28AM +0100, Michal Hocko wrote:
> > On Sat 03-11-12 01:38:32, Tejun Heo wrote:
> > > freezer->state was an enum value - one of THAWED, FREEZING and FROZEN.
> > > As the scheduled full hierarchy support requires more than one
> > > freezing condition, switch it to mask of flags.  If FREEZING is not
> > > set, it's thawed.  FREEZING is set if freezing or frozen.  If frozen,
> > > both FREEZING and FROZEN are set.  Now that tasks can be attached to
> > > an already frozen cgroup, this also makes freezing condition checks
> > > more natural.
> > > 
> > > This patch doesn't introduce any behavior change.
> > > 
> > > Signed-off-by: Tejun Heo 
> > 
> > I think it would be nicer to use freezer_state_flags enum rather than
> > unsigned int for the state. I would even expect gcc to complain about
> > that but it looks like -fstrict-enums is c++ specific (so long enum
> > safety).
> 
> But if you use it as flag bits, the resulting value no longer is
> inside the defined enum values.  Isn't that weird?

Ahh, there is always CGROUP_FREEZER_ONLINE which would overcomplicate
the possible and valid masks.
Please ignore this...

-- 
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 6/9] cgroup_freezer: make freezer->state mask of flags

2012-11-08 Thread Tejun Heo
Hello, Michal.

On Thu, Nov 08, 2012 at 11:39:28AM +0100, Michal Hocko wrote:
> On Sat 03-11-12 01:38:32, Tejun Heo wrote:
> > freezer->state was an enum value - one of THAWED, FREEZING and FROZEN.
> > As the scheduled full hierarchy support requires more than one
> > freezing condition, switch it to mask of flags.  If FREEZING is not
> > set, it's thawed.  FREEZING is set if freezing or frozen.  If frozen,
> > both FREEZING and FROZEN are set.  Now that tasks can be attached to
> > an already frozen cgroup, this also makes freezing condition checks
> > more natural.
> > 
> > This patch doesn't introduce any behavior change.
> > 
> > Signed-off-by: Tejun Heo 
> 
> I think it would be nicer to use freezer_state_flags enum rather than
> unsigned int for the state. I would even expect gcc to complain about
> that but it looks like -fstrict-enums is c++ specific (so long enum
> safety).

But if you use it as flag bits, the resulting value no longer is
inside the defined enum values.  Isn't that weird?

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 6/9] cgroup_freezer: make freezer->state mask of flags

2012-11-08 Thread Tejun Heo
Hello, Kamezawa.

On Thu, Nov 08, 2012 at 02:00:34PM +0900, Kamezawa Hiroyuki wrote:
> (2012/11/08 13:42), Tejun Heo wrote:
> >Hello, Kame.
> >
> >On Thu, Nov 08, 2012 at 01:37:50PM +0900, Kamezawa Hiroyuki wrote:
> >>How about
> >>enum {
> >>__CGROUP_FREEZING,
> >>__CGROUP_FROZEN,
> >>};
> >>
> >>#define CGROUP_FREEZER_STATE_MASK  0x3
> >>#define CGROUP_FREEZER_STATE(state) ((state) & CGROUP_FREEZER_STATE_MASK)
> >>#define CGROUP_THAW(state)  (CGROUP_FREEZER_STATE(state) == 0)
> >>#define CGROUP_FREEZING(state)  (CGROUP_FREEZER_STATE(state) == 
> >>__CGROUP_FREEZING)
> >>#define CGROUP_FROZEN(state)\
> >>(CGROUP_FREEZER_STATE(state) == (__CGROUP_FREEZING | __CGROUP_FROZEN))
> >
> >I think it's a bit overdone and we have cases where we test for
> >FREEZING regardless of FROZEN and cases where test for FREEZING &&
> >!FROZEN.  We can have, say, CGROUP_FREZING() and then
> >CGROUP_FREEZING_BUT_NOT_FROZEN(), but it feels more like obfuscation
> >than anything else.
> >
> 
> Hm, then, I'm glad if I can see what combinations of flags are valid and
> meanings of them in source code comments.

Ooh, the following comment is added by a later patch when the whole
thing is complete.

/*
 * A cgroup is freezing if any FREEZING flags are set.  FREEZING_SELF is
 * set if "FROZEN" is written to freezer.state cgroupfs file, and cleared
 * for "THAWED".  FREEZING_PARENT is set if the parent freezer is FREEZING
 * for whatever reason.  IOW, a cgroup has FREEZING_PARENT set if one of
 * its ancestors has FREEZING_SELF set.
 */

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 6/9] cgroup_freezer: make freezer->state mask of flags

2012-11-08 Thread Michal Hocko
On Sat 03-11-12 01:38:32, Tejun Heo wrote:
> freezer->state was an enum value - one of THAWED, FREEZING and FROZEN.
> As the scheduled full hierarchy support requires more than one
> freezing condition, switch it to mask of flags.  If FREEZING is not
> set, it's thawed.  FREEZING is set if freezing or frozen.  If frozen,
> both FREEZING and FROZEN are set.  Now that tasks can be attached to
> an already frozen cgroup, this also makes freezing condition checks
> more natural.
> 
> This patch doesn't introduce any behavior change.
> 
> Signed-off-by: Tejun Heo 

I think it would be nicer to use freezer_state_flags enum rather than
unsigned int for the state. I would even expect gcc to complain about
that but it looks like -fstrict-enums is c++ specific (so long enum
safety).

Anyway
Reviewed-by: Michal Hocko 

> ---
>  kernel/cgroup_freezer.c | 60 
> ++---
>  1 file changed, 27 insertions(+), 33 deletions(-)
> 
> diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
> index 2690830..e76aa9f 100644
> --- a/kernel/cgroup_freezer.c
> +++ b/kernel/cgroup_freezer.c
> @@ -22,15 +22,14 @@
>  #include 
>  #include 
>  
> -enum freezer_state {
> - CGROUP_THAWED = 0,
> - CGROUP_FREEZING,
> - CGROUP_FROZEN,
> +enum freezer_state_flags {
> + CGROUP_FREEZING = (1 << 1), /* this freezer is freezing */
> + CGROUP_FROZEN   = (1 << 3), /* this and its descendants frozen 
> */
>  };
>  
>  struct freezer {
>   struct cgroup_subsys_state  css;
> - enum freezer_state  state;
> + unsigned intstate;
>   spinlock_t  lock;
>  };
>  
> @@ -48,12 +47,10 @@ static inline struct freezer *task_freezer(struct 
> task_struct *task)
>  
>  bool cgroup_freezing(struct task_struct *task)
>  {
> - enum freezer_state state;
>   bool ret;
>  
>   rcu_read_lock();
> - state = task_freezer(task)->state;
> - ret = state == CGROUP_FREEZING || state == CGROUP_FROZEN;
> + ret = task_freezer(task)->state & CGROUP_FREEZING;
>   rcu_read_unlock();
>  
>   return ret;
> @@ -63,10 +60,13 @@ bool cgroup_freezing(struct task_struct *task)
>   * cgroups_write_string() limits the size of freezer state strings to
>   * CGROUP_LOCAL_BUFFER_SIZE
>   */
> -static const char *freezer_state_strs[] = {
> - "THAWED",
> - "FREEZING",
> - "FROZEN",
> +static const char *freezer_state_strs(unsigned int state)
> +{
> + if (state & CGROUP_FROZEN)
> + return "FROZEN";
> + if (state & CGROUP_FREEZING)
> + return "FREEZING";
> + return "THAWED";
>  };
>  
>  /*
> @@ -91,7 +91,6 @@ static struct cgroup_subsys_state *freezer_create(struct 
> cgroup *cgroup)
>   return ERR_PTR(-ENOMEM);
>  
>   spin_lock_init(>lock);
> - freezer->state = CGROUP_THAWED;
>   return >css;
>  }
>  
> @@ -99,7 +98,7 @@ static void freezer_destroy(struct cgroup *cgroup)
>  {
>   struct freezer *freezer = cgroup_freezer(cgroup);
>  
> - if (freezer->state != CGROUP_THAWED)
> + if (freezer->state & CGROUP_FREEZING)
>   atomic_dec(_freezing_cnt);
>   kfree(freezer);
>  }
> @@ -129,15 +128,13 @@ static void freezer_attach(struct cgroup *new_cgrp, 
> struct cgroup_taskset *tset)
>* Tasks in @tset are on @new_cgrp but may not conform to its
>* current state before executing the following - !frozen tasks may
>* be visible in a FROZEN cgroup and frozen tasks in a THAWED one.
> -  * This means that, to determine whether to freeze, one should test
> -  * whether the state equals THAWED.
>*/
>   cgroup_taskset_for_each(task, new_cgrp, tset) {
> - if (freezer->state == CGROUP_THAWED) {
> + if (!(freezer->state & CGROUP_FREEZING)) {
>   __thaw_task(task);
>   } else {
>   freeze_task(task);
> - freezer->state = CGROUP_FREEZING;
> + freezer->state &= ~CGROUP_FROZEN;
>   }
>   }
>  
> @@ -159,11 +156,7 @@ static void freezer_fork(struct task_struct *task)
>   goto out;
>  
>   spin_lock_irq(>lock);
> - /*
> -  * @task might have been just migrated into a FROZEN cgroup.  Test
> -  * equality with THAWED.  Read the comment in freezer_attach().
> -  */
> - if (freezer->state != CGROUP_THAWED)
> + if (freezer->state & CGROUP_FREEZING)
>   freeze_task(task);
>   spin_unlock_irq(>lock);
>  out:
> @@ -184,7 +177,8 @@ static void update_if_frozen(struct freezer *freezer)
>   struct cgroup_iter it;
>   struct task_struct *task;
>  
> - if (freezer->state != CGROUP_FREEZING)
> + if (!(freezer->state & CGROUP_FREEZING) ||
> + (freezer->state & CGROUP_FROZEN))
>   return;
>  
>   cgroup_iter_start(cgroup, );
> @@ -202,7 +196,7 @@ static void update_if_frozen(struct 

Re: [PATCH 6/9] cgroup_freezer: make freezer-state mask of flags

2012-11-08 Thread Michal Hocko
On Sat 03-11-12 01:38:32, Tejun Heo wrote:
 freezer-state was an enum value - one of THAWED, FREEZING and FROZEN.
 As the scheduled full hierarchy support requires more than one
 freezing condition, switch it to mask of flags.  If FREEZING is not
 set, it's thawed.  FREEZING is set if freezing or frozen.  If frozen,
 both FREEZING and FROZEN are set.  Now that tasks can be attached to
 an already frozen cgroup, this also makes freezing condition checks
 more natural.
 
 This patch doesn't introduce any behavior change.
 
 Signed-off-by: Tejun Heo t...@kernel.org

I think it would be nicer to use freezer_state_flags enum rather than
unsigned int for the state. I would even expect gcc to complain about
that but it looks like -fstrict-enums is c++ specific (so long enum
safety).

Anyway
Reviewed-by: Michal Hocko mho...@suse.cz

 ---
  kernel/cgroup_freezer.c | 60 
 ++---
  1 file changed, 27 insertions(+), 33 deletions(-)
 
 diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
 index 2690830..e76aa9f 100644
 --- a/kernel/cgroup_freezer.c
 +++ b/kernel/cgroup_freezer.c
 @@ -22,15 +22,14 @@
  #include linux/freezer.h
  #include linux/seq_file.h
  
 -enum freezer_state {
 - CGROUP_THAWED = 0,
 - CGROUP_FREEZING,
 - CGROUP_FROZEN,
 +enum freezer_state_flags {
 + CGROUP_FREEZING = (1  1), /* this freezer is freezing */
 + CGROUP_FROZEN   = (1  3), /* this and its descendants frozen 
 */
  };
  
  struct freezer {
   struct cgroup_subsys_state  css;
 - enum freezer_state  state;
 + unsigned intstate;
   spinlock_t  lock;
  };
  
 @@ -48,12 +47,10 @@ static inline struct freezer *task_freezer(struct 
 task_struct *task)
  
  bool cgroup_freezing(struct task_struct *task)
  {
 - enum freezer_state state;
   bool ret;
  
   rcu_read_lock();
 - state = task_freezer(task)-state;
 - ret = state == CGROUP_FREEZING || state == CGROUP_FROZEN;
 + ret = task_freezer(task)-state  CGROUP_FREEZING;
   rcu_read_unlock();
  
   return ret;
 @@ -63,10 +60,13 @@ bool cgroup_freezing(struct task_struct *task)
   * cgroups_write_string() limits the size of freezer state strings to
   * CGROUP_LOCAL_BUFFER_SIZE
   */
 -static const char *freezer_state_strs[] = {
 - THAWED,
 - FREEZING,
 - FROZEN,
 +static const char *freezer_state_strs(unsigned int state)
 +{
 + if (state  CGROUP_FROZEN)
 + return FROZEN;
 + if (state  CGROUP_FREEZING)
 + return FREEZING;
 + return THAWED;
  };
  
  /*
 @@ -91,7 +91,6 @@ static struct cgroup_subsys_state *freezer_create(struct 
 cgroup *cgroup)
   return ERR_PTR(-ENOMEM);
  
   spin_lock_init(freezer-lock);
 - freezer-state = CGROUP_THAWED;
   return freezer-css;
  }
  
 @@ -99,7 +98,7 @@ static void freezer_destroy(struct cgroup *cgroup)
  {
   struct freezer *freezer = cgroup_freezer(cgroup);
  
 - if (freezer-state != CGROUP_THAWED)
 + if (freezer-state  CGROUP_FREEZING)
   atomic_dec(system_freezing_cnt);
   kfree(freezer);
  }
 @@ -129,15 +128,13 @@ static void freezer_attach(struct cgroup *new_cgrp, 
 struct cgroup_taskset *tset)
* Tasks in @tset are on @new_cgrp but may not conform to its
* current state before executing the following - !frozen tasks may
* be visible in a FROZEN cgroup and frozen tasks in a THAWED one.
 -  * This means that, to determine whether to freeze, one should test
 -  * whether the state equals THAWED.
*/
   cgroup_taskset_for_each(task, new_cgrp, tset) {
 - if (freezer-state == CGROUP_THAWED) {
 + if (!(freezer-state  CGROUP_FREEZING)) {
   __thaw_task(task);
   } else {
   freeze_task(task);
 - freezer-state = CGROUP_FREEZING;
 + freezer-state = ~CGROUP_FROZEN;
   }
   }
  
 @@ -159,11 +156,7 @@ static void freezer_fork(struct task_struct *task)
   goto out;
  
   spin_lock_irq(freezer-lock);
 - /*
 -  * @task might have been just migrated into a FROZEN cgroup.  Test
 -  * equality with THAWED.  Read the comment in freezer_attach().
 -  */
 - if (freezer-state != CGROUP_THAWED)
 + if (freezer-state  CGROUP_FREEZING)
   freeze_task(task);
   spin_unlock_irq(freezer-lock);
  out:
 @@ -184,7 +177,8 @@ static void update_if_frozen(struct freezer *freezer)
   struct cgroup_iter it;
   struct task_struct *task;
  
 - if (freezer-state != CGROUP_FREEZING)
 + if (!(freezer-state  CGROUP_FREEZING) ||
 + (freezer-state  CGROUP_FROZEN))
   return;
  
   cgroup_iter_start(cgroup, it);
 @@ -202,7 +196,7 @@ static void update_if_frozen(struct freezer *freezer)
   }
   }
  
 - freezer-state = CGROUP_FROZEN;
 

Re: [PATCH 6/9] cgroup_freezer: make freezer-state mask of flags

2012-11-08 Thread Tejun Heo
Hello, Kamezawa.

On Thu, Nov 08, 2012 at 02:00:34PM +0900, Kamezawa Hiroyuki wrote:
 (2012/11/08 13:42), Tejun Heo wrote:
 Hello, Kame.
 
 On Thu, Nov 08, 2012 at 01:37:50PM +0900, Kamezawa Hiroyuki wrote:
 How about
 enum {
 __CGROUP_FREEZING,
 __CGROUP_FROZEN,
 };
 
 #define CGROUP_FREEZER_STATE_MASK  0x3
 #define CGROUP_FREEZER_STATE(state) ((state)  CGROUP_FREEZER_STATE_MASK)
 #define CGROUP_THAW(state)  (CGROUP_FREEZER_STATE(state) == 0)
 #define CGROUP_FREEZING(state)  (CGROUP_FREEZER_STATE(state) == 
 __CGROUP_FREEZING)
 #define CGROUP_FROZEN(state)\
 (CGROUP_FREEZER_STATE(state) == (__CGROUP_FREEZING | __CGROUP_FROZEN))
 
 I think it's a bit overdone and we have cases where we test for
 FREEZING regardless of FROZEN and cases where test for FREEZING 
 !FROZEN.  We can have, say, CGROUP_FREZING() and then
 CGROUP_FREEZING_BUT_NOT_FROZEN(), but it feels more like obfuscation
 than anything else.
 
 
 Hm, then, I'm glad if I can see what combinations of flags are valid and
 meanings of them in source code comments.

Ooh, the following comment is added by a later patch when the whole
thing is complete.

/*
 * A cgroup is freezing if any FREEZING flags are set.  FREEZING_SELF is
 * set if FROZEN is written to freezer.state cgroupfs file, and cleared
 * for THAWED.  FREEZING_PARENT is set if the parent freezer is FREEZING
 * for whatever reason.  IOW, a cgroup has FREEZING_PARENT set if one of
 * its ancestors has FREEZING_SELF set.
 */

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 6/9] cgroup_freezer: make freezer-state mask of flags

2012-11-08 Thread Tejun Heo
Hello, Michal.

On Thu, Nov 08, 2012 at 11:39:28AM +0100, Michal Hocko wrote:
 On Sat 03-11-12 01:38:32, Tejun Heo wrote:
  freezer-state was an enum value - one of THAWED, FREEZING and FROZEN.
  As the scheduled full hierarchy support requires more than one
  freezing condition, switch it to mask of flags.  If FREEZING is not
  set, it's thawed.  FREEZING is set if freezing or frozen.  If frozen,
  both FREEZING and FROZEN are set.  Now that tasks can be attached to
  an already frozen cgroup, this also makes freezing condition checks
  more natural.
  
  This patch doesn't introduce any behavior change.
  
  Signed-off-by: Tejun Heo t...@kernel.org
 
 I think it would be nicer to use freezer_state_flags enum rather than
 unsigned int for the state. I would even expect gcc to complain about
 that but it looks like -fstrict-enums is c++ specific (so long enum
 safety).

But if you use it as flag bits, the resulting value no longer is
inside the defined enum values.  Isn't that weird?

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 6/9] cgroup_freezer: make freezer-state mask of flags

2012-11-08 Thread Michal Hocko
On Thu 08-11-12 06:39:52, Tejun Heo wrote:
 Hello, Michal.
 
 On Thu, Nov 08, 2012 at 11:39:28AM +0100, Michal Hocko wrote:
  On Sat 03-11-12 01:38:32, Tejun Heo wrote:
   freezer-state was an enum value - one of THAWED, FREEZING and FROZEN.
   As the scheduled full hierarchy support requires more than one
   freezing condition, switch it to mask of flags.  If FREEZING is not
   set, it's thawed.  FREEZING is set if freezing or frozen.  If frozen,
   both FREEZING and FROZEN are set.  Now that tasks can be attached to
   an already frozen cgroup, this also makes freezing condition checks
   more natural.
   
   This patch doesn't introduce any behavior change.
   
   Signed-off-by: Tejun Heo t...@kernel.org
  
  I think it would be nicer to use freezer_state_flags enum rather than
  unsigned int for the state. I would even expect gcc to complain about
  that but it looks like -fstrict-enums is c++ specific (so long enum
  safety).
 
 But if you use it as flag bits, the resulting value no longer is
 inside the defined enum values.  Isn't that weird?

Ahh, there is always CGROUP_FREEZER_ONLINE which would overcomplicate
the possible and valid masks.
Please ignore this...

-- 
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 6/9] cgroup_freezer: make freezer->state mask of flags

2012-11-07 Thread Kamezawa Hiroyuki

(2012/11/08 13:42), Tejun Heo wrote:

Hello, Kame.

On Thu, Nov 08, 2012 at 01:37:50PM +0900, Kamezawa Hiroyuki wrote:

How about
enum {
__CGROUP_FREEZING,
__CGROUP_FROZEN,
};

#define CGROUP_FREEZER_STATE_MASK  0x3
#define CGROUP_FREEZER_STATE(state) ((state) & CGROUP_FREEZER_STATE_MASK)
#define CGROUP_THAW(state)  (CGROUP_FREEZER_STATE(state) == 0)
#define CGROUP_FREEZING(state)  (CGROUP_FREEZER_STATE(state) == 
__CGROUP_FREEZING)
#define CGROUP_FROZEN(state)\
(CGROUP_FREEZER_STATE(state) == (__CGROUP_FREEZING | __CGROUP_FROZEN))


I think it's a bit overdone and we have cases where we test for
FREEZING regardless of FROZEN and cases where test for FREEZING &&
!FROZEN.  We can have, say, CGROUP_FREZING() and then
CGROUP_FREEZING_BUT_NOT_FROZEN(), but it feels more like obfuscation
than anything else.



Hm, then, I'm glad if I can see what combinations of flags are valid and
meanings of them in source code comments.

Anyway,
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 6/9] cgroup_freezer: make freezer->state mask of flags

2012-11-07 Thread Tejun Heo
Hello, Kame.

On Thu, Nov 08, 2012 at 01:37:50PM +0900, Kamezawa Hiroyuki wrote:
> How about
> enum {
>__CGROUP_FREEZING,
>__CGROUP_FROZEN,
> };
> 
> #define CGROUP_FREEZER_STATE_MASK  0x3
> #define CGROUP_FREEZER_STATE(state)   ((state) & CGROUP_FREEZER_STATE_MASK)
> #define CGROUP_THAW(state)(CGROUP_FREEZER_STATE(state) == 0)
> #define CGROUP_FREEZING(state)(CGROUP_FREEZER_STATE(state) == 
> __CGROUP_FREEZING)
> #define CGROUP_FROZEN(state)\
>   (CGROUP_FREEZER_STATE(state) == (__CGROUP_FREEZING | __CGROUP_FROZEN))

I think it's a bit overdone and we have cases where we test for
FREEZING regardless of FROZEN and cases where test for FREEZING &&
!FROZEN.  We can have, say, CGROUP_FREZING() and then
CGROUP_FREEZING_BUT_NOT_FROZEN(), but it feels more like obfuscation
than anything else.

> >@@ -290,9 +284,9 @@ static int freezer_write(struct cgroup *cgroup, struct 
> >cftype *cft,
> >  {
> > bool freeze;
> >
> >-if (strcmp(buffer, freezer_state_strs[CGROUP_THAWED]) == 0)
> >+if (strcmp(buffer, freezer_state_strs(0)) == 0)
> 
> Can't we have a name for "0" ?

We can define CGROUP_THAWED to be zero.  I'd rather keep it explicitly
zero tho.  freezer state is mask of freezing and frozen flags and no
flag set means thawed.

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 6/9] cgroup_freezer: make freezer->state mask of flags

2012-11-07 Thread Kamezawa Hiroyuki

(2012/11/03 17:38), Tejun Heo wrote:

freezer->state was an enum value - one of THAWED, FREEZING and FROZEN.
As the scheduled full hierarchy support requires more than one
freezing condition, switch it to mask of flags.  If FREEZING is not
set, it's thawed.  FREEZING is set if freezing or frozen.  If frozen,
both FREEZING and FROZEN are set.  Now that tasks can be attached to
an already frozen cgroup, this also makes freezing condition checks
more natural.

This patch doesn't introduce any behavior change.

Signed-off-by: Tejun Heo 
---
  kernel/cgroup_freezer.c | 60 ++---
  1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 2690830..e76aa9f 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -22,15 +22,14 @@
  #include 
  #include 

-enum freezer_state {
-   CGROUP_THAWED = 0,
-   CGROUP_FREEZING,
-   CGROUP_FROZEN,
+enum freezer_state_flags {
+   CGROUP_FREEZING = (1 << 1), /* this freezer is freezing */
+   CGROUP_FROZEN   = (1 << 3), /* this and its descendants frozen 
*/
  };

  struct freezer {
struct cgroup_subsys_state  css;
-   enum freezer_state  state;
+   unsigned intstate;
spinlock_t  lock;
  };

@@ -48,12 +47,10 @@ static inline struct freezer *task_freezer(struct 
task_struct *task)

  bool cgroup_freezing(struct task_struct *task)
  {
-   enum freezer_state state;
bool ret;

rcu_read_lock();
-   state = task_freezer(task)->state;
-   ret = state == CGROUP_FREEZING || state == CGROUP_FROZEN;
+   ret = task_freezer(task)->state & CGROUP_FREEZING;
rcu_read_unlock();

return ret;
@@ -63,10 +60,13 @@ bool cgroup_freezing(struct task_struct *task)
   * cgroups_write_string() limits the size of freezer state strings to
   * CGROUP_LOCAL_BUFFER_SIZE
   */
-static const char *freezer_state_strs[] = {
-   "THAWED",
-   "FREEZING",
-   "FROZEN",
+static const char *freezer_state_strs(unsigned int state)
+{
+   if (state & CGROUP_FROZEN)
+   return "FROZEN";
+   if (state & CGROUP_FREEZING)
+   return "FREEZING";
+   return "THAWED";
  };

  /*
@@ -91,7 +91,6 @@ static struct cgroup_subsys_state *freezer_create(struct 
cgroup *cgroup)
return ERR_PTR(-ENOMEM);

spin_lock_init(>lock);
-   freezer->state = CGROUP_THAWED;
return >css;
  }

@@ -99,7 +98,7 @@ static void freezer_destroy(struct cgroup *cgroup)
  {
struct freezer *freezer = cgroup_freezer(cgroup);

-   if (freezer->state != CGROUP_THAWED)
+   if (freezer->state & CGROUP_FREEZING)
atomic_dec(_freezing_cnt);
kfree(freezer);
  }
@@ -129,15 +128,13 @@ static void freezer_attach(struct cgroup *new_cgrp, 
struct cgroup_taskset *tset)
 * Tasks in @tset are on @new_cgrp but may not conform to its
 * current state before executing the following - !frozen tasks may
 * be visible in a FROZEN cgroup and frozen tasks in a THAWED one.
-* This means that, to determine whether to freeze, one should test
-* whether the state equals THAWED.
 */
cgroup_taskset_for_each(task, new_cgrp, tset) {
-   if (freezer->state == CGROUP_THAWED) {
+   if (!(freezer->state & CGROUP_FREEZING)) {
__thaw_task(task);
} else {
freeze_task(task);
-   freezer->state = CGROUP_FREEZING;
+   freezer->state &= ~CGROUP_FROZEN;
}
}

@@ -159,11 +156,7 @@ static void freezer_fork(struct task_struct *task)
goto out;

spin_lock_irq(>lock);
-   /*
-* @task might have been just migrated into a FROZEN cgroup.  Test
-* equality with THAWED.  Read the comment in freezer_attach().
-*/
-   if (freezer->state != CGROUP_THAWED)
+   if (freezer->state & CGROUP_FREEZING)
freeze_task(task);
spin_unlock_irq(>lock);
  out:
@@ -184,7 +177,8 @@ static void update_if_frozen(struct freezer *freezer)
struct cgroup_iter it;
struct task_struct *task;

-   if (freezer->state != CGROUP_FREEZING)
+   if (!(freezer->state & CGROUP_FREEZING) ||
+   (freezer->state & CGROUP_FROZEN))
return;


Hmm

How about
enum {
   __CGROUP_FREEZING,
   __CGROUP_FROZEN,
};

#define CGROUP_FREEZER_STATE_MASK  0x3
#define CGROUP_FREEZER_STATE(state) ((state) & CGROUP_FREEZER_STATE_MASK)
#define CGROUP_THAW(state)  (CGROUP_FREEZER_STATE(state) == 0)
#define CGROUP_FREEZING(state)  (CGROUP_FREEZER_STATE(state) == 
__CGROUP_FREEZING)
#define CGROUP_FROZEN(state)\
(CGROUP_FREEZER_STATE(state) == (__CGROUP_FREEZING | __CGROUP_FROZEN))


cgroup_iter_start(cgroup, );

Re: [PATCH 6/9] cgroup_freezer: make freezer-state mask of flags

2012-11-07 Thread Kamezawa Hiroyuki

(2012/11/03 17:38), Tejun Heo wrote:

freezer-state was an enum value - one of THAWED, FREEZING and FROZEN.
As the scheduled full hierarchy support requires more than one
freezing condition, switch it to mask of flags.  If FREEZING is not
set, it's thawed.  FREEZING is set if freezing or frozen.  If frozen,
both FREEZING and FROZEN are set.  Now that tasks can be attached to
an already frozen cgroup, this also makes freezing condition checks
more natural.

This patch doesn't introduce any behavior change.

Signed-off-by: Tejun Heo t...@kernel.org
---
  kernel/cgroup_freezer.c | 60 ++---
  1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 2690830..e76aa9f 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -22,15 +22,14 @@
  #include linux/freezer.h
  #include linux/seq_file.h

-enum freezer_state {
-   CGROUP_THAWED = 0,
-   CGROUP_FREEZING,
-   CGROUP_FROZEN,
+enum freezer_state_flags {
+   CGROUP_FREEZING = (1  1), /* this freezer is freezing */
+   CGROUP_FROZEN   = (1  3), /* this and its descendants frozen 
*/
  };

  struct freezer {
struct cgroup_subsys_state  css;
-   enum freezer_state  state;
+   unsigned intstate;
spinlock_t  lock;
  };

@@ -48,12 +47,10 @@ static inline struct freezer *task_freezer(struct 
task_struct *task)

  bool cgroup_freezing(struct task_struct *task)
  {
-   enum freezer_state state;
bool ret;

rcu_read_lock();
-   state = task_freezer(task)-state;
-   ret = state == CGROUP_FREEZING || state == CGROUP_FROZEN;
+   ret = task_freezer(task)-state  CGROUP_FREEZING;
rcu_read_unlock();

return ret;
@@ -63,10 +60,13 @@ bool cgroup_freezing(struct task_struct *task)
   * cgroups_write_string() limits the size of freezer state strings to
   * CGROUP_LOCAL_BUFFER_SIZE
   */
-static const char *freezer_state_strs[] = {
-   THAWED,
-   FREEZING,
-   FROZEN,
+static const char *freezer_state_strs(unsigned int state)
+{
+   if (state  CGROUP_FROZEN)
+   return FROZEN;
+   if (state  CGROUP_FREEZING)
+   return FREEZING;
+   return THAWED;
  };

  /*
@@ -91,7 +91,6 @@ static struct cgroup_subsys_state *freezer_create(struct 
cgroup *cgroup)
return ERR_PTR(-ENOMEM);

spin_lock_init(freezer-lock);
-   freezer-state = CGROUP_THAWED;
return freezer-css;
  }

@@ -99,7 +98,7 @@ static void freezer_destroy(struct cgroup *cgroup)
  {
struct freezer *freezer = cgroup_freezer(cgroup);

-   if (freezer-state != CGROUP_THAWED)
+   if (freezer-state  CGROUP_FREEZING)
atomic_dec(system_freezing_cnt);
kfree(freezer);
  }
@@ -129,15 +128,13 @@ static void freezer_attach(struct cgroup *new_cgrp, 
struct cgroup_taskset *tset)
 * Tasks in @tset are on @new_cgrp but may not conform to its
 * current state before executing the following - !frozen tasks may
 * be visible in a FROZEN cgroup and frozen tasks in a THAWED one.
-* This means that, to determine whether to freeze, one should test
-* whether the state equals THAWED.
 */
cgroup_taskset_for_each(task, new_cgrp, tset) {
-   if (freezer-state == CGROUP_THAWED) {
+   if (!(freezer-state  CGROUP_FREEZING)) {
__thaw_task(task);
} else {
freeze_task(task);
-   freezer-state = CGROUP_FREEZING;
+   freezer-state = ~CGROUP_FROZEN;
}
}

@@ -159,11 +156,7 @@ static void freezer_fork(struct task_struct *task)
goto out;

spin_lock_irq(freezer-lock);
-   /*
-* @task might have been just migrated into a FROZEN cgroup.  Test
-* equality with THAWED.  Read the comment in freezer_attach().
-*/
-   if (freezer-state != CGROUP_THAWED)
+   if (freezer-state  CGROUP_FREEZING)
freeze_task(task);
spin_unlock_irq(freezer-lock);
  out:
@@ -184,7 +177,8 @@ static void update_if_frozen(struct freezer *freezer)
struct cgroup_iter it;
struct task_struct *task;

-   if (freezer-state != CGROUP_FREEZING)
+   if (!(freezer-state  CGROUP_FREEZING) ||
+   (freezer-state  CGROUP_FROZEN))
return;


Hmm

How about
enum {
   __CGROUP_FREEZING,
   __CGROUP_FROZEN,
};

#define CGROUP_FREEZER_STATE_MASK  0x3
#define CGROUP_FREEZER_STATE(state) ((state)  CGROUP_FREEZER_STATE_MASK)
#define CGROUP_THAW(state)  (CGROUP_FREEZER_STATE(state) == 0)
#define CGROUP_FREEZING(state)  (CGROUP_FREEZER_STATE(state) == 
__CGROUP_FREEZING)
#define CGROUP_FROZEN(state)\
(CGROUP_FREEZER_STATE(state) == (__CGROUP_FREEZING | __CGROUP_FROZEN))



Re: [PATCH 6/9] cgroup_freezer: make freezer-state mask of flags

2012-11-07 Thread Tejun Heo
Hello, Kame.

On Thu, Nov 08, 2012 at 01:37:50PM +0900, Kamezawa Hiroyuki wrote:
 How about
 enum {
__CGROUP_FREEZING,
__CGROUP_FROZEN,
 };
 
 #define CGROUP_FREEZER_STATE_MASK  0x3
 #define CGROUP_FREEZER_STATE(state)   ((state)  CGROUP_FREEZER_STATE_MASK)
 #define CGROUP_THAW(state)(CGROUP_FREEZER_STATE(state) == 0)
 #define CGROUP_FREEZING(state)(CGROUP_FREEZER_STATE(state) == 
 __CGROUP_FREEZING)
 #define CGROUP_FROZEN(state)\
   (CGROUP_FREEZER_STATE(state) == (__CGROUP_FREEZING | __CGROUP_FROZEN))

I think it's a bit overdone and we have cases where we test for
FREEZING regardless of FROZEN and cases where test for FREEZING 
!FROZEN.  We can have, say, CGROUP_FREZING() and then
CGROUP_FREEZING_BUT_NOT_FROZEN(), but it feels more like obfuscation
than anything else.

 @@ -290,9 +284,9 @@ static int freezer_write(struct cgroup *cgroup, struct 
 cftype *cft,
   {
  bool freeze;
 
 -if (strcmp(buffer, freezer_state_strs[CGROUP_THAWED]) == 0)
 +if (strcmp(buffer, freezer_state_strs(0)) == 0)
 
 Can't we have a name for 0 ?

We can define CGROUP_THAWED to be zero.  I'd rather keep it explicitly
zero tho.  freezer state is mask of freezing and frozen flags and no
flag set means thawed.

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 6/9] cgroup_freezer: make freezer-state mask of flags

2012-11-07 Thread Kamezawa Hiroyuki

(2012/11/08 13:42), Tejun Heo wrote:

Hello, Kame.

On Thu, Nov 08, 2012 at 01:37:50PM +0900, Kamezawa Hiroyuki wrote:

How about
enum {
__CGROUP_FREEZING,
__CGROUP_FROZEN,
};

#define CGROUP_FREEZER_STATE_MASK  0x3
#define CGROUP_FREEZER_STATE(state) ((state)  CGROUP_FREEZER_STATE_MASK)
#define CGROUP_THAW(state)  (CGROUP_FREEZER_STATE(state) == 0)
#define CGROUP_FREEZING(state)  (CGROUP_FREEZER_STATE(state) == 
__CGROUP_FREEZING)
#define CGROUP_FROZEN(state)\
(CGROUP_FREEZER_STATE(state) == (__CGROUP_FREEZING | __CGROUP_FROZEN))


I think it's a bit overdone and we have cases where we test for
FREEZING regardless of FROZEN and cases where test for FREEZING 
!FROZEN.  We can have, say, CGROUP_FREZING() and then
CGROUP_FREEZING_BUT_NOT_FROZEN(), but it feels more like obfuscation
than anything else.



Hm, then, I'm glad if I can see what combinations of flags are valid and
meanings of them in source code comments.

Anyway,
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/


[PATCH 6/9] cgroup_freezer: make freezer->state mask of flags

2012-11-03 Thread Tejun Heo
freezer->state was an enum value - one of THAWED, FREEZING and FROZEN.
As the scheduled full hierarchy support requires more than one
freezing condition, switch it to mask of flags.  If FREEZING is not
set, it's thawed.  FREEZING is set if freezing or frozen.  If frozen,
both FREEZING and FROZEN are set.  Now that tasks can be attached to
an already frozen cgroup, this also makes freezing condition checks
more natural.

This patch doesn't introduce any behavior change.

Signed-off-by: Tejun Heo 
---
 kernel/cgroup_freezer.c | 60 ++---
 1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 2690830..e76aa9f 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -22,15 +22,14 @@
 #include 
 #include 
 
-enum freezer_state {
-   CGROUP_THAWED = 0,
-   CGROUP_FREEZING,
-   CGROUP_FROZEN,
+enum freezer_state_flags {
+   CGROUP_FREEZING = (1 << 1), /* this freezer is freezing */
+   CGROUP_FROZEN   = (1 << 3), /* this and its descendants frozen 
*/
 };
 
 struct freezer {
struct cgroup_subsys_state  css;
-   enum freezer_state  state;
+   unsigned intstate;
spinlock_t  lock;
 };
 
@@ -48,12 +47,10 @@ static inline struct freezer *task_freezer(struct 
task_struct *task)
 
 bool cgroup_freezing(struct task_struct *task)
 {
-   enum freezer_state state;
bool ret;
 
rcu_read_lock();
-   state = task_freezer(task)->state;
-   ret = state == CGROUP_FREEZING || state == CGROUP_FROZEN;
+   ret = task_freezer(task)->state & CGROUP_FREEZING;
rcu_read_unlock();
 
return ret;
@@ -63,10 +60,13 @@ bool cgroup_freezing(struct task_struct *task)
  * cgroups_write_string() limits the size of freezer state strings to
  * CGROUP_LOCAL_BUFFER_SIZE
  */
-static const char *freezer_state_strs[] = {
-   "THAWED",
-   "FREEZING",
-   "FROZEN",
+static const char *freezer_state_strs(unsigned int state)
+{
+   if (state & CGROUP_FROZEN)
+   return "FROZEN";
+   if (state & CGROUP_FREEZING)
+   return "FREEZING";
+   return "THAWED";
 };
 
 /*
@@ -91,7 +91,6 @@ static struct cgroup_subsys_state *freezer_create(struct 
cgroup *cgroup)
return ERR_PTR(-ENOMEM);
 
spin_lock_init(>lock);
-   freezer->state = CGROUP_THAWED;
return >css;
 }
 
@@ -99,7 +98,7 @@ static void freezer_destroy(struct cgroup *cgroup)
 {
struct freezer *freezer = cgroup_freezer(cgroup);
 
-   if (freezer->state != CGROUP_THAWED)
+   if (freezer->state & CGROUP_FREEZING)
atomic_dec(_freezing_cnt);
kfree(freezer);
 }
@@ -129,15 +128,13 @@ static void freezer_attach(struct cgroup *new_cgrp, 
struct cgroup_taskset *tset)
 * Tasks in @tset are on @new_cgrp but may not conform to its
 * current state before executing the following - !frozen tasks may
 * be visible in a FROZEN cgroup and frozen tasks in a THAWED one.
-* This means that, to determine whether to freeze, one should test
-* whether the state equals THAWED.
 */
cgroup_taskset_for_each(task, new_cgrp, tset) {
-   if (freezer->state == CGROUP_THAWED) {
+   if (!(freezer->state & CGROUP_FREEZING)) {
__thaw_task(task);
} else {
freeze_task(task);
-   freezer->state = CGROUP_FREEZING;
+   freezer->state &= ~CGROUP_FROZEN;
}
}
 
@@ -159,11 +156,7 @@ static void freezer_fork(struct task_struct *task)
goto out;
 
spin_lock_irq(>lock);
-   /*
-* @task might have been just migrated into a FROZEN cgroup.  Test
-* equality with THAWED.  Read the comment in freezer_attach().
-*/
-   if (freezer->state != CGROUP_THAWED)
+   if (freezer->state & CGROUP_FREEZING)
freeze_task(task);
spin_unlock_irq(>lock);
 out:
@@ -184,7 +177,8 @@ static void update_if_frozen(struct freezer *freezer)
struct cgroup_iter it;
struct task_struct *task;
 
-   if (freezer->state != CGROUP_FREEZING)
+   if (!(freezer->state & CGROUP_FREEZING) ||
+   (freezer->state & CGROUP_FROZEN))
return;
 
cgroup_iter_start(cgroup, );
@@ -202,7 +196,7 @@ static void update_if_frozen(struct freezer *freezer)
}
}
 
-   freezer->state = CGROUP_FROZEN;
+   freezer->state |= CGROUP_FROZEN;
 notyet:
cgroup_iter_end(cgroup, );
 }
@@ -211,14 +205,14 @@ static int freezer_read(struct cgroup *cgroup, struct 
cftype *cft,
struct seq_file *m)
 {
struct freezer *freezer = cgroup_freezer(cgroup);
-   enum freezer_state state;
+   unsigned int state;
 
  

[PATCH 6/9] cgroup_freezer: make freezer-state mask of flags

2012-11-03 Thread Tejun Heo
freezer-state was an enum value - one of THAWED, FREEZING and FROZEN.
As the scheduled full hierarchy support requires more than one
freezing condition, switch it to mask of flags.  If FREEZING is not
set, it's thawed.  FREEZING is set if freezing or frozen.  If frozen,
both FREEZING and FROZEN are set.  Now that tasks can be attached to
an already frozen cgroup, this also makes freezing condition checks
more natural.

This patch doesn't introduce any behavior change.

Signed-off-by: Tejun Heo t...@kernel.org
---
 kernel/cgroup_freezer.c | 60 ++---
 1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 2690830..e76aa9f 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -22,15 +22,14 @@
 #include linux/freezer.h
 #include linux/seq_file.h
 
-enum freezer_state {
-   CGROUP_THAWED = 0,
-   CGROUP_FREEZING,
-   CGROUP_FROZEN,
+enum freezer_state_flags {
+   CGROUP_FREEZING = (1  1), /* this freezer is freezing */
+   CGROUP_FROZEN   = (1  3), /* this and its descendants frozen 
*/
 };
 
 struct freezer {
struct cgroup_subsys_state  css;
-   enum freezer_state  state;
+   unsigned intstate;
spinlock_t  lock;
 };
 
@@ -48,12 +47,10 @@ static inline struct freezer *task_freezer(struct 
task_struct *task)
 
 bool cgroup_freezing(struct task_struct *task)
 {
-   enum freezer_state state;
bool ret;
 
rcu_read_lock();
-   state = task_freezer(task)-state;
-   ret = state == CGROUP_FREEZING || state == CGROUP_FROZEN;
+   ret = task_freezer(task)-state  CGROUP_FREEZING;
rcu_read_unlock();
 
return ret;
@@ -63,10 +60,13 @@ bool cgroup_freezing(struct task_struct *task)
  * cgroups_write_string() limits the size of freezer state strings to
  * CGROUP_LOCAL_BUFFER_SIZE
  */
-static const char *freezer_state_strs[] = {
-   THAWED,
-   FREEZING,
-   FROZEN,
+static const char *freezer_state_strs(unsigned int state)
+{
+   if (state  CGROUP_FROZEN)
+   return FROZEN;
+   if (state  CGROUP_FREEZING)
+   return FREEZING;
+   return THAWED;
 };
 
 /*
@@ -91,7 +91,6 @@ static struct cgroup_subsys_state *freezer_create(struct 
cgroup *cgroup)
return ERR_PTR(-ENOMEM);
 
spin_lock_init(freezer-lock);
-   freezer-state = CGROUP_THAWED;
return freezer-css;
 }
 
@@ -99,7 +98,7 @@ static void freezer_destroy(struct cgroup *cgroup)
 {
struct freezer *freezer = cgroup_freezer(cgroup);
 
-   if (freezer-state != CGROUP_THAWED)
+   if (freezer-state  CGROUP_FREEZING)
atomic_dec(system_freezing_cnt);
kfree(freezer);
 }
@@ -129,15 +128,13 @@ static void freezer_attach(struct cgroup *new_cgrp, 
struct cgroup_taskset *tset)
 * Tasks in @tset are on @new_cgrp but may not conform to its
 * current state before executing the following - !frozen tasks may
 * be visible in a FROZEN cgroup and frozen tasks in a THAWED one.
-* This means that, to determine whether to freeze, one should test
-* whether the state equals THAWED.
 */
cgroup_taskset_for_each(task, new_cgrp, tset) {
-   if (freezer-state == CGROUP_THAWED) {
+   if (!(freezer-state  CGROUP_FREEZING)) {
__thaw_task(task);
} else {
freeze_task(task);
-   freezer-state = CGROUP_FREEZING;
+   freezer-state = ~CGROUP_FROZEN;
}
}
 
@@ -159,11 +156,7 @@ static void freezer_fork(struct task_struct *task)
goto out;
 
spin_lock_irq(freezer-lock);
-   /*
-* @task might have been just migrated into a FROZEN cgroup.  Test
-* equality with THAWED.  Read the comment in freezer_attach().
-*/
-   if (freezer-state != CGROUP_THAWED)
+   if (freezer-state  CGROUP_FREEZING)
freeze_task(task);
spin_unlock_irq(freezer-lock);
 out:
@@ -184,7 +177,8 @@ static void update_if_frozen(struct freezer *freezer)
struct cgroup_iter it;
struct task_struct *task;
 
-   if (freezer-state != CGROUP_FREEZING)
+   if (!(freezer-state  CGROUP_FREEZING) ||
+   (freezer-state  CGROUP_FROZEN))
return;
 
cgroup_iter_start(cgroup, it);
@@ -202,7 +196,7 @@ static void update_if_frozen(struct freezer *freezer)
}
}
 
-   freezer-state = CGROUP_FROZEN;
+   freezer-state |= CGROUP_FROZEN;
 notyet:
cgroup_iter_end(cgroup, it);
 }
@@ -211,14 +205,14 @@ static int freezer_read(struct cgroup *cgroup, struct 
cftype *cft,
struct seq_file *m)
 {
struct freezer *freezer = cgroup_freezer(cgroup);
-   enum freezer_state