New Software Suspend Patch for testing.

New Software Suspend Patch for testing.

[Nigel Cunningham]

Hi.

A new 2.5 port of the 2.4 version is available on
www.sourceforge.net/projects/swsusp. 

The changes from the last version are not huge; perhaps most significant
is that the dynamically allocated bitmaps (in place of pageflags) are
implemented. This won't mean anything to most people, but Andrew Morton,
Pavel and Patrick should be happy. Note for Patrick: The driver code was
using KERN_EMERGENCY to print when devices were suspended/resumed. Can
we lower this to INFO (I did something alone these lines in this patch).
It mucks up the nice display :>

Hopefully I'll soon find time to start bombarding Pavel, Patrick and
Linus with incremental patches for merging :>. In the meantime, please
give it a go.

Under 2.4, we use scripts that unload problematic drivers, switch to a
text console and so on before starting the process. Hopefully this will
end up being unneeded in 2.5 once the driver model is complete. In the
meantime, however (especially if you get problems!), you might like to
try first with a minimal load, and slowly add things. It does work fine
on my Omnibook XE3! YMMV.

Brief howto:
(Assuming you've applied the patch against 2.5.66 - other versions may
work too).
1. Patch, compile as normal.
2. Ensure your kernel commandline includes resume=/dev/hdaX where X is a
swap partition you will be using. More than one swap partition can be
used. This just specifies which will have its header used to indicate
that the system is suspended and where to find the data.
3. If you want debugging info: echo 1 20 15 31 > /proc/sys/kernel/swsusp
(See line 201ff of kernel/suspend.c for what the numbers mean).
4. echo 4 > /proc/acpi/sleep to start the process. 
The system should save the image and powerdown.
If you choose the debugging info, you'll have to press SHIFT at the end
of each step.
If you press SHIFT at other times, you can toggle this pausing on and
off.
Whether you selected the debugging info or not, you can press ALT to
cancel the process.
5. To resume, reboot with the same kernel. The image should be detected
and reloaded without you needing to do anything special. Pausing or not
is state-persistent from when you suspended.
6. You should (DV) get your system back to where it was when you started
the process.

Please report success/failure/questions on the swsusp mailing list. See
Florent's home page for details.

Regards,

Nigel


-- 
Nigel Cunningham
495 St Georges Road South, Hastings 4201, New Zealand

Be diligent to present yourself approved to God as a workman who does
not need to be ashamed, handling accurately the word of truth.
	-- 2 Timothy 2:14, NASB.

Re: New Software Suspend Patch for testing.

[Pavel Machek]

Hi!

> Hopefully I'll soon find time to start bombarding Pavel, Patrick and
> Linus with incremental patches for merging :>. In the meantime, please
> give it a go.

Okay, lets take a look.

diff -ruN linux-2.5.66-original/arch/i386/kernel/cpu/mtrr/main.c linux-2.5.66-04/arch/i386/kernel/cpu/mtrr/main.c
--- linux-2.5.66-original/arch/i386/kernel/cpu/mtrr/main.c	2003-01-15 17:00:38.000000000 +1300
+++ linux-2.5.66-04/arch/i386/kernel/cpu/mtrr/main.c	2003-03-26 09:00:29.000000000 +1200
@@ -35,6 +35,7 @@
 #include <linux/init.h>
 #include <linux/pci.h>
 #include <linux/smp.h>
+#include <linux/suspend.h>
 
 #include <asm/mtrr.h>
 
@@ -644,6 +645,65 @@
     "write-protect",            /* 5 */
     "write-back",               /* 6 */
 };
-
+ 
+#ifdef SOFTWARE_SUSPEND_MTRR
+struct mtrr_suspend_state
+{
+     mtrr_type ltype;
+     unsigned long lbase;
+     unsigned int lsize;
+};
...

Please convert this to driver model and submit to mtrr maintainer.

diff -ruN linux-2.5.66-original/drivers/base/power.c linux-2.5.66-04/drivers/base/power.c
--- linux-2.5.66-original/drivers/base/power.c	2003-01-15 17:01:06.000000000 +1300
+++ linux-2.5.66-04/drivers/base/power.c	2003-04-04 20:51:40.000000000 +1200
@@ -35,8 +35,6 @@
 	struct list_head * node;
 	int error = 0;
 
