[PATCH 1/2] PM / Freezer: Remove the "userspace only" constraint from freezer[_do_not]_count()

[PATCH 1/2] PM / Freezer: Remove the "userspace only" constraint from freezer[_do_not]_count()

[Srivatsa S. Bhat]

At present, the functions freezer_count() and freezer_do_not_count()
impose the restriction that they are effective only for userspace processes.
However, now, these functions have found more utility than originally
intended by the commit which introduced it: ba96a0c8 (freezer:
fix vfork problem). And moreover, even the vfork issue does not need
the above restriction in these functions, since they are anyway called by
userspace processes.

So, modify these functions to make them work even for kernel threads, so
that they can be used at other places in the kernel, where the userspace
restriction doesn't apply.

Suggested-by: Oleg Nesterov <oleg@redhat.com>
Suggested-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 include/linux/freezer.h |   28 +++++++++++-----------------
 1 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 30f06c2..7bcfe73 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -67,33 +67,27 @@ static inline bool cgroup_freezing(struct task_struct *task)
  * appropriately in case the child has exited before the freezing of tasks is
  * complete.  However, we don't want kernel threads to be frozen in unexpected
  * places, so we allow them to block freeze_processes() instead or to set
- * PF_NOFREEZE if needed and PF_FREEZER_SKIP is only set for userland vfork
- * parents.  Fortunately, in the ____call_usermodehelper() case the parent won't
- * really block freeze_processes(), since ____call_usermodehelper() (the child)
- * does a little before exec/exit and it can't be frozen before waking up the
- * parent.
+ * PF_NOFREEZE if needed. Fortunately, in the ____call_usermodehelper() case the
+ * parent won't really block freeze_processes(), since ____call_usermodehelper()
+ * (the child) does a little before exec/exit and it can't be frozen before
+ * waking up the parent.
  */
 
-/*
- * If the current task is a user space one, tell the freezer not to count it as
- * freezable.
- */
+
+/* Tell the freezer not to count the current task as freezable. */
 static inline void freezer_do_not_count(void)
 {
-	if (current->mm)
-		current->flags |= PF_FREEZER_SKIP;
+	current->flags |= PF_FREEZER_SKIP;
 }
 
 /*
- * If the current task is a user space one, tell the freezer to count it as
- * freezable again and try to freeze it.
+ * Tell the freezer to count the current task as freezable again and try to
+ * freeze it.
  */
 static inline void freezer_count(void)
 {
-	if (current->mm) {
-		current->flags &= ~PF_FREEZER_SKIP;
-		try_to_freeze();
-	}
+	current->flags &= ~PF_FREEZER_SKIP;
+	try_to_freeze();
 }
 
 /*

[PATCH 2/2] PM: Use the freezer_count() functions in [un]lock_system_sleep() APIs

[Srivatsa S. Bhat]

Now that freezer_count() and freezer_do_not_count() don't have the restriction
that they are effective only when called by userspace processes, use
them in lock_system_sleep() and unlock_system_sleep() instead of open-coding
their parts.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 include/linux/suspend.h |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 1f7fff4..906d62c 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -6,6 +6,7 @@
 #include <linux/init.h>
 #include <linux/pm.h>
 #include <linux/mm.h>
+#include <linux/freezer.h>
 #include <asm/errno.h>
 
 #ifdef CONFIG_VT
@@ -380,16 +381,14 @@ static inline void unlock_system_sleep(void) {}
 
 static inline void lock_system_sleep(void)
 {
-	/* simplified freezer_do_not_count() */
-	current->flags |= PF_FREEZER_SKIP;
+	freezer_do_not_count();
 	mutex_lock(&pm_mutex);
 }
 
 static inline void unlock_system_sleep(void)
 {
 	mutex_unlock(&pm_mutex);
-	/* simplified freezer_count() */
-	current->flags &= ~PF_FREEZER_SKIP;
+	freezer_count();
 }
 #endif
 

Re: [PATCH 2/2] PM: Use the freezer_count() functions in [un]lock_system_sleep() APIs

[Tejun Heo]

On Tue, Dec 06, 2011 at 09:34:02PM +0530, Srivatsa S. Bhat wrote:
> Now that freezer_count() and freezer_do_not_count() don't have the restriction
> that they are effective only when called by userspace processes, use
> them in lock_system_sleep() and unlock_system_sleep() instead of open-coding
> their parts.
> 
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>

