Messages in this thread | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH] PM/Freezer: Thaw only kernel threads if freezing of kernel threads fails | Date | Wed, 1 Feb 2012 23:49:04 +0100 |
| |
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
| |