-	printk(KERN_EMERG "Suspending devices\n");
-
 	down_write(&devices_subsys.rwsem);
 	list_for_each(node,&devices_subsys.kset.list) {
 		struct device * dev = to_dev(node);
@@ -73,7 +71,7 @@
 	}
 	up_write(&devices_subsys.rwsem);
 
-	printk(KERN_EMERG "Devices Resumed\n");
+	printk(KERN_INFO "Devices Resumed\n");
 }
 
 /**
@@ -83,7 +81,7 @@
 {
 	struct list_head * entry;
 	
-	printk(KERN_EMERG "Shutting down devices\n");
+	printk(KERN_INFO "Shutting down devices\n");
 
 	down_write(&devices_subsys.rwsem);
 	list_for_each(entry,&devices_subsys.kset.list) {

I guess that submitting this as trivial is okay.

diff -ruN linux-2.5.66-original/drivers/char/vt.c linux-2.5.66-04/drivers/char/vt.c
--- linux-2.5.66-original/drivers/char/vt.c	2003-03-26 08:56:47.000000000 +1200
+++ linux-2.5.66-04/drivers/char/vt.c	2003-03-26 09:00:29.000000000 +1200
@@ -149,13 +149,13 @@
 static void vc_init(unsigned int console, unsigned int rows,
 		    unsigned int cols, int do_clear);
 static void blank_screen(unsigned long dummy);
-static void gotoxy(int currcons, int new_x, int new_y);
+void gotoxy(int currcons, int new_x, int new_y);
 static void save_cur(int currcons);
-static void reset_terminal(int currcons, int do_clear);
+void reset_terminal(int currcons, int do_clear);
 static void con_flush_chars(struct tty_struct *tty);
 static void set_vesa_blanking(unsigned long arg);
 static void set_cursor(int currcons);
-static void hide_cursor(int currcons);
+void hide_cursor(int currcons);
 static void unblank_screen_t(unsigned long dummy);
 static void console_callback(void *ignored);
 static void __init con_init_devfs (void);

This is ugly as night. Is not there any public function (sys_write?)
you could use instead?

diff -ruN linux-2.5.66-original/drivers/ide/ide-disk.c linux-2.5.66-04/drivers/ide/ide-disk.c
--- linux-2.5.66-original/drivers/ide/ide-disk.c	2003-03-26 08:56:49.000000000 +1200
+++ linux-2.5.66-04/drivers/ide/ide-disk.c	2003-04-04 20:22:09.000000000 +1200
@@ -1515,8 +1515,6 @@
 {
 	ide_drive_t *drive = dev->driver_data;
 
-	printk("Suspending device %p\n", dev->driver_data);
-
 	/* I hope that every freeze operation from the upper levels have
 	 * already been done...
 	 */
@@ -1525,7 +1523,6 @@
 		return 0;
 
 	/* set the drive to standby */
-	printk(KERN_INFO "suspending: %s ", drive->name);
 	do_idedisk_standby(drive);
 	drive->blocked = 1;
 
@@ -1539,8 +1536,8 @@
 
 	if (level != RESUME_RESTORE_STATE)
 		return 0;
-	BUG_ON(!drive->blocked);
-	drive->blocked = 0;
+	if (drive->blocked)
+		drive->blocked--;
 	return 0;
 }
 

Please submitt this to alan, as soon as possible.