Acked-by: Tejun Heo <tj@kernel.org>

-- 
tejun

Re: [PATCH 1/2] PM / Freezer: Remove the "userspace only" constraint from freezer[_do_not]_count()

[Tejun Heo]

On Tue, Dec 06, 2011 at 09:33:44PM +0530, Srivatsa S. Bhat wrote:
> At present, the functions freezer_count() and freezer_do_not_count()
> impose the restriction that they are effective only for userspace processes.
> However, now, these functions have found more utility than originally
> intended by the commit which introduced it: ba96a0c8 (freezer:
> fix vfork problem). And moreover, even the vfork issue does not need
> the above restriction in these functions, since they are anyway called by
> userspace processes.
> 
> So, modify these functions to make them work even for kernel threads, so
> that they can be used at other places in the kernel, where the userspace
> restriction doesn't apply.
> 
> Suggested-by: Oleg Nesterov <oleg@redhat.com>
> Suggested-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

Re: [PATCH 1/2] PM / Freezer: Remove the "userspace only" constraint from freezer[_do_not]_count()

[Oleg Nesterov]

On 12/06, Srivatsa S. Bhat wrote:
>
> And moreover, even the vfork issue does not need
> the above restriction in these functions, since they are anyway called by
> userspace processes.

This looks a bit confusing, as if CLONE_VFORK is not used by kthreads.
It is actually used, but only by the non-freezeable workqueue threads,
so the patch is fine.

Reviewed-by: Oleg Nesterov <oleg@redhat.com>

Re: [PATCH 1/2] PM / Freezer: Remove the "userspace only" constraint from freezer[_do_not]_count()

[Srivatsa S. Bhat]

On 12/06/2011 10:50 PM, Oleg Nesterov wrote:

> On 12/06, Srivatsa S. Bhat wrote:
>>
>> And moreover, even the vfork issue does not need
>> the above restriction in these functions, since they are anyway called by
>> userspace processes.
> 
> This looks a bit confusing, as if CLONE_VFORK is not used by kthreads.
> It is actually used, but only by the non-freezeable workqueue threads,
> so the patch is fine.
> 
> Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> 


Thank you! I have updated the commit message by omitting the confusing part:

---
From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
[PATCH 1/2] PM / Freezer: Remove the "userspace only" constraint from freezer[_do_not]_count()

At present, the functions freezer_count() and freezer_do_not_count()
impose the restriction that they are effective only for userspace processes.
However, now, these functions have found more utility than originally
intended by the commit which introduced it: ba96a0c8 (freezer:
fix vfork problem). And moreover, even the vfork issue actually does not
need the above restriction in these functions.

So, modify these functions to make them work even for kernel threads, so
that they can be used at other places in the kernel, where the userspace
restriction doesn't apply.

Suggested-by: Oleg Nesterov <oleg@redhat.com>
Suggested-by: Tejun Heo <tj@kernel.org>
Acked-by: Tejun Heo <tj@kernel.org>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 include/linux/freezer.h |   28 +++++++++++-----------------
 1 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 30f06c2..7bcfe73 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -67,33 +67,27 @@ static inline bool cgroup_freezing(struct task_struct *task)
  * appropriately in case the child has exited before the freezing of tasks is
  * complete.  However, we don't want kernel threads to be frozen in unexpected
  * places, so we allow them to block freeze_processes() instead or to set
- * PF_NOFREEZE if needed and PF_FREEZER_SKIP is only set for userland vfork
- * parents.  Fortunately, in the ____call_usermodehelper() case the parent won't
- * really block freeze_processes(), since ____call_usermodehelper() (the child)
- * does a little before exec/exit and it can't be frozen before waking up the
- * parent.
+ * PF_NOFREEZE if needed. Fortunately, in the ____call_usermodehelper() case the
+ * parent won't really block freeze_processes(), since ____call_usermodehelper()
+ * (the child) does a little before exec/exit and it can't be frozen before
+ * waking up the parent.
  */
 
