[PATCH] Thaw userspace and kernel space separately.

[PATCH] Thaw userspace and kernel space separately.

[Nigel Cunningham]

Modify process thawing so that we can thaw kernel space without thawing
userspace, and thaw kernelspace first. This will be useful in later
patches, where I intend to get swsusp thawing kernel threads only before
seeking to free memory.

Signed-off-by: Nigel Cunningham <nigel@suspend2.net>

diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 266373f..294ebea 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -1,5 +1,8 @@
 /* Freezer declarations */
 
+#define FREEZER_KERNEL_THREADS 0
+#define FREEZER_ALL_THREADS 1
+
 #ifdef CONFIG_PM
 /*
  * Check if a process has been frozen
@@ -57,7 +60,8 @@ static inline void frozen_process(struct
 
 extern void refrigerator(void);
 extern int freeze_processes(void);
-extern void thaw_processes(void);
+#define thaw_processes() do { thaw_some_processes(FREEZER_ALL_THREADS); } while(0)
+#define thaw_kernel_threads() do { thaw_some_processes(FREEZER_KERNEL_THREADS); } while(0)
 
 static inline int try_to_freeze(void)
 {
@@ -67,6 +71,9 @@ static inline int try_to_freeze(void)
 	} else
 		return 0;
 }
+
+extern void thaw_some_processes(int all);
+
 #else
 static inline int frozen(struct task_struct *p) { return 0; }
 static inline int freezing(struct task_struct *p) { return 0; }
diff --git a/kernel/power/process.c b/kernel/power/process.c
index fedabad..4a001fe 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -153,18 +153,29 @@ int freeze_processes(void)
 	return 0;
 }
 
-void thaw_processes(void)
+void thaw_some_processes(int all)
 {
 	struct task_struct *g, *p;
+	int pass = 0; /* Pass 0 = Kernel space, 1 = Userspace */
 
 	printk("Restarting tasks... ");
 	read_lock(&tasklist_lock);
-	do_each_thread(g, p) {
-		if (!freezeable(p))
-			continue;
-		if (!thaw_process(p))
-			printk(KERN_INFO "Strange, %s not stopped\n", p->comm);
-	} while_each_thread(g, p);
+	do {
+		do_each_thread(g, p) {
+			/* 
+			 * is_user = 0 if kernel thread or borrowed mm,
+			 * 1 otherwise.
+			 */
+			int is_user = !!(p->mm && !(p->flags & PF_BORROWED_MM));
+			if (!freezeable(p) || (is_user != pass))
+				continue;
+			if (!thaw_process(p))
+				printk(KERN_INFO
+					"Strange, %s not stopped\n", p->comm );
+		} while_each_thread(g, p);
+
+		pass++;
+	} while (pass < 2 && all);
 
 	read_unlock(&tasklist_lock);
 	schedule();

Re: [PATCH] Thaw userspace and kernel space separately.

[Rafael J. Wysocki]

On Monday, 23 October 2006 01:48, Nigel Cunningham wrote:
> Modify process thawing so that we can thaw kernel space without thawing
> userspace, and thaw kernelspace first. This will be useful in later
> patches, where I intend to get swsusp thawing kernel threads only before
> seeking to free memory.

Please explain why you think it will be necessary/useful.

I remember a discussion about it some time ago that didn't indicate
we would need/want to do this.

Greetings,
Rafael


-- 
You never change things by fighting the existing reality.
		R. Buckminster Fuller

Re: [PATCH] Thaw userspace and kernel space separately.

[Nigel Cunningham]

Hi.

On Mon, 2006-10-23 at 12:26 +0200, Rafael J. Wysocki wrote:
> On Monday, 23 October 2006 01:48, Nigel Cunningham wrote:
> > Modify process thawing so that we can thaw kernel space without thawing
> > userspace, and thaw kernelspace first. This will be useful in later
> > patches, where I intend to get swsusp thawing kernel threads only before
> > seeking to free memory.
> 
> Please explain why you think it will be necessary/useful.
> 
> I remember a discussion about it some time ago that didn't indicate
> we would need/want to do this.