@@ -1804,7 +1801,8 @@
 	if ((!drive->head || drive->head > 16) && !drive->select.b.lba) {
 		printk(KERN_ERR "%s: INVALID GEOMETRY: %d PHYSICAL HEADS?\n",
 			drive->name, drive->head);
-		if ((drive->id->cfs_enable_2 & 0x3000) && drive->wcache)
+		if (((drive->id->cfs_enable_2 & 0x3000) && drive->wcache) ||
+		    ((drive->id->command_set_1 & 0x20) && drive->id->cfs_enable_1 & 0x20))
 			if (do_idedisk_flushcache(drive))
 				printk (KERN_INFO "%s: Write Cache FAILED Flushing!\n",
 					drive->name);

Is this swsusp related?

diff -ruN linux-2.5.66-original/include/linux/pagemap.h linux-2.5.66-04/include/linux/pagemap.h
--- linux-2.5.66-original/include/linux/pagemap.h	2003-03-26 08:57:06.000000000 +1200
+++ linux-2.5.66-04/include/linux/pagemap.h	2003-03-26 09:00:29.000000000 +1200
@@ -8,6 +8,9 @@
 #include <linux/fs.h>
 #include <linux/list.h>
 #include <linux/highmem.h>
+#ifdef CONFIG_SOFTWARE_SUSPEND
+#include <linux/suspend.h>
+#endif
 #include <asm/uaccess.h>
 
 /*

Do not use #ifdefs around includes.

@@ -123,6 +126,8 @@
 	page->mapping = mapping;
 	page->index = index;
 
+	if (suspend_task) 
+		last_suspend_cache_page = page;
 	mapping->nrpages++;
 	pagecache_acct(1);
 }
diff -ruN linux-2.5.66-original/include/linux/suspend-debug.h linux-2.5.66-04/include/linux/suspend-debug.h
--- linux-2.5.66-original/include/linux/suspend-debug.h	1970-01-01 12:00:00.000000000 +1200
+++ linux-2.5.66-04/include/linux/suspend-debug.h	2003-04-04 19:37:44.000000000 +1200
@@ -0,0 +1,61 @@
+
+#ifndef _LINUX_SWSUSP_DEBUG_H
+#define _LINUX_SWSUSP_DEBUG_H
+
+#include <linux/suspend.h>
+
+#define name_suspend "Suspend Machine:  "
+#define name_resume  "Resume Machine:   "
+#define swsusp_version "beta 19-20"
+#define name_swsusp  "Swsusp " swsusp_version ":   "
+#define console_suspend "S U S P E N D   T O   D I S K"
+#define console_resume "R E S U M E   F R O M   D I S K"

This is ugly. Are you trying to make it hard to read on purpose?

diff -ruN linux-2.5.66-original/include/linux/sysctl.h linux-2.5.66-04/include/linux/sysctl.h
--- linux-2.5.66-original/include/linux/sysctl.h	2003-03-01 15:10:38.000000000 +1300
+++ linux-2.5.66-04/include/linux/sysctl.h	2003-03-26 09:00:29.000000000 +1200
@@ -129,6 +129,7 @@
 	KERN_CADPID=54,		/* int: PID of the process to notify on CAD */
 	KERN_PIDMAX=55,		/* int: PID # limit */
   	KERN_CORE_PATTERN=56,	/* string: pattern for core-file names */
+        KERN_SWSUSP=57,		/* struct: interface to configure & activate software suspension */
 };
 
 

Is sysctl being used besides debugging?
 
 unsigned char software_suspend_enabled = 0;
+unsigned int suspend_task = 0;
+/*
+ * Poll the swsusp state every second
+ */
+#define SWSUSP_CHECK_TIMEOUT	(HZ)

What is this?

 /*
  * XXX: We try to keep some more pages free so that I/O operations succeed
  * without paging. Might this be more?
  */
