linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] PM / Freezer: Fix the semantics of thaw_processes() and thaw_kernel_threads()
@ 2012-01-30 22:04 Srivatsa S. Bhat
  2012-01-30 22:04 ` [PATCH 1/4] PM/Freezer: Make thaw_processes() thaw only userspace tasks Srivatsa S. Bhat
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Srivatsa S. Bhat @ 2012-01-30 22:04 UTC (permalink / raw)
  To: rjw; +Cc: pavel, len.brown, tj, linux-pm, linux-kernel, Srivatsa S. Bhat

With the introduction of thaw_kernel_threads(), we can now finally fix the
semantics of thawing tasks, in such a way as to be just the opposite of
how tasks are frozen. This opens up some opportunities for code cleanup
as well. This patchset implements both of these improvements.

This patchset applies on top of the patch posted at
https://lkml.org/lkml/2012/1/30/439

--
 Srivatsa S. Bhat (4):
      PM/Freezer: Make thaw_processes() thaw only userspace tasks
      PM/Freezer: Use thaw_processes() and thaw_kernel_threads() correctly
      PM/Hibernate: Thaw kernel threads in hibernation_snapshot() in error/test path
      PM/Hibernate: Refactor and simplify freezer_test_done


  kernel/power/hibernate.c |   22 +++++++++++++++-------
 kernel/power/power.h     |   15 +++++++++++++--
 kernel/power/process.c   |   13 ++++++-------
 kernel/power/user.c      |   20 ++++++++++++--------
 4 files changed, 46 insertions(+), 24 deletions(-)


Regards,
Srivatsa S. Bhat
IBM Linux Technology Center


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] PM/Freezer: Make thaw_processes() thaw only userspace tasks
  2012-01-30 22:04 [PATCH 0/4] PM / Freezer: Fix the semantics of thaw_processes() and thaw_kernel_threads() Srivatsa S. Bhat
@ 2012-01-30 22:04 ` Srivatsa S. Bhat
  2012-01-30 22:09   ` Tejun Heo
  2012-01-30 22:05 ` [PATCH 2/4] PM/Freezer: Use thaw_processes() and thaw_kernel_threads() correctly Srivatsa S. Bhat
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Srivatsa S. Bhat @ 2012-01-30 22:04 UTC (permalink / raw)
  To: rjw; +Cc: pavel, len.brown, tj, linux-pm, linux-kernel, Srivatsa S. Bhat

Currently the situation is:

freeze_processes() - freezes only userspace tasks
freeze_kernel_threads() - freezes only kernel threads
thaw_kernel_threads() - thaws only kernel threads
thaw_processes() - thaws *everything* (both userspace tasks and kernel threads)

The point that thaw_processes() thaws everything is rather unintuitive
and can lead to bugs. So, modify thaw_processes() so that it thaws only
userspace processes. This way we can also have more control over what
exactly gets thawed in different situations.

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

 kernel/power/process.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/power/process.c b/kernel/power/process.c
index eeca003..def6b1b 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -170,17 +170,15 @@ void thaw_processes(void)
 	if (pm_freezing)
 		atomic_dec(&system_freezing_cnt);
 	pm_freezing = false;
-	pm_nosig_freezing = false;
 
 	oom_killer_enable();
 
 	printk("Restarting tasks ... ");
 
-	thaw_workqueues();
-
 	read_lock(&tasklist_lock);
 	do_each_thread(g, p) {
-		__thaw_task(p);
+		if (!(p->flags & (PF_KTHREAD | PF_WQ_WORKER)))
+			__thaw_task(p);
 	} while_each_thread(g, p);
 	read_unlock(&tasklist_lock);
 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/4] PM/Freezer: Use thaw_processes() and thaw_kernel_threads() correctly
  2012-01-30 22:04 [PATCH 0/4] PM / Freezer: Fix the semantics of thaw_processes() and thaw_kernel_threads() Srivatsa S. Bhat
  2012-01-30 22:04 ` [PATCH 1/4] PM/Freezer: Make thaw_processes() thaw only userspace tasks Srivatsa S. Bhat
@ 2012-01-30 22:05 ` Srivatsa S. Bhat
  2012-01-30 22:05 ` [PATCH 3/4] PM/Hibernate: Thaw kernel threads in hibernation_snapshot() in error/test path Srivatsa S. Bhat
  2012-01-30 22:05 ` [PATCH 4/4] PM/Hibernate: Refactor and simplify freezer_test_done Srivatsa S. Bhat
  3 siblings, 0 replies; 8+ messages in thread
