* [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
@ 2012-02-01 22:31 Srivatsa S. Bhat
2012-02-01 22:36 ` Tejun Heo
0 siblings, 1 reply; 13+ messages in thread
From: Srivatsa S. Bhat @ 2012-02-01 22:31 UTC (permalink / raw)
To: rjw; +Cc: pavel, len.brown, tj, linux-pm, linux-kernel, srivatsa.bhat
If freezing of kernel threads fails, we are expected to automatically thaw
tasks in the error recovery path. However, at times, we encounter situations
in which we would like the automatic error recovery path to thaw only the
kernel threads, because we want to be able to do some more cleanup before
we thaw userspace. Something like:
error = freeze_kernel_threads();
if (error) {
/* Do some cleanup */
/* Only then thaw userspace tasks*/
thaw_processes();
}
An example of such a situation is where we freeze/thaw filesystems during
suspend/hibernation. There, if freezing of kernel threads fails, we would
like to thaw the frozen filesystems before thawing the userspace tasks.
So, modify freeze_kernel_threads() to thaw only kernel threads in case of
freezing failure.
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
kernel/power/power.h | 26 ++++++++++++++++++++++++--
kernel/power/process.c | 7 +++++--
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/kernel/power/power.h b/kernel/power/power.h
index 0c4defe..cf41c03 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -231,8 +231,30 @@ extern int pm_test_level;
#ifdef CONFIG_SUSPEND_FREEZER
static inline int suspend_freeze_processes(void)
{
- int error = freeze_processes();
- return error ? : freeze_kernel_threads();
+ int error;
+
+ error = freeze_processes();
+
+ /*
+ * freeze_processes() automatically thaws
+ * every task if freezing fails. So we need
+ * not do anything extra upon error.
+ */
+ if (error)
+ goto Finish;
+
+ error = freeze_kernel_threads();
+
+ /*
+ * freeze_kernel_threads() thaws only kernel
+ * threads upon freezing failure. So we have
+ * to thaw the userspace tasks ourselves.
+ */
+ if (error)
+ thaw_processes();
+
+ Finish:
+ return error;
}
static inline void suspend_thaw_processes(void)
diff --git a/kernel/power/process.c b/kernel/power/process.c
index eeca003..7e42645 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -143,7 +143,10 @@ int freeze_processes(void)
/**
* freeze_kernel_threads - Make freezable kernel threads go to the refrigerator.
*
- * On success, returns 0. On failure, -errno and system is fully thawed.
+ * On success, returns 0. On failure, -errno and only the kernel threads are
+ * thawed, so as to give a chance to the caller to do additional cleanups
+ * (if any) before thawing the userspace tasks. So, it is the responsibility
+ * of the caller to thaw the userspace tasks, when the time is right.
*/
int freeze_kernel_threads(void)
{
@@ -159,7 +162,7 @@ int freeze_kernel_threads(void)
BUG_ON(in_atomic());
if (error)
- thaw_processes();
+ thaw_kernel_threads();
return error;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-01 22:31 [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails Srivatsa S. Bhat
@ 2012-02-01 22:36 ` Tejun Heo
2012-02-01 22:49 ` Rafael J. Wysocki
2012-02-01 22:59 ` Srivatsa S. Bhat
0 siblings, 2 replies; 13+ messages in thread
From: Tejun Heo @ 2012-02-01 22:36 UTC (permalink / raw)
To: Srivatsa S. Bhat; +Cc: rjw, pavel, len.brown, linux-pm, linux-kernel
On Thu, Feb 02, 2012 at 04:01:12AM +0530, Srivatsa S. Bhat wrote:
> If freezing of kernel threads fails, we are expected to automatically thaw
> tasks in the error recovery path. However, at times, we encounter situations
> in which we would like the automatic error recovery path to thaw only the
> kernel threads, because we want to be able to do some more cleanup before
> we thaw userspace. Something like:
>
> error = freeze_kernel_threads();
> if (error) {
> /* Do some cleanup */
>
> /* Only then thaw userspace tasks*/
> thaw_processes();
> }
>
> An example of such a situation is where we freeze/thaw filesystems during
> suspend/hibernation. There, if freezing of kernel threads fails, we would
> like to thaw the frozen filesystems before thawing the userspace tasks.
>
> So, modify freeze_kernel_threads() to thaw only kernel threads in case of
> freezing failure.
>
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Acked-by: Tejun Heo <tj@kernel.org>
with nits below,
> ---
>
> kernel/power/power.h | 26 ++++++++++++++++++++++++--
> kernel/power/process.c | 7 +++++--
> 2 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 0c4defe..cf41c03 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -231,8 +231,30 @@ extern int pm_test_level;
> #ifdef CONFIG_SUSPEND_FREEZER
> static inline int suspend_freeze_processes(void)
> {
> - int error = freeze_processes();
> - return error ? : freeze_kernel_threads();
> + int error;
> +
> + error = freeze_processes();
> +
> + /*
> + * freeze_processes() automatically thaws
> + * every task if freezing fails. So we need
> + * not do anything extra upon error.
> + */
> + if (error)
> + goto Finish;
> +
> + error = freeze_kernel_threads();
> +
> + /*
> + * freeze_kernel_threads() thaws only kernel
> + * threads upon freezing failure. So we have
> + * to thaw the userspace tasks ourselves.
> + */
> + if (error)
> + thaw_processes();
> +
> + Finish:
> + return error;
Heh, the original ?: code was scary. That said, IMHO, the above seems
a bit too verbose. Functions usually shouldn't have side effects on
failure and the above code is usual in that sense and it would be nice
to mention the reformatting (from cryptic to sane) in the commit log.
Also, how many columns are you flowing the text to? It seems quite
narrower than they usually should be.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-01 22:36 ` Tejun Heo
@ 2012-02-01 22:49 ` Rafael J. Wysocki
2012-02-01 22:59 ` Srivatsa S. Bhat
1 sibling, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2012-02-01 22:49 UTC (permalink / raw)
To: Tejun Heo; +Cc: Srivatsa S. Bhat, pavel, len.brown, linux-pm, linux-kernel
On Wednesday, February 01, 2012, Tejun Heo wrote:
> On Thu, Feb 02, 2012 at 04:01:12AM +0530, Srivatsa S. Bhat wrote:
> > If freezing of kernel threads fails, we are expected to automatically thaw
> > tasks in the error recovery path. However, at times, we encounter situations
> > in which we would like the automatic error recovery path to thaw only the
> > kernel threads, because we want to be able to do some more cleanup before
> > we thaw userspace. Something like:
> >
> > error = freeze_kernel_threads();
> > if (error) {
> > /* Do some cleanup */
> >
> > /* Only then thaw userspace tasks*/
> > thaw_processes();
> > }
> >
> > An example of such a situation is where we freeze/thaw filesystems during
> > suspend/hibernation. There, if freezing of kernel threads fails, we would
> > like to thaw the frozen filesystems before thawing the userspace tasks.
> >
> > So, modify freeze_kernel_threads() to thaw only kernel threads in case of
> > freezing failure.
> >
> > Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
>
> Acked-by: Tejun Heo <tj@kernel.org>
>
> with nits below,
>
> > ---
> >
> > kernel/power/power.h | 26 ++++++++++++++++++++++++--
> > kernel/power/process.c | 7 +++++--
> > 2 files changed, 29 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/power/power.h b/kernel/power/power.h
> > index 0c4defe..cf41c03 100644
> > --- a/kernel/power/power.h
> > +++ b/kernel/power/power.h
> > @@ -231,8 +231,30 @@ extern int pm_test_level;
> > #ifdef CONFIG_SUSPEND_FREEZER
> > static inline int suspend_freeze_processes(void)
> > {
> > - int error = freeze_processes();
> > - return error ? : freeze_kernel_threads();
> > + int error;
> > +
> > + error = freeze_processes();
> > +
> > + /*
> > + * freeze_processes() automatically thaws
> > + * every task if freezing fails. So we need
> > + * not do anything extra upon error.
> > + */
> > + if (error)
> > + goto Finish;
> > +
> > + error = freeze_kernel_threads();
> > +
> > + /*
> > + * freeze_kernel_threads() thaws only kernel
> > + * threads upon freezing failure. So we have
> > + * to thaw the userspace tasks ourselves.
> > + */
> > + if (error)
> > + thaw_processes();
> > +
> > + Finish:
> > + return error;
>
> Heh, the original ?: code was scary.
C'mon. ;-)
> That said, IMHO, the above seems
> a bit too verbose. Functions usually shouldn't have side effects on
> failure and the above code is usual in that sense and it would be nice
> to mention the reformatting (from cryptic to sane) in the commit log.
> Also, how many columns are you flowing the text to? It seems quite
> narrower than they usually should be.
I agree.
Srivatsa, care to resubmit?
Rafael
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-01 22:36 ` Tejun Heo
2012-02-01 22:49 ` Rafael J. Wysocki
@ 2012-02-01 22:59 ` Srivatsa S. Bhat
2012-02-02 3:15 ` Nigel Cunningham
` (2 more replies)
1 sibling, 3 replies; 13+ messages in thread
From: Srivatsa S. Bhat @ 2012-02-01 22:59 UTC (permalink / raw)
To: Tejun Heo; +Cc: rjw, pavel, len.brown, linux-pm, linux-kernel, Nigel Cunningham
On 02/02/2012 04:06 AM, Tejun Heo wrote:
> On Thu, Feb 02, 2012 at 04:01:12AM +0530, Srivatsa S. Bhat wrote:
>> So, modify freeze_kernel_threads() to thaw only kernel threads in case of
>> freezing failure.
>>
>> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
>
> Acked-by: Tejun Heo <tj@kernel.org>
>
Thanks a lot!
> with nits below,
>
>
> Heh, the original ?: code was scary. That said, IMHO, the above seems
> a bit too verbose. Functions usually shouldn't have side effects on
> failure and the above code is usual in that sense and it would be nice
> to mention the reformatting (from cryptic to sane) in the commit log.
Ok :-)
> Also, how many columns are you flowing the text to? It seems quite
> narrower than they usually should be.
Oh right, I didn't give attention to that. I'll try to fit it in lesser
number of lines. But, should I really cut short on the explanation itself?
I remember Nigel had requested some comments on a different patch
on roughly similar code path https://lkml.org/lkml/2012/1/28/111.
So I thought it would be a good idea to explicitly document the differences,
to be clear...
Here is the updated patch:
From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Subject: [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
If freezing of kernel threads fails, we are expected to automatically thaw
tasks in the error recovery path. However, at times, we encounter situations
in which we would like the automatic error recovery path to thaw only the
kernel threads, because we want to be able to do some more cleanup before
we thaw userspace. Something like:
error = freeze_kernel_threads();
if (error) {
/* Do some cleanup */
/* Only then thaw userspace tasks*/
thaw_processes();
}
An example of such a situation is where we freeze/thaw filesystems during
suspend/hibernation. There, if freezing of kernel threads fails, we would
like to thaw the frozen filesystems before thawing the userspace tasks.
So, modify freeze_kernel_threads() to thaw only kernel threads in case of
freezing failure. And change suspend_freeze_processes() accordingly.
(At the same time, let us also get rid of the rather cryptic usage of the
conditional operator (:?) in that function.)
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
kernel/power/power.h | 24 ++++++++++++++++++++++--
kernel/power/process.c | 7 +++++--
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/kernel/power/power.h b/kernel/power/power.h
index 0c4defe..21724ee 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -231,8 +231,28 @@ extern int pm_test_level;
#ifdef CONFIG_SUSPEND_FREEZER
static inline int suspend_freeze_processes(void)
{
- int error = freeze_processes();
- return error ? : freeze_kernel_threads();
+ int error;
+
+ error = freeze_processes();
+
+ /*
+ * freeze_processes() automatically thaws every task if freezing
+ * fails. So we need not do anything extra upon error.
+ */
+ if (error)
+ goto Finish;
+
+ error = freeze_kernel_threads();
+
+ /*
+ * freeze_kernel_threads() thaws only kernel threads upon freezing
+ * failure. So we have to thaw the userspace tasks ourselves.
+ */
+ if (error)
+ thaw_processes();
+
+ Finish:
+ return error;
}
static inline void suspend_thaw_processes(void)
diff --git a/kernel/power/process.c b/kernel/power/process.c
index eeca003..7e42645 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -143,7 +143,10 @@ int freeze_processes(void)
/**
* freeze_kernel_threads - Make freezable kernel threads go to the refrigerator.
*
- * On success, returns 0. On failure, -errno and system is fully thawed.
+ * On success, returns 0. On failure, -errno and only the kernel threads are
+ * thawed, so as to give a chance to the caller to do additional cleanups
+ * (if any) before thawing the userspace tasks. So, it is the responsibility
+ * of the caller to thaw the userspace tasks, when the time is right.
*/
int freeze_kernel_threads(void)
{
@@ -159,7 +162,7 @@ int freeze_kernel_threads(void)
BUG_ON(in_atomic());
if (error)
- thaw_processes();
+ thaw_kernel_threads();
return error;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-01 22:59 ` Srivatsa S. Bhat
@ 2012-02-02 3:15 ` Nigel Cunningham
2012-02-02 19:38 ` Rafael J. Wysocki
2012-02-04 14:11 ` [PATCH Updated] " Srivatsa S. Bhat
2 siblings, 0 replies; 13+ messages in thread
From: Nigel Cunningham @ 2012-02-02 3:15 UTC (permalink / raw)
To: Srivatsa S. Bhat; +Cc: Tejun Heo, rjw, pavel, len.brown, linux-pm, linux-kernel
Hi
On 02/02/12 09:59, Srivatsa S. Bhat wrote:
> On 02/02/2012 04:06 AM, Tejun Heo wrote:
>
>> On Thu, Feb 02, 2012 at 04:01:12AM +0530, Srivatsa S. Bhat wrote:
>>> So, modify freeze_kernel_threads() to thaw only kernel threads in case of
>>> freezing failure.
>>>
>>> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
>>
>> Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: Nigel Cunningham <nigel@tuxonice.net>
Regards,
Nigel
--
Evolution (n): A hypothetical process whereby improbable
events occur with alarming frequency, order arises from chaos, and
no one is given credit.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-01 22:59 ` Srivatsa S. Bhat
2012-02-02 3:15 ` Nigel Cunningham
@ 2012-02-02 19:38 ` Rafael J. Wysocki
2012-02-04 14:11 ` [PATCH Updated] " Srivatsa S. Bhat
2 siblings, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2012-02-02 19:38 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: Tejun Heo, pavel, len.brown, linux-pm, linux-kernel, Nigel Cunningham
On Wednesday, February 01, 2012, Srivatsa S. Bhat wrote:
> On 02/02/2012 04:06 AM, Tejun Heo wrote:
>
> > On Thu, Feb 02, 2012 at 04:01:12AM +0530, Srivatsa S. Bhat wrote:
> >> So, modify freeze_kernel_threads() to thaw only kernel threads in case of
> >> freezing failure.
> >>
> >> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> >
> > Acked-by: Tejun Heo <tj@kernel.org>
> >
>
>
> Thanks a lot!
>
> > with nits below,
> >
> >
> > Heh, the original ?: code was scary. That said, IMHO, the above seems
> > a bit too verbose. Functions usually shouldn't have side effects on
> > failure and the above code is usual in that sense and it would be nice
> > to mention the reformatting (from cryptic to sane) in the commit log.
>
>
> Ok :-)
>
> > Also, how many columns are you flowing the text to? It seems quite
> > narrower than they usually should be.
>
>
> Oh right, I didn't give attention to that. I'll try to fit it in lesser
> number of lines. But, should I really cut short on the explanation itself?
> I remember Nigel had requested some comments on a different patch
> on roughly similar code path https://lkml.org/lkml/2012/1/28/111.
> So I thought it would be a good idea to explicitly document the differences,
> to be clear...
>
> Here is the updated patch:
>
> From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Subject: [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
>
> If freezing of kernel threads fails, we are expected to automatically thaw
> tasks in the error recovery path. However, at times, we encounter situations
> in which we would like the automatic error recovery path to thaw only the
> kernel threads, because we want to be able to do some more cleanup before
> we thaw userspace. Something like:
>
> error = freeze_kernel_threads();
> if (error) {
> /* Do some cleanup */
>
> /* Only then thaw userspace tasks*/
> thaw_processes();
> }
>
> An example of such a situation is where we freeze/thaw filesystems during
> suspend/hibernation. There, if freezing of kernel threads fails, we would
> like to thaw the frozen filesystems before thawing the userspace tasks.
>
> So, modify freeze_kernel_threads() to thaw only kernel threads in case of
> freezing failure. And change suspend_freeze_processes() accordingly.
> (At the same time, let us also get rid of the rather cryptic usage of the
> conditional operator (:?) in that function.)
>
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Acked-by: Tejun Heo <tj@kernel.org>
Applied.
Thanks,
Rafael
> ---
>
> kernel/power/power.h | 24 ++++++++++++++++++++++--
> kernel/power/process.c | 7 +++++--
> 2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 0c4defe..21724ee 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -231,8 +231,28 @@ extern int pm_test_level;
> #ifdef CONFIG_SUSPEND_FREEZER
> static inline int suspend_freeze_processes(void)
> {
> - int error = freeze_processes();
> - return error ? : freeze_kernel_threads();
> + int error;
> +
> + error = freeze_processes();
> +
> + /*
> + * freeze_processes() automatically thaws every task if freezing
> + * fails. So we need not do anything extra upon error.
> + */
> + if (error)
> + goto Finish;
> +
> + error = freeze_kernel_threads();
> +
> + /*
> + * freeze_kernel_threads() thaws only kernel threads upon freezing
> + * failure. So we have to thaw the userspace tasks ourselves.
> + */
> + if (error)
> + thaw_processes();
> +
> + Finish:
> + return error;
> }
>
> static inline void suspend_thaw_processes(void)
> diff --git a/kernel/power/process.c b/kernel/power/process.c
> index eeca003..7e42645 100644
> --- a/kernel/power/process.c
> +++ b/kernel/power/process.c
> @@ -143,7 +143,10 @@ int freeze_processes(void)
> /**
> * freeze_kernel_threads - Make freezable kernel threads go to the refrigerator.
> *
> - * On success, returns 0. On failure, -errno and system is fully thawed.
> + * On success, returns 0. On failure, -errno and only the kernel threads are
> + * thawed, so as to give a chance to the caller to do additional cleanups
> + * (if any) before thawing the userspace tasks. So, it is the responsibility
> + * of the caller to thaw the userspace tasks, when the time is right.
> */
> int freeze_kernel_threads(void)
> {
> @@ -159,7 +162,7 @@ int freeze_kernel_threads(void)
> BUG_ON(in_atomic());
>
> if (error)
> - thaw_processes();
> + thaw_kernel_threads();
> return error;
> }
>
>
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH Updated] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-01 22:59 ` Srivatsa S. Bhat
2012-02-02 3:15 ` Nigel Cunningham
2012-02-02 19:38 ` Rafael J. Wysocki
@ 2012-02-04 14:11 ` Srivatsa S. Bhat
2012-02-04 22:27 ` Rafael J. Wysocki
2 siblings, 1 reply; 13+ messages in thread
From: Srivatsa S. Bhat @ 2012-02-04 14:11 UTC (permalink / raw)
To: Tejun Heo; +Cc: rjw, pavel, len.brown, linux-pm, linux-kernel, Nigel Cunningham
On 02/02/2012 04:29 AM, Srivatsa S. Bhat wrote:
>
> From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Subject: [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
>
> If freezing of kernel threads fails, we are expected to automatically thaw
> tasks in the error recovery path. However, at times, we encounter situations
> in which we would like the automatic error recovery path to thaw only the
> kernel threads, because we want to be able to do some more cleanup before
> we thaw userspace. Something like:
>
> error = freeze_kernel_threads();
> if (error) {
> /* Do some cleanup */
>
> /* Only then thaw userspace tasks*/
> thaw_processes();
> }
>
> An example of such a situation is where we freeze/thaw filesystems during
> suspend/hibernation. There, if freezing of kernel threads fails, we would
> like to thaw the frozen filesystems before thawing the userspace tasks.
>
> So, modify freeze_kernel_threads() to thaw only kernel threads in case of
> freezing failure. And change suspend_freeze_processes() accordingly.
> (At the same time, let us also get rid of the rather cryptic usage of the
> conditional operator (:?) in that function.)
>
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Acked-by: Tejun Heo <tj@kernel.org>
> ---
Hi Rafael,
I have reworked the changelog of this patch to indicate that this fixes a
regression. Here is the updated patch:
From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Subject: [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
If freezing of kernel threads fails, we are expected to automatically thaw
tasks in the error recovery path. However, at times, we encounter situations
in which we would like the automatic error recovery path to thaw only the
kernel threads, because we want to be able to do some more cleanup before
we thaw userspace. Something like:
error = freeze_kernel_threads();
if (error) {
/* Do some cleanup */
/* Only then thaw userspace tasks*/
thaw_processes();
}
An example of such a situation is where we freeze/thaw filesystems during
suspend/hibernation. There, if freezing of kernel threads fails, we would
like to thaw the frozen filesystems before thawing the userspace tasks.
So, modify freeze_kernel_threads() to thaw only kernel threads in case of
freezing failure.
This also fixes a regression in hibernation_snapshot() by which, if the
freezing of kernel threads happened to fail, we would thaw every task
(including userspace tasks) before freeing the memory pre-allocated for the
purpose of hibernation (which could be quite significant).
So, thawing only kernel threads upon failure to freeze kernel threads will
give us a chance to free up the pre-allocated memory before thawing userspace
tasks, thereby reducing the probability of unreasonable memory allocation
failures.
And while we are updating suspend_freeze_processes() to adapt to this change,
let us also get rid of the rather cryptic usage of the conditional operator
(:?) in that function.
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: Nigel Cunningham <nigel@tuxonice.net>
---
kernel/power/power.h | 24 ++++++++++++++++++++++--
kernel/power/process.c | 7 +++++--
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/kernel/power/power.h b/kernel/power/power.h
index 0c4defe..21724ee 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -231,8 +231,28 @@ extern int pm_test_level;
#ifdef CONFIG_SUSPEND_FREEZER
static inline int suspend_freeze_processes(void)
{
- int error = freeze_processes();
- return error ? : freeze_kernel_threads();
+ int error;
+
+ error = freeze_processes();
+
+ /*
+ * freeze_processes() automatically thaws every task if freezing
+ * fails. So we need not do anything extra upon error.
+ */
+ if (error)
+ goto Finish;
+
+ error = freeze_kernel_threads();
+
+ /*
+ * freeze_kernel_threads() thaws only kernel threads upon freezing
+ * failure. So we have to thaw the userspace tasks ourselves.
+ */
+ if (error)
+ thaw_processes();
+
+ Finish:
+ return error;
}
static inline void suspend_thaw_processes(void)
diff --git a/kernel/power/process.c b/kernel/power/process.c
index eeca003..7e42645 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -143,7 +143,10 @@ int freeze_processes(void)
/**
* freeze_kernel_threads - Make freezable kernel threads go to the refrigerator.
*
- * On success, returns 0. On failure, -errno and system is fully thawed.
+ * On success, returns 0. On failure, -errno and only the kernel threads are
+ * thawed, so as to give a chance to the caller to do additional cleanups
+ * (if any) before thawing the userspace tasks. So, it is the responsibility
+ * of the caller to thaw the userspace tasks, when the time is right.
*/
int freeze_kernel_threads(void)
{
@@ -159,7 +162,7 @@ int freeze_kernel_threads(void)
BUG_ON(in_atomic());
if (error)
- thaw_processes();
+ thaw_kernel_threads();
return error;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH Updated] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-04 14:11 ` [PATCH Updated] " Srivatsa S. Bhat
@ 2012-02-04 22:27 ` Rafael J. Wysocki
2012-02-05 6:06 ` Hillf Danton
0 siblings, 1 reply; 13+ messages in thread
From: Rafael J. Wysocki @ 2012-02-04 22:27 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: Tejun Heo, pavel, len.brown, linux-pm, linux-kernel, Nigel Cunningham
On Saturday, February 04, 2012, Srivatsa S. Bhat wrote:
> On 02/02/2012 04:29 AM, Srivatsa S. Bhat wrote:
>
> >
> > From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> > Subject: [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
> >
> > If freezing of kernel threads fails, we are expected to automatically thaw
> > tasks in the error recovery path. However, at times, we encounter situations
> > in which we would like the automatic error recovery path to thaw only the
> > kernel threads, because we want to be able to do some more cleanup before
> > we thaw userspace. Something like:
> >
> > error = freeze_kernel_threads();
> > if (error) {
> > /* Do some cleanup */
> >
> > /* Only then thaw userspace tasks*/
> > thaw_processes();
> > }
> >
> > An example of such a situation is where we freeze/thaw filesystems during
> > suspend/hibernation. There, if freezing of kernel threads fails, we would
> > like to thaw the frozen filesystems before thawing the userspace tasks.
> >
> > So, modify freeze_kernel_threads() to thaw only kernel threads in case of
> > freezing failure. And change suspend_freeze_processes() accordingly.
> > (At the same time, let us also get rid of the rather cryptic usage of the
> > conditional operator (:?) in that function.)
> >
> > Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> > Acked-by: Tejun Heo <tj@kernel.org>
> > ---
>
>
> Hi Rafael,
>
> I have reworked the changelog of this patch to indicate that this fixes a
> regression. Here is the updated patch:
Well, I've already fixed that up in my tree, but thanks anyway. :-)
Rafael
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH Updated] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-04 22:27 ` Rafael J. Wysocki
@ 2012-02-05 6:06 ` Hillf Danton
2012-02-05 10:57 ` Srivatsa S. Bhat
2012-02-05 10:59 ` Rafael J. Wysocki
0 siblings, 2 replies; 13+ messages in thread
From: Hillf Danton @ 2012-02-05 6:06 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Srivatsa S. Bhat, Tejun Heo, pavel, len.brown, linux-pm,
linux-kernel, Nigel Cunningham
On Sun, Feb 5, 2012 at 6:27 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Saturday, February 04, 2012, Srivatsa S. Bhat wrote:
>> On 02/02/2012 04:29 AM, Srivatsa S. Bhat wrote:
>> Hi Rafael,
>>
>> I have reworked the changelog of this patch to indicate that this fixes a
>> regression. Here is the updated patch:
>
> Well, I've already fixed that up in my tree, but thanks anyway. :-)
>
Nicer if gurus open wider windows for patches received, no?
Good weekend
Hillf
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH Updated] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-05 6:06 ` Hillf Danton
@ 2012-02-05 10:57 ` Srivatsa S. Bhat
2012-02-05 11:09 ` Rafael J. Wysocki
2012-02-05 11:31 ` Hillf Danton
2012-02-05 10:59 ` Rafael J. Wysocki
1 sibling, 2 replies; 13+ messages in thread
From: Srivatsa S. Bhat @ 2012-02-05 10:57 UTC (permalink / raw)
To: Hillf Danton
Cc: Rafael J. Wysocki, Tejun Heo, pavel, len.brown, linux-pm,
linux-kernel, Nigel Cunningham
On 02/05/2012 11:36 AM, Hillf Danton wrote:
> On Sun, Feb 5, 2012 at 6:27 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> On Saturday, February 04, 2012, Srivatsa S. Bhat wrote:
>>> On 02/02/2012 04:29 AM, Srivatsa S. Bhat wrote:
>>> Hi Rafael,
>>>
>>> I have reworked the changelog of this patch to indicate that this fixes a
>>> regression. Here is the updated patch:
>>
>> Well, I've already fixed that up in my tree, but thanks anyway. :-)
>>
>
> Nicer if gurus open wider windows for patches received, no?
>
I am not quite sure what you meant by that, but just in case you got the
wrong idea by any chance, let me clarify:
Working with Rafael has *always* been an immensely pleasurable experience!
There is absolutely no doubt in that :-)
So there really is no reason to feel bad about the above incident :-)
Regards,
Srivatsa S. Bhat
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH Updated] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-05 6:06 ` Hillf Danton
2012-02-05 10:57 ` Srivatsa S. Bhat
@ 2012-02-05 10:59 ` Rafael J. Wysocki
1 sibling, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2012-02-05 10:59 UTC (permalink / raw)
To: Hillf Danton
Cc: Srivatsa S. Bhat, Tejun Heo, pavel, len.brown, linux-pm,
linux-kernel, Nigel Cunningham
On Sunday, February 05, 2012, Hillf Danton wrote:
> On Sun, Feb 5, 2012 at 6:27 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Saturday, February 04, 2012, Srivatsa S. Bhat wrote:
> >> On 02/02/2012 04:29 AM, Srivatsa S. Bhat wrote:
> >> Hi Rafael,
> >>
> >> I have reworked the changelog of this patch to indicate that this fixes a
> >> regression. Here is the updated patch:
> >
> > Well, I've already fixed that up in my tree, but thanks anyway. :-)
> >
>
> Nicer if gurus open wider windows for patches received, no?
Well, I hope Srivatsa didn't mind. :-)
Thanks,
Rafael
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH Updated] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-05 10:57 ` Srivatsa S. Bhat
@ 2012-02-05 11:09 ` Rafael J. Wysocki
2012-02-05 11:31 ` Hillf Danton
1 sibling, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2012-02-05 11:09 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: Hillf Danton, Tejun Heo, pavel, len.brown, linux-pm,
linux-kernel, Nigel Cunningham
On Sunday, February 05, 2012, Srivatsa S. Bhat wrote:
> On 02/05/2012 11:36 AM, Hillf Danton wrote:
>
> > On Sun, Feb 5, 2012 at 6:27 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> On Saturday, February 04, 2012, Srivatsa S. Bhat wrote:
> >>> On 02/02/2012 04:29 AM, Srivatsa S. Bhat wrote:
> >>> Hi Rafael,
> >>>
> >>> I have reworked the changelog of this patch to indicate that this fixes a
> >>> regression. Here is the updated patch:
> >>
> >> Well, I've already fixed that up in my tree, but thanks anyway. :-)
> >>
> >
> > Nicer if gurus open wider windows for patches received, no?
> >
>
>
> I am not quite sure what you meant by that, but just in case you got the
> wrong idea by any chance, let me clarify:
>
> Working with Rafael has *always* been an immensely pleasurable experience!
> There is absolutely no doubt in that :-)
Well, thanks! :-)
Rafael
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH Updated] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails
2012-02-05 10:57 ` Srivatsa S. Bhat
2012-02-05 11:09 ` Rafael J. Wysocki
@ 2012-02-05 11:31 ` Hillf Danton
1 sibling, 0 replies; 13+ messages in thread
From: Hillf Danton @ 2012-02-05 11:31 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: Rafael J. Wysocki, Tejun Heo, pavel, len.brown, linux-pm,
linux-kernel, Nigel Cunningham
On Sun, Feb 5, 2012 at 6:57 PM, Srivatsa S. Bhat
<srivatsa.bhat@linux.vnet.ibm.com> wrote:
> On 02/05/2012 11:36 AM, Hillf Danton wrote:
>
>> On Sun, Feb 5, 2012 at 6:27 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>>> On Saturday, February 04, 2012, Srivatsa S. Bhat wrote:
>>>> On 02/02/2012 04:29 AM, Srivatsa S. Bhat wrote:
>>>> Hi Rafael,
>>>>
>>>> I have reworked the changelog of this patch to indicate that this fixes a
>>>> regression. Here is the updated patch:
>>>
>>> Well, I've already fixed that up in my tree, but thanks anyway. :-)
>>>
>>
>> Nicer if gurus open wider windows for patches received, no?
>>
>
>
> I am not quite sure what you meant by that, but just in case you got the
> wrong idea by any chance, let me clarify:
>
> Working with Rafael has *always* been an immensely pleasurable experience!
> There is absolutely no doubt in that :-)
>
> So there really is no reason to feel bad about the above incident :-)
>
Yes, no reason not to keep mind open.
Good weekend
Hillf
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-02-05 11:31 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-01 22:31 [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails Srivatsa S. Bhat
2012-02-01 22:36 ` Tejun Heo
2012-02-01 22:49 ` Rafael J. Wysocki
2012-02-01 22:59 ` Srivatsa S. Bhat
2012-02-02 3:15 ` Nigel Cunningham
2012-02-02 19:38 ` Rafael J. Wysocki
2012-02-04 14:11 ` [PATCH Updated] " Srivatsa S. Bhat
2012-02-04 22:27 ` Rafael J. Wysocki
2012-02-05 6:06 ` Hillf Danton
2012-02-05 10:57 ` Srivatsa S. Bhat
2012-02-05 11:09 ` Rafael J. Wysocki
2012-02-05 11:31 ` Hillf Danton
2012-02-05 10:59 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).