-#define PAGES_FOR_IO	512
+#define PAGES_FOR_IO 512
+
+#ifdef DEBUG_DEFAULT
+int currentstage = 0;
+int swsusp_state[5] = {0,	/* when set to 1 swsusp_mainloop activates software_suspend (2.4 only)
+				   bit 0: off = normal state, on = suspend required
+				   bit 1: aborting suspend
+				*/

Please kill 2.4 stuff from 2.5 patch.

-	if (mode == MARK_SWAP_RESUME) {
-	  	if (!memcmp("S1",cur->swh.magic.magic,2))
-		  	memcpy(cur->swh.magic.magic,"SWAP-SPACE",10);
-		else if (!memcmp("S2",cur->swh.magic.magic,2))
-			memcpy(cur->swh.magic.magic,"SWAPSPACE2",10);
-		else printk("%sUnable to find suspended-data signature (%.10s - misspelled?\n", 
-		      	name_resume, cur->swh.magic.magic);
-	} else {
-	  	if ((!memcmp("SWAP-SPACE",cur->swh.magic.magic,10)))
-		  	memcpy(cur->swh.magic.magic,"S1SUSP....",10);
-		else if ((!memcmp("SWAPSPACE2",cur->swh.magic.magic,10)))
-			memcpy(cur->swh.magic.magic,"S2SUSP....",10);
-		else panic("\nSwapspace is not swapspace (%.10s)\n", cur->swh.magic.magic);
-		cur->link.next = prev; /* prev is the first/last swap page of the resume area */
-		/* link.next lies *no more* in last 4/8 bytes of magic */
+	switch(mode) {
+	case MARK_SWAP_RESUME:
+		if (!memcmp("1R",cur.pointer->swh.magic.magic,2))
+			memcpy(cur.pointer->swh.magic.magic,"SWAP-SPACE",10);
+		else if (!memcmp("2R",cur.pointer->swh.magic.magic,2))
+			memcpy(cur.pointer->swh.magic.magic,"SWAPSPACE2",10);
+		else printk(name_resume "Unable to find suspended-data signature (%.10s - misspelled?\n", 
+			    cur.pointer->swh.magic.magic);
+		break;

I guess this is nicer than previous code, good.

Snip. Sorry, have to go.
									Pavel


-- 
When do you have heart between your knees?

Re: New Software Suspend Patch for testing.

[Nigel Cunningham]

Hi.

On Sat, 2003-04-05 at 01:30, Pavel Machek wrote:
> diff -ruN linux-2.5.66-original/arch/i386/kernel/cpu/mtrr/main.c linux-2.5.66-04/arch/i386/kernel/cpu/mtrr/main.c
> --- linux-2.5.66-original/arch/i386/kernel/cpu/mtrr/main.c	2003-01-15 17:00:38.000000000 +1300
> +++ linux-2.5.66-04/arch/i386/kernel/cpu/mtrr/main.c	2003-03-26 09:00:29.000000000 +1200
> @@ -35,6 +35,7 @@
> ...
> 
> Please convert this to driver model and submit to mtrr maintainer.

Okee doke.

> diff -ruN linux-2.5.66-original/drivers/char/vt.c linux-2.5.66-04/drivers/char/vt.c
> --- linux-2.5.66-original/drivers/char/vt.c	2003-03-26 08:56:47.000000000 +1200
> +++ linux-2.5.66-04/drivers/char/vt.c	2003-03-26 09:00:29.000000000 +1200

> This is ugly as night. Is not there any public function (sys_write?)
> you could use instead?

I didn't know about sys_write. I'll take a look at it.

> diff -ruN linux-2.5.66-original/drivers/ide/ide-disk.c linux-2.5.66-04/drivers/ide/ide-disk.c
> --- linux-2.5.66-original/drivers/ide/ide-disk.c	2003-03-26 08:56:49.000000000 +1200
> +++ linux-2.5.66-04/drivers/ide/ide-disk.c	2003-04-04 20:22:09.000000000 +1200
> Please submitt this to alan, as soon as possible.

Okay.

> 
> @@ -1804,7 +1801,8 @@
>  	if ((!drive->head || drive->head > 16) && !drive->select.b.lba) {
>  		printk(KERN_ERR "%s: INVALID GEOMETRY: %d PHYSICAL HEADS?\n",
>  			drive->name, drive->head);
> -		if ((drive->id->cfs_enable_2 & 0x3000) && drive->wcache)
> +		if (((drive->id->cfs_enable_2 & 0x3000) && drive->wcache) ||
> +		    ((drive->id->command_set_1 & 0x20) && drive->id->cfs_enable_1 & 0x20))
>  			if (do_idedisk_flushcache(drive))
>  				printk (KERN_INFO "%s: Write Cache FAILED Flushing!\n",
>  					drive->name);
> 
> Is this swsusp related?

Yes. Under 2.4, some people found that if the writeback cache isn't
flushed before we powerdown, the image isn't completely saved. This was
the fix (the first test is the original one, the second is based upon
hdparm's tests for writeback cache).

> diff -ruN linux-2.5.66-original/include/linux/pagemap.h linux-2.5.66-04/include/linux/pagemap.h
> --- linux-2.5.66-original/include/linux/pagemap.h	2003-03-26 08:57:06.000000000 +1200
> +++ linux-2.5.66-04/include/linux/pagemap.h	2003-03-26 09:00:29.000000000 +1200
> Do not use #ifdefs around includes.

Okay.

> diff -ruN linux-2.5.66-original/include/linux/suspend-debug.h linux-2.5.66-04/include/linux/suspend-debug.h
> --- linux-2.5.66-original/include/linux/suspend-debug.h	1970-01-01 12:00:00.000000000 +1200
> +++ linux-2.5.66-04/include/linux/suspend-debug.h	2003-04-04 19:37:44.000000000 +1200
> This is ugly. Are you trying to make it hard to read on purpose?

Nope. Just cut and paste from what was already in. I didn't like it much
either, but haven't changed it (this isn't _all_ my work).

> diff -ruN linux-2.5.66-original/include/linux/sysctl.h linux-2.5.66-04/include/linux/sysctl.h
> --- linux-2.5.66-original/include/linux/sysctl.h	2003-03-01 15:10:38.000000000 +1300
> +++ linux-2.5.66-04/include/linux/sysctl.h	2003-03-26 09:00:29.000000000 +1200
> Is sysctl being used besides debugging?

Yes. The main non-debugging thing that occurs to me right now is that
you can specify the image size you would like swsusp to aim for.

>  
>  unsigned char software_suspend_enabled = 0;
> +unsigned int suspend_task = 0;
> +/*
> + * Poll the swsusp state every second
> + */
> +#define SWSUSP_CHECK_TIMEOUT	(HZ)
> 
> What is this?

Used in the 2.4 version for kswsusp daemon. It provides another way to
start the process. I agree with you that we probably don't want to
implement this in 2.5 - I just forgot about removing it (I'm not
claiming that this code is perfectly cleaned up!)

> Please kill 2.4 stuff from 2.5 patch.

Yes, I will.

> -	if (mode == MARK_SWAP_RESUME) {
> -	  	if (!memcmp("S1",cur->swh.magic.magic,2))
> -		  	memcpy(cur->swh.magic.magic,"SWAP-SPACE",10);
> -		else if (!memcmp("S2",cur->swh.magic.magic,2))
> -			memcpy(cur->swh.magic.magic,"SWAPSPACE2",10);
> -		else printk("%sUnable to find suspended-data signature (%.10s - misspelled?\n", 
> -		      	name_resume, cur->swh.magic.magic);
> -	} else {
> -	  	if ((!memcmp("SWAP-SPACE",cur->swh.magic.magic,10)))
> -		  	memcpy(cur->swh.magic.magic,"S1SUSP....",10);
> -		else if ((!memcmp("SWAPSPACE2",cur->swh.magic.magic,10)))
> -			memcpy(cur->swh.magic.magic,"S2SUSP....",10);
> -		else panic("\nSwapspace is not swapspace (%.10s)\n", cur->swh.magic.magic);
> -		cur->link.next = prev; /* prev is the first/last swap page of the resume area */
> -		/* link.next lies *no more* in last 4/8 bytes of magic */
> +	switch(mode) {
> +	case MARK_SWAP_RESUME:
> +		if (!memcmp("1R",cur.pointer->swh.magic.magic,2))
> +			memcpy(cur.pointer->swh.magic.magic,"SWAP-SPACE",10);
> +		else if (!memcmp("2R",cur.pointer->swh.magic.magic,2))
> +			memcpy(cur.pointer->swh.magic.magic,"SWAPSPACE2",10);
> +		else printk(name_resume "Unable to find suspended-data signature (%.10s - misspelled?\n", 
> +			    cur.pointer->swh.magic.magic);
> +		break;
> 
> I guess this is nicer than previous code, good.

This is Florent's, I believe. I actually think the header stuff is still
ugly, but haven't gotten around to cleaning it up yet.

Thanks for the suggestions on how to do a better job. I know I'm not
finished - I just wanted to get it tested to start with.

Regards,

Nigel

-- 
Nigel Cunningham
495 St Georges Road South, Hastings 4201, New Zealand

Be diligent to present yourself approved to God as a workman who does
not need to be ashamed, handling accurately the word of truth.
	-- 2 Timothy 2:14, NASB.

Re: New Software Suspend Patch for testing.

[Pavel Machek]

Hi!

> The changes from the last version are not huge; perhaps most significant
> is that the dynamically allocated bitmaps (in place of pageflags) are
> implemented. This won't mean anything to most people, but Andrew
> Morton,

@@ -536,6 +519,9 @@
 	struct page *page;
 	int i;
 	int cold;
+#if CONFIG_SOFTWARE_SUSPEND
+	static unsigned int loopcount;
+#endif	
 
 	if (wait)
 		might_sleep();
@@ -589,7 +575,23 @@
 
 	/* here we're in the low on memory slow path */
 
+#if CONFIG_SOFTWARE_SUSPEND
+	loopcount=0;
+#endif
 rebalance:
+#ifdef CONFIG_SOFTWARE_SUSPEND
+	if(gfp_mask & __GFP_FAST) {
+/* when using memeat, we ask for all pages that are really free.
+   5 calls to reschedule should be sufficient to recall all of them since
+   when a page can be found, it is after only one reschedule.
+   Actually I consider this as a bug of alloc_pages, since allocating a
+   page should not hang in an endless loop when it is clear that no
+   memory is available (cbd) */	  
+		loopcount++;
+		if(loopcount > 5)
+			return NULL;
+	}
+#endif
 	if ((current->flags & (PF_MEMALLOC | PF_MEMDIE)) && !in_interrupt()) {
 		/* go through the zonelist yet again, ignoring mins */
 		for (i = 0; zones[i] != NULL; i++) {

Again this is *really really* ugly. Why do you need it? 2.5. has
special function for freeing memory.

							Pavel
-- 
When do you have heart between your knees?

Re: New Software Suspend Patch for testing.

[Pavel Machek]

Hi!

> > @@ -1804,7 +1801,8 @@
> >  	if ((!drive->head || drive->head > 16) && !drive->select.b.lba) {
> >  		printk(KERN_ERR "%s: INVALID GEOMETRY: %d PHYSICAL HEADS?\n",
> >  			drive->name, drive->head);
> > -		if ((drive->id->cfs_enable_2 & 0x3000) && drive->wcache)
> > +		if (((drive->id->cfs_enable_2 & 0x3000) && drive->wcache) ||
> > +		    ((drive->id->command_set_1 & 0x20) && drive->id->cfs_enable_1 & 0x20))
> >  			if (do_idedisk_flushcache(drive))
> >  				printk (KERN_INFO "%s: Write Cache FAILED Flushing!\n",
> >  					drive->name);
> > 
> > Is this swsusp related?
> 
> Yes. Under 2.4, some people found that if the writeback cache isn't
> flushed before we powerdown, the image isn't completely saved. This was
> the fix (the first test is the original one, the second is based upon
> hdparm's tests for writeback cache).

But this looks like you are fixing generic bug, which could make
kernel do something very wrong even without swsusp, right? If so,
submit it to Alan, ASAP.

> > What is this?
> 
> Used in the 2.4 version for kswsusp daemon. It provides another way to
> start the process. I agree with you that we probably don't want to
> implement this in 2.5 - I just forgot about removing it (I'm not
> claiming that this code is perfectly cleaned up!)

Sorry, I figured out it might be easier if I give you at least some
comments before you start splitting patch up -- it should be easier
for you this way.

								Pavel

-- 
When do you have heart between your knees?

Re: New Software Suspend Patch for testing.

[Alan Cox]

On Sad, 2003-04-05 at 00:03, Pavel Machek wrote:
> > Yes. Under 2.4, some people found that if the writeback cache isn't
> > flushed before we powerdown, the image isn't completely saved. This was
> > the fix (the first test is the original one, the second is based upon
> > hdparm's tests for writeback cache).
> 
> But this looks like you are fixing generic bug, which could make
> kernel do something very wrong even without swsusp, right? If so,
> submit it to Alan, ASAP.

I'll grab the older ATA spec and take a look. The other changes are
broken, but this one looks quite beliavable

Re: New Software Suspend Patch for testing.

[Patrick Mochel]

> Brief howto:
> (Assuming you've applied the patch against 2.5.66 - other versions may
> work too).
> 1. Patch, compile as normal.
> 2. Ensure your kernel commandline includes resume=/dev/hdaX where X is a
> swap partition you will be using. More than one swap partition can be
> used. This just specifies which will have its header used to indicate
> that the system is suspended and where to find the data.
> 3. If you want debugging info: echo 1 20 15 31 > /proc/sys/kernel/swsusp
> (See line 201ff of kernel/suspend.c for what the numbers mean).
> 4. echo 4 > /proc/acpi/sleep to start the process. 
> The system should save the image and powerdown.
> If you choose the debugging info, you'll have to press SHIFT at the end
> of each step.
> If you press SHIFT at other times, you can toggle this pausing on and
> off.
> Whether you selected the debugging info or not, you can press ALT to
> cancel the process.
> 5. To resume, reboot with the same kernel. The image should be detected
> and reloaded without you needing to do anything special. Pausing or not
> is state-persistent from when you suspended.
> 6. You should (DV) get your system back to where it was when you started
> the process.

I have a (fairly large) request: 

Would you mind updating Documentation/swsusp.txt (and moving it to 
Documentation/power/swsusp.txt)? 

This will accomplish a few things:

- Make the documentation readable.

- Explain exactly how swsusp works internally.

- Explain how to use it.

- Explain what the current dependencies and bugs are. 

- Explain what the cryptic debug values above are. 


Frankly, the code is a mess and difficult to follow. It's also poorly
commented. I spent the time about a month ago unwinding and deciphering 
it. Unfortunately, my work conflicts heavily with the work that you're 
doing, and the lack of documentation describing the intent of the code 
makes it difficult to make sane judgements of it. 

Thanks,


	-pat

Re: New Software Suspend Patch for testing.

[Nigel Cunningham]

Hi.

On Wed, 2003-04-16 at 08:06, Patrick Mochel wrote:
> I have a (fairly large) request: 
> 
> Would you mind updating Documentation/swsusp.txt (and moving it to 
> Documentation/power/swsusp.txt)? 

Okay. I've been meaning to update it.
> 
> Frankly, the code is a mess and difficult to follow. It's also poorly
> commented. I spent the time about a month ago unwinding and deciphering 
> it. Unfortunately, my work conflicts heavily with the work that you're 
> doing, and the lack of documentation describing the intent of the code 
> makes it difficult to make sane judgements of it. 

One more problem is that you're also trying to follow a moving target
:>. In response to Pavel's last request, I'm preparing to work toward
using a linked list for the meta information (to allow unlimited image
size), and at the request of others, compressing (zlib) the saved pages
to make the image faster to save/load.

As to the layout and documentation, I hate them too. I had to keep
things as similar as possible to existing code to make merging easier,
but now that I'm merged with the 2.4 code, perhaps I can do some
cleanups.

Regards,

Nigel

Re: New Software Suspend Patch for testing.

[Patrick Mochel]

> One more problem is that you're also trying to follow a moving target
> :>. In response to Pavel's last request, I'm preparing to work toward
> using a linked list for the meta information (to allow unlimited image
> size), and at the request of others, compressing (zlib) the saved pages
> to make the image faster to save/load.

The issue is not following a moving target. It's reading poorly maintained 
and documented code. 

If I were you, I would work first at cleaning up the code, then work on 
adding features to it. There were many opportunities I found for 
improvement when reworking the code, plus it became orders of magnitude 
easier to follow. 


	-pat