From: Srivatsa S. Bhat @ 2012-01-30 22:05 UTC (permalink / raw)
  To: rjw; +Cc: pavel, len.brown, tj, linux-pm, linux-kernel, Srivatsa S. Bhat

Now thaw_processes() thaws only userspace tasks (instead of thawing
everything, unlike what it used to do before). So fix the uses of
thaw_processes() by using thaw_kernel_threads() where appropriate.

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

 kernel/power/hibernate.c |    5 +++++
 kernel/power/power.h     |   15 +++++++++++++--
 kernel/power/process.c   |    7 ++++---
 kernel/power/user.c      |    8 ++++++++
 4 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index a5d4cf0..1670387 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -656,6 +656,11 @@ int hibernate(void)
 	}
 
  Thaw:
+	/*
+	 * Kernel threads had been frozen inside hibernation_snapshot().
+	 * So thaw them here, on the success path.
+	 */
+	thaw_kernel_threads();
 	thaw_processes();
  Finish:
 	free_basic_memory_bitmaps();
diff --git a/kernel/power/power.h b/kernel/power/power.h
index 0c4defe..e37a1e0 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -231,12 +231,23 @@ 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();
+	if (error)
+		goto Finish;
+
+	error = freeze_kernel_threads();
+	if (error)
+		thaw_processes();
+
+Finish:
+	return error;
 }
 
 static inline void suspend_thaw_processes(void)
 {
+	thaw_kernel_threads();
 	thaw_processes();
 }
 #else
diff --git a/kernel/power/process.c b/kernel/power/process.c
index def6b1b..8ae6c4b 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -116,7 +116,8 @@ static int try_to_freeze_tasks(bool user_only)
 /**
  * freeze_processes - Signal user space processes to enter the refrigerator.
  *
- * On success, returns 0.  On failure, -errno and system is fully thawed.
+ * On success, returns 0.  On failure, -errno and all user space processes
+ * are thawed.
  */
 int freeze_processes(void)
 {
@@ -143,7 +144,7 @@ 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 all kernel threads are thawed.
  */
 int freeze_kernel_threads(void)
 {
@@ -159,7 +160,7 @@ int freeze_kernel_threads(void)
 	BUG_ON(in_atomic());
 
 	if (error)
-		thaw_processes();
+		thaw_kernel_threads();
 	return error;
 }
 
diff --git a/kernel/power/user.c b/kernel/power/user.c
index 3e10007..eaf5c97 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -119,6 +119,7 @@ static int snapshot_release(struct inode *inode, struct file *filp)
 	free_all_swap_pages(data->swap);
 	if (data->frozen) {
 		pm_restore_gfp_mask();
+		thaw_kernel_threads();
 		thaw_processes();
 	}
 	pm_notifier_call_chain(data->mode == O_RDONLY ?
@@ -237,6 +238,13 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
 		if (!data->frozen || data->ready)
 			break;
 		pm_restore_gfp_mask();
+
+		/*
+		 * Be paranoid and thaw kernel threads as well, in order to
+		 * make sure that the system gets thawed fully, irrespective
+		 * of which (error) path we took to come here.
+		 */
+		thaw_kernel_threads();
 		thaw_processes();
 		usermodehelper_enable();
 		data->frozen = 0;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/4] PM/Hibernate: Thaw kernel threads in hibernation_snapshot() in error/test path
  2012-01-30 22:04 [PATCH 0/4] PM / Freezer: Fix the semantics of thaw_processes() and thaw_kernel_threads() Srivatsa S. Bhat
  2012-01-30 22:04 ` [PATCH 1/4] PM/Freezer: Make thaw_processes() thaw only userspace tasks Srivatsa S. Bhat
  2012-01-30 22:05 ` [PATCH 2/4] PM/Freezer: Use thaw_processes() and thaw_kernel_threads() correctly Srivatsa S. Bhat
@ 2012-01-30 22:05 ` Srivatsa S. Bhat
  2012-01-30 22:05 ` [PATCH 4/4] PM/Hibernate: Refactor and simplify freezer_test_done Srivatsa S. Bhat
  3 siblings, 0 replies; 8+ messages in thread
From: Srivatsa S. Bhat @ 2012-01-30 22:05 UTC (permalink / raw)
  To: rjw; +Cc: pavel, len.brown, tj, linux-pm, linux-kernel, Srivatsa S. Bhat

In the hibernation call path, the kernel threads are frozen inside
hibernation_snapshot(). If we happen to encounter an error further down
the road or if we are exiting early due to a successful freezer test,
then thaw kernel threads before returning to the caller.

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

 kernel/power/hibernate.c |    9 ++++++---
 kernel/power/user.c      |    8 ++------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 1670387..0ce1220 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -343,13 +343,13 @@ int hibernation_snapshot(int platform_mode)
 		 * successful freezer test.
 		 */
 		freezer_test_done = true;
-		goto Cleanup;
+		goto Thaw;
 	}
 
 	error = dpm_prepare(PMSG_FREEZE);
 	if (error) {
 		dpm_complete(PMSG_RECOVER);
-		goto Cleanup;
+		goto Thaw;
 	}
 
 	suspend_console();