-/*
- * If the current task is a user space one, tell the freezer not to count it as
- * freezable.
- */
+
+/* Tell the freezer not to count the current task as freezable. */
 static inline void freezer_do_not_count(void)
 {
-	if (current->mm)
-		current->flags |= PF_FREEZER_SKIP;
+	current->flags |= PF_FREEZER_SKIP;
 }
 
 /*
- * If the current task is a user space one, tell the freezer to count it as
- * freezable again and try to freeze it.
+ * Tell the freezer to count the current task as freezable again and try to
+ * freeze it.
  */
 static inline void freezer_count(void)
 {
-	if (current->mm) {
-		current->flags &= ~PF_FREEZER_SKIP;
-		try_to_freeze();
-	}
+	current->flags &= ~PF_FREEZER_SKIP;
+	try_to_freeze();
 }
 
 /*

Re: [PATCH 1/2] PM / Freezer: Remove the "userspace only" constraint from freezer[_do_not]_count()

[Rafael J. Wysocki]

On Tuesday, December 06, 2011, Srivatsa S. Bhat wrote:
> On 12/06/2011 10:50 PM, Oleg Nesterov wrote:
> 
> > On 12/06, Srivatsa S. Bhat wrote:
> >>
> >> And moreover, even the vfork issue does not need
> >> the above restriction in these functions, since they are anyway called by
> >> userspace processes.
> > 
> > This looks a bit confusing, as if CLONE_VFORK is not used by kthreads.
> > It is actually used, but only by the non-freezeable workqueue threads,
> > so the patch is fine.
> > 
> > Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> > 
> 
> 
> Thank you! I have updated the commit message by omitting the confusing part:
> 
> ---
> From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> [PATCH 1/2] PM / Freezer: Remove the "userspace only" constraint from freezer[_do_not]_count()
> 
> At present, the functions freezer_count() and freezer_do_not_count()
> impose the restriction that they are effective only for userspace processes.
> However, now, these functions have found more utility than originally
> intended by the commit which introduced it: ba96a0c8 (freezer:
> fix vfork problem). And moreover, even the vfork issue actually does not
> need the above restriction in these functions.
> 
> So, modify these functions to make them work even for kernel threads, so
> that they can be used at other places in the kernel, where the userspace
> restriction doesn't apply.
> 
> Suggested-by: Oleg Nesterov <oleg@redhat.com>
> Suggested-by: Tejun Heo <tj@kernel.org>
> Acked-by: Tejun Heo <tj@kernel.org>
> Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>

Applied to linux-pm/linux-next ([2/2] too).

Thanks,
Rafael


> ---
> 
>  include/linux/freezer.h |   28 +++++++++++-----------------
>  1 files changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> index 30f06c2..7bcfe73 100644
> --- a/include/linux/freezer.h
> +++ b/include/linux/freezer.h
> @@ -67,33 +67,27 @@ static inline bool cgroup_freezing(struct task_struct *task)
>   * appropriately in case the child has exited before the freezing of tasks is
>   * complete.  However, we don't want kernel threads to be frozen in unexpected
>   * places, so we allow them to block freeze_processes() instead or to set
> - * PF_NOFREEZE if needed and PF_FREEZER_SKIP is only set for userland vfork
> - * parents.  Fortunately, in the ____call_usermodehelper() case the parent won't
> - * really block freeze_processes(), since ____call_usermodehelper() (the child)
> - * does a little before exec/exit and it can't be frozen before waking up the
> - * parent.
> + * PF_NOFREEZE if needed. Fortunately, in the ____call_usermodehelper() case the
> + * parent won't really block freeze_processes(), since ____call_usermodehelper()
> + * (the child) does a little before exec/exit and it can't be frozen before
> + * waking up the parent.
>   */
>  
> -/*
> - * If the current task is a user space one, tell the freezer not to count it as
> - * freezable.
> - */
> +
> +/* Tell the freezer not to count the current task as freezable. */
>  static inline void freezer_do_not_count(void)
>  {
> -	if (current->mm)
> -		current->flags |= PF_FREEZER_SKIP;
> +	current->flags |= PF_FREEZER_SKIP;
>  }
>  
>  /*
> - * If the current task is a user space one, tell the freezer to count it as
> - * freezable again and try to freeze it.
> + * Tell the freezer to count the current task as freezable again and try to
> + * freeze it.
>   */
>  static inline void freezer_count(void)
>  {
> -	if (current->mm) {
> -		current->flags &= ~PF_FREEZER_SKIP;
> -		try_to_freeze();
> -	}
> +	current->flags &= ~PF_FREEZER_SKIP;
> +	try_to_freeze();
>  }
>  
>  /*
> 
> 
> 
>