This is needed to make suspending faster and more reliable when the
system is in a low memory situation. Imagine that you have a number of
processes trying to allocate memory at the time you're trying to
suspend. They want so much memory that when you come to prepare the
image, you find that you need to free pages. But your swapfile is on
ext3, and you've just frozen all processes, so any attempt to free
memory could result in a deadlock while the vm tries to swap out pages
using the frozen kjournald. So you need to thaw processes to free the
memory. But thawing processes will start the processes allocating memory
again, so you'll be fighting an uphill battle.

If you can only thaw the kernel threads, you can free memory without
restarting userspace or deadlocking against a frozen kjournald.

Regards,

Nigel

Re: [PATCH] Thaw userspace and kernel space separately.

[Rafael J. Wysocki]

On Monday, 23 October 2006 14:00, Nigel Cunningham wrote:
> Hi.
> 
> On Mon, 2006-10-23 at 12:26 +0200, Rafael J. Wysocki wrote:
> > On Monday, 23 October 2006 01:48, Nigel Cunningham wrote:
> > > Modify process thawing so that we can thaw kernel space without thawing
> > > userspace, and thaw kernelspace first. This will be useful in later
> > > patches, where I intend to get swsusp thawing kernel threads only before
> > > seeking to free memory.
> > 
> > Please explain why you think it will be necessary/useful.
> > 
> > I remember a discussion about it some time ago that didn't indicate
> > we would need/want to do this.
> 
> This is needed to make suspending faster and more reliable when the
> system is in a low memory situation. Imagine that you have a number of
> processes trying to allocate memory at the time you're trying to
> suspend. They want so much memory that when you come to prepare the
> image, you find that you need to free pages. But your swapfile is on
> ext3, and you've just frozen all processes, so any attempt to free
> memory could result in a deadlock while the vm tries to swap out pages
> using the frozen kjournald.

This is not true, sorry.

Swapfiles are handled at the block device level.  The filesystem is only
needed when you run swapon, to create swap extents that are used later
to handle the swap file.

Greetings,
Rafael


-- 
You never change things by fighting the existing reality.
		R. Buckminster Fuller

Re: [PATCH] Thaw userspace and kernel space separately.

[Andrew Morton]

> On Mon, 23 Oct 2006 22:00:11 +1000 Nigel Cunningham <ncunningham@linuxmail.org> wrote:
> Hi.
> 
> On Mon, 2006-10-23 at 12:26 +0200, Rafael J. Wysocki wrote:
> > On Monday, 23 October 2006 01:48, Nigel Cunningham wrote:
> > > Modify process thawing so that we can thaw kernel space without thawing
> > > userspace, and thaw kernelspace first. This will be useful in later
> > > patches, where I intend to get swsusp thawing kernel threads only before
> > > seeking to free memory.
> > 
> > Please explain why you think it will be necessary/useful.
> > 
> > I remember a discussion about it some time ago that didn't indicate
> > we would need/want to do this.
> 
> This is needed to make suspending faster and more reliable when the
> system is in a low memory situation. Imagine that you have a number of
> processes trying to allocate memory at the time you're trying to
> suspend. They want so much memory that when you come to prepare the
> image, you find that you need to free pages. But your swapfile is on
> ext3, and you've just frozen all processes, so any attempt to free
> memory could result in a deadlock while the vm tries to swap out pages
> using the frozen kjournald. So you need to thaw processes to free the
> memory. But thawing processes will start the processes allocating memory
> again, so you'll be fighting an uphill battle.
> 
> If you can only thaw the kernel threads, you can free memory without
> restarting userspace or deadlocking against a frozen kjournald.
> 

kjournald will not participate in writing to swapfiles.

The situation where we would need this feature is where the loop driver is
involved in the path-to-disk.  But I doubt if that's a thing we'd want to
support.

otoh there may be other kernel threads which are a saner thing to have in
the swapout path and which we do want to support.  md_thread, perhaps?

Re: [PATCH] Thaw userspace and kernel space separately.

[Rafael J. Wysocki]