@@ -385,6 +385,8 @@ int hibernation_snapshot(int platform_mode)
 	platform_end(platform_mode);
 	return error;
 
+ Thaw:
+	thaw_kernel_threads();
  Cleanup:
 	swsusp_free();
 	goto Close;
@@ -655,12 +657,13 @@ int hibernate(void)
 		pr_debug("PM: Image restored successfully.\n");
 	}
 
- Thaw:
 	/*
 	 * Kernel threads had been frozen inside hibernation_snapshot().
 	 * So thaw them here, on the success path.
 	 */
 	thaw_kernel_threads();
+
+ Thaw:
 	thaw_processes();
  Finish:
 	free_basic_memory_bitmaps();
diff --git a/kernel/power/user.c b/kernel/power/user.c
index eaf5c97..62dbe93 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -257,16 +257,12 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
 		}
 		pm_restore_gfp_mask();
 		error = hibernation_snapshot(data->platform_support);
-		if (error) {
-			thaw_kernel_threads();
-		} else {
+		if (!error) {
 			error = put_user(in_suspend, (int __user *)arg);
 			if (!error && !freezer_test_done)
 				data->ready = 1;
-			if (freezer_test_done) {
+			if (freezer_test_done)
 				freezer_test_done = false;
-				thaw_kernel_threads();
-			}
 		}
 		break;
 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 4/4] PM/Hibernate: Refactor and simplify freezer_test_done
  2012-01-30 22:04 [PATCH 0/4] PM / Freezer: Fix the semantics of thaw_processes() and thaw_kernel_threads() Srivatsa S. Bhat
                   ` (2 preceding siblings ...)
  2012-01-30 22:05 ` [PATCH 3/4] PM/Hibernate: Thaw kernel threads in hibernation_snapshot() in error/test path Srivatsa S. Bhat
@ 2012-01-30 22:05 ` Srivatsa S. Bhat
  3 siblings, 0 replies; 8+ messages in thread
From: Srivatsa S. Bhat @ 2012-01-30 22:05 UTC (permalink / raw)
  To: rjw; +Cc: pavel, len.brown, tj, linux-pm, linux-kernel, Srivatsa S. Bhat

The code related to 'freezer_test_done' is needlessly convoluted.
Refactor the code and simplify the implementation.

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

 kernel/power/hibernate.c |   10 +++++-----
 kernel/power/user.c      |    4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 0ce1220..30b315a 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -629,12 +629,8 @@ int hibernate(void)
 		goto Finish;
 
 	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
-	if (error)
-		goto Thaw;
-	if (freezer_test_done) {
-		freezer_test_done = false;
+	if (error || freezer_test_done)
 		goto Thaw;
-	}
 
 	if (in_suspend) {
 		unsigned int flags = 0;
@@ -665,6 +661,10 @@ int hibernate(void)
 
  Thaw:
 	thaw_processes();
+
+	/* Don't bother checking whether freezer_test_done is true */
+	freezer_test_done = false;
+
  Finish:
 	free_basic_memory_bitmaps();
 	usermodehelper_enable();
diff --git a/kernel/power/user.c b/kernel/power/user.c
index 62dbe93..056088e 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -259,10 +259,10 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
 		error = hibernation_snapshot(data->platform_support);
 		if (!error) {
 			error = put_user(in_suspend, (int __user *)arg);
-			if (!error && !freezer_test_done)
-				data->ready = 1;
 			if (freezer_test_done)
 				freezer_test_done = false;
+			else if (!error)
+				data->ready = 1;
 		}
 		break;
 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/4] PM/Freezer: Make thaw_processes() thaw only userspace tasks
  2012-01-30 22:04 ` [PATCH 1/4] PM/Freezer: Make thaw_processes() thaw only userspace tasks Srivatsa S. Bhat
@ 2012-01-30 22:09   ` Tejun Heo
  2012-01-30 22:23     ` Srivatsa S. Bhat
  0 siblings, 1 reply; 8+ messages in thread
From: Tejun Heo @ 2012-01-30 22:09 UTC (permalink / raw)
  To: Srivatsa S. Bhat; +Cc: rjw, pavel, len.brown, linux-pm, linux-kernel

On Tue, Jan 31, 2012 at 03:34:57AM +0530, Srivatsa S. Bhat wrote:
> Currently the situation is:
> 
> freeze_processes() - freezes only userspace tasks
> freeze_kernel_threads() - freezes only kernel threads
> thaw_kernel_threads() - thaws only kernel threads
> thaw_processes() - thaws *everything* (both userspace tasks and kernel threads)
> 
> The point that thaw_processes() thaws everything is rather unintuitive
> and can lead to bugs. So, modify thaw_processes() so that it thaws only
> userspace processes. This way we can also have more control over what
> exactly gets thawed in different situations.
> 
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>

Maybe I'm misreading it but doesn't this introduce window where kernel
tasks aren't thawed between this patch and the following ones?  It
looks like this one should come later.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/4] PM/Freezer: Make thaw_processes() thaw only userspace tasks
  2012-01-30 22:09   ` Tejun Heo
@ 2012-01-30 22:23     ` Srivatsa S. Bhat
  2012-01-30 22:27       ` Tejun Heo
  0 siblings, 1 reply; 8+ messages in thread
From: Srivatsa S. Bhat @ 2012-01-30 22:23 UTC (permalink / raw)
  To: Tejun Heo; +Cc: rjw, pavel, len.brown, linux-pm, linux-kernel

On 01/31/2012 03:39 AM, Tejun Heo wrote:

> On Tue, Jan 31, 2012 at 03:34:57AM +0530, Srivatsa S. Bhat wrote:
>> Currently the situation is:
>>
>> freeze_processes() - freezes only userspace tasks
>> freeze_kernel_threads() - freezes only kernel threads
>> thaw_kernel_threads() - thaws only kernel threads
>> thaw_processes() - thaws *everything* (both userspace tasks and kernel threads)
>>
>> The point that thaw_processes() thaws everything is rather unintuitive
>> and can lead to bugs. So, modify thaw_processes() so that it thaws only
>> userspace processes. This way we can also have more control over what
>> exactly gets thawed in different situations.
>>
>> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> 
> Maybe I'm misreading it but doesn't this introduce window where kernel
> tasks aren't thawed between this patch and the following ones?  It
> looks like this one should come later.
> 


Yes, I was aware that it introduces such a window. But I ignored it in the
interest of making the patch series sensible (as in, for example, patch
2/4 wouldn't make much sense without patch 1/4).
Maybe I will interchange patch 1 and patch 2 and just reword the patch
descriptions suitably so that they still make sense..

Thanks a lot!

Regards,
Srivatsa S. Bhat


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/4] PM/Freezer: Make thaw_processes() thaw only userspace tasks
  2012-01-30 22:23     ` Srivatsa S. Bhat
@ 2012-01-30 22:27       ` Tejun Heo
  0 siblings, 0 replies; 8+ messages in thread
From: Tejun Heo @ 2012-01-30 22:27 UTC (permalink / raw)
  To: Srivatsa S. Bhat; +Cc: rjw, pavel, len.brown, linux-pm, linux-kernel

On Tue, Jan 31, 2012 at 03:53:05AM +0530, Srivatsa S. Bhat wrote:
> Yes, I was aware that it introduces such a window. But I ignored it in the
> interest of making the patch series sensible (as in, for example, patch
> 2/4 wouldn't make much sense without patch 1/4).
> Maybe I will interchange patch 1 and patch 2 and just reword the patch
> descriptions suitably so that they still make sense..

Yeah, I think saying sth like "Make extra calls to thaw_kthread in
preparation for blah blah.." should do.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-01-30 22:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-30 22:04 [PATCH 0/4] PM / Freezer: Fix the semantics of thaw_processes() and thaw_kernel_threads() Srivatsa S. Bhat
2012-01-30 22:04 ` [PATCH 1/4] PM/Freezer: Make thaw_processes() thaw only userspace tasks Srivatsa S. Bhat
2012-01-30 22:09   ` Tejun Heo
2012-01-30 22:23     ` Srivatsa S. Bhat
2012-01-30 22:27       ` Tejun Heo
2012-01-30 22:05 ` [PATCH 2/4] PM/Freezer: Use thaw_processes() and thaw_kernel_threads() correctly Srivatsa S. Bhat
2012-01-30 22:05 ` [PATCH 3/4] PM/Hibernate: Thaw kernel threads in hibernation_snapshot() in error/test path Srivatsa S. Bhat
2012-01-30 22:05 ` [PATCH 4/4] PM/Hibernate: Refactor and simplify freezer_test_done Srivatsa S. Bhat

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).