On Monday, 23 October 2006 18:51, Andrew Morton wrote:
> > On Mon, 23 Oct 2006 22:00:11 +1000 Nigel Cunningham <ncunningham@linuxmail.org> wrote:
> > Hi.
> > 
> > On Mon, 2006-10-23 at 12:26 +0200, Rafael J. Wysocki wrote:
> > > On Monday, 23 October 2006 01:48, Nigel Cunningham wrote:
> > > > Modify process thawing so that we can thaw kernel space without thawing
> > > > userspace, and thaw kernelspace first. This will be useful in later
> > > > patches, where I intend to get swsusp thawing kernel threads only before
> > > > seeking to free memory.
> > > 
> > > Please explain why you think it will be necessary/useful.
> > > 
> > > I remember a discussion about it some time ago that didn't indicate
> > > we would need/want to do this.
> > 
> > This is needed to make suspending faster and more reliable when the
> > system is in a low memory situation. Imagine that you have a number of
> > processes trying to allocate memory at the time you're trying to
> > suspend. They want so much memory that when you come to prepare the
> > image, you find that you need to free pages. But your swapfile is on
> > ext3, and you've just frozen all processes, so any attempt to free
> > memory could result in a deadlock while the vm tries to swap out pages
> > using the frozen kjournald. So you need to thaw processes to free the
> > memory. But thawing processes will start the processes allocating memory
> > again, so you'll be fighting an uphill battle.
> > 
> > If you can only thaw the kernel threads, you can free memory without
> > restarting userspace or deadlocking against a frozen kjournald.
> > 
> 
> kjournald will not participate in writing to swapfiles.
> 
> The situation where we would need this feature is where the loop driver is
> involved in the path-to-disk.  But I doubt if that's a thing we'd want to
> support.
> 
> otoh there may be other kernel threads which are a saner thing to have in
> the swapout path and which we do want to support.  md_thread, perhaps?

md_thread needs some consideration I think.  Having a swapfile on RAID
is a legit thing and we should support that.


-- 
You never change things by fighting the existing reality.
		R. Buckminster Fuller

Re: [PATCH] Thaw userspace and kernel space separately.

[dean gaudet]

On Mon, 23 Oct 2006, Andrew Morton wrote:

> > On Mon, 23 Oct 2006 22:00:11 +1000 Nigel Cunningham <ncunningham@linuxmail.org> wrote:
> > If you can only thaw the kernel threads, you can free memory without
> > restarting userspace or deadlocking against a frozen kjournald.
> > 
> 
> kjournald will not participate in writing to swapfiles.
> 
> The situation where we would need this feature is where the loop driver is
> involved in the path-to-disk.  But I doubt if that's a thing we'd want to
> support.

dm-crypt?  that seems like a very important thing to support for suspend 
to disk.

-dean

Re: [PATCH] Thaw userspace and kernel space separately.

[Pavel Machek]

Hi!

> Modify process thawing so that we can thaw kernel space without thawing
> userspace, and thaw kernelspace first. This will be useful in later
> patches, where I intend to get swsusp thawing kernel threads only before
> seeking to free memory.
> 
> Signed-off-by: Nigel Cunningham <nigel@suspend2.net>

NAK. "May be useful in future" is not good reason to merge it now. (If
you did not want it merged, just mark it so).
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Thaw userspace and kernel space separately.

[Bill Davidsen]

Pavel Machek wrote:
> Hi!
> 
>> Modify process thawing so that we can thaw kernel space without thawing
>> userspace, and thaw kernelspace first. This will be useful in later
>> patches, where I intend to get swsusp thawing kernel threads only before
>> seeking to free memory.
>>
>> Signed-off-by: Nigel Cunningham <nigel@suspend2.net>
> 
> NAK. "May be useful in future" is not good reason to merge it now. (If
> you did not want it merged, just mark it so).

I hope Nigel will keep working at it, since low memory machines like old 
laptops would benefit from suspend are a reality. It can be kept as a 
separate patch somewhere, like suspend2, which people can patch in to 
enhance the example code currently in the kernel.

-- 
Bill Davidsen <davidsen@tmr.com>
   Obscure bug of 2004: BASH BUFFER OVERFLOW - if bash is being run by a
normal user and is setuid root, with the "vi" line edit mode selected,
and the character set is "big5," an off-by-one errors occurs during
wildcard (glob) expansion.