[PATCH][3/3] mm: swsusp post resume aggressive swap prefetch

Con Kolivas

author

2006-03-19 15:34 UTC #0 lore raw

PATCH 3/3 mm: swsusp post resume aggressive swap prefetch

Swsusp reclaims a lot of memory during the suspend cycle and can benefit from the aggressive_swap_prefetch mode immediately upon resuming.

Signed-off-by Con Kolivas <[email protected]>

---
 kernel/power/swsusp.c |    3 +++
 1 files changed, 3 insertions(+)

kernel/power/swsusp.c +3 -0

Index: linux-2.6.16-rc6-mm2/kernel/power/swsusp.c
===================================================================
@@ -49,6 +49,7 @@
 #include <linux/bootmem.h>
 #include <linux/syscalls.h>
 #include <linux/highmem.h>
+#include <linux/swap-prefetch.h>
 
 #include "power.h"

@@ -239,6 +240,8 @@ Restore_highmem:
 	device_power_up();
 Enable_irqs:
 	local_irq_enable();
+	if (!in_suspend)
+		aggressive_swap_prefetch();
 	return error;
 }

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Rafael J. Wysocki

2006-03-20 21:47 UTC #1 lore raw

Hi,

Quoted 3 lines · Con Kolivas

Swsusp reclaims a lot of memory during the suspend cycle and can benefit from the aggressive_swap_prefetch mode immediately upon resuming.

It slows down the resume on my box way too much. Last time it took 10x more time than actually reading the image.

I think the problem is for the userland suspend (which I use) it's done too early, when the image pages are still in the swap, so they are taken into consideration by the aggressive prefetch. If that really is the case, the solution would be to trigger the aggressive prefetch from the userland, if needed, after the image pages have been released.

Greetings,
Rafael

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

[email protected]

author

2006-03-20 21:25 UTC #2 lore raw

Quoting "Rafael J. Wysocki" <[email protected]>:

Quoted 10 lines

Hi,
Quoted 3 lines · Con Kolivas
Swsusp reclaims a lot of memory during the suspend cycle and can benefit from the aggressive_swap_prefetch mode immediately upon resuming.
It slows down the resume on my box way too much. Last time it took 10x more time than actually reading the image.

Quoted 9 lines

I think the problem is for the userland suspend (which I use) it's done too early,
when the image pages are still in the swap, so they are taken into consideration
by the aggressive prefetch. If that really is the case, the solution would be to
trigger the aggressive prefetch from the userland, if needed, after the image
pages have been released.

I assume this is unique to the userland resume as the in-kernel resume is not slowed down? If so, is there a way to differentiate the two so we only aggressively prefetch on kernel resume - is that what you meant by doing it in the other file?

Cheers,
Con

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Rafael J. Wysocki

2006-03-20 23:22 UTC #3 lore raw

Quoted 22 lines · On Monday 20 March 2006 22:25, [email protected] wrote:

Quoting "Rafael J. Wysocki" <[email protected]>:
Quoted 10 lines
Hi,
Quoted 3 lines · Con Kolivas
Swsusp reclaims a lot of memory during the suspend cycle and can benefit from the aggressive_swap_prefetch mode immediately upon resuming.
It slows down the resume on my box way too much. Last time it took 10x more time than actually reading the image.
Quoted 9 lines
I think the problem is for the userland suspend (which I use) it's done too early,
when the image pages are still in the swap, so they are taken into consideration
by the aggressive prefetch. If that really is the case, the solution would be to
trigger the aggressive prefetch from the userland, if needed, after the image
pages have been released.

Quoted 2 lines

I assume this is unique to the userland resume as the in-kernel resume is not slowed down?

Sorry, I was wrong. After resume the image pages in the swap are visible as free, because we allocate them after we have created the image (ie. the image contains the system state in which these pages are free).

Well, this means I really don't know what happens and what causes the slowdown. It certainly is related to the aggressive prefetch hook in swsusp_suspend(). [It seems to search the whole swap, but it doesn't actually prefetch anything. Strange.]

Quoted 3 lines

If so, is there a way to differentiate the two so we only aggressively prefetch on kernel resume - is that what you meant by doing it in the other file?

Basically, yes. swsusp.c and snapshot.c contain common functions, disk.c and swap.c contain the code used by the built-in swsusp only, and user.c contains the userland interface. If you want something to be run by the built-in swsusp only, place it in disk.c.

Still in this particular case it won't matter, I'm afraid.

Greetings,
Rafael

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

[email protected]

author

2006-03-21 00:44 UTC #4 lore raw

Quoting "Rafael J. Wysocki" <[email protected]>:

Quoted 9 lines

Sorry, I was wrong. After resume the image pages in the swap are visible as free, because we allocate them after we have created the image (ie. the image
contains the system state in which these pages are free).
Well, this means I really don't know what happens and what causes the slowdown. It certainly is related to the aggressive prefetch hook in swsusp_suspend(). [It seems to search the whole swap, but it doesn't actually prefetch anything. Strange.]

Are you looking at swap still in use? Swap prefetch keeps a copy of prefetched pages on backing store as well as in ram so the swap space will not be freed on prefetching.

Quoted 10 lines

Quoted 3 lines
If so, is there a way to differentiate the two so we only aggressively prefetch on kernel resume - is that what you meant by doing it in the other file?
Basically, yes. swsusp.c and snapshot.c contain common functions, disk.c and swap.c contain the code used by the built-in swsusp only, and user.c contains the userland interface. If you want something to be run by the built-in swsusp only, place it in disk.c.
Still in this particular case it won't matter, I'm afraid.

I don't understand what you mean by it won't matter?

Cheers,
Con

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Rafael J. Wysocki

2006-03-21 18:11 UTC #5 lore raw

Quoted 6 lines · On Tuesday 21 March 2006 01:44, [email protected] wrote:

Quoting "Rafael J. Wysocki" <[email protected]>:
Quoted 5 lines
Sorry, I was wrong. After resume the image pages in the swap are visible as free, because we allocate them after we have created the image (ie. the image
contains the system state in which these pages are free).

Quoted 8 lines

Quoted 4 lines
Well, this means I really don't know what happens and what causes the slowdown. It certainly is related to the aggressive prefetch hook in swsusp_suspend(). [It seems to search the whole swap, but it doesn't actually prefetch anything. Strange.]
Are you looking at swap still in use? Swap prefetch keeps a copy of prefetched pages on backing store as well as in ram so the swap space will not be freed on prefetching.

It looks like I have to debug it a bit more. Unfortunately I've been having a lot of work to do recently, so it'll take some time.

Quoted 4 lines

Quoted 4 lines
Quoted 3 lines
If so, is there a way to differentiate the two so we only aggressively prefetch on kernel resume - is that what you meant by doing it in the other file?

Quoted 8 lines

Quoted 6 lines
Basically, yes. swsusp.c and snapshot.c contain common functions, disk.c and swap.c contain the code used by the built-in swsusp only, and user.c contains the userland interface. If you want something to be run by the built-in swsusp only, place it in disk.c.
Still in this particular case it won't matter, I'm afraid.
I don't understand what you mean by it won't matter?

Well, sorry. Of course it will matter. What I wanted to say is that in this case tbe built-in swsusp would be affected as well as the userland suspend, because the hook was in a function used by both.

Greetings,
Rafael

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Con Kolivas

author

2006-03-22 06:11 UTC #6 lore raw

Quoted 7 lines · Rafael J. Wysocki

Quoted 3 lines · On Tuesday 21 March 2006 01:44, [email protected] wrote:
Are you looking at swap still in use? Swap prefetch keeps a copy of prefetched pages on backing store as well as in ram so the swap space will not be freed on prefetching.
It looks like I have to debug it a bit more. Unfortunately I've been having a lot of work to do recently, so it'll take some time.

No rush whatsoever! I'd just like to help on this.

Quoted 5 lines

Quoted 1 line
I don't understand what you mean by it won't matter?
Well, sorry. Of course it will matter. What I wanted to say is that in this case tbe built-in swsusp would be affected as well as the userland suspend, because the hook was in a function used by both.

Understood. So I'll modify it to hook into only built-in swsusp restore when I get a chance.

Cheers,
Con

Con Kolivas

author

2006-03-24 05:00 UTC #7 lore raw

PATCH 1/1 swswsup: return correct load_image error

In reply to Rafael J. Wysocki

Quoted 4 lines · Rafael J. Wysocki

Basically, yes. swsusp.c and snapshot.c contain common functions, disk.c and swap.c contain the code used by the built-in swsusp only, and user.c contains the userland interface. If you want something to be run by the built-in swsusp only, place it in disk.c.

Ok. A quick look at the code in swap.c makes me wonder if we need this patch.

Rafael?

Cheers,
Con
---
If there's an error in load_image() we should return that without checking snapshot_image_loaded.

Signed-off-by Con Kolivas <[email protected]>

---
 kernel/power/swap.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

kernel/power/swap.c +4 -3

Index: linux-2.6.16-mm1/kernel/power/swap.c
===================================================================
@@ -454,10 +454,11 @@ static int load_image(struct swap_map_ha
 			nr_pages++;
 		}
 	} while (ret > 0);
-	if (!error)
+	if (!error) {
 		printk("\b\b\b\bdone\n");
-	if (!snapshot_image_loaded(snapshot))
-		error = -ENODATA;
+		if (!snapshot_image_loaded(snapshot))
+			error = -ENODATA;
+	}
 	return error;
 }

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Con Kolivas

author

2006-03-24 05:17 UTC #8 lore raw

Re: [PATCH] swswsup: return correct load_image error

Quoted 5 lines · Rafael J. Wysocki

Quoted 4 lines
Basically, yes. swsusp.c and snapshot.c contain common functions, disk.c and swap.c contain the code used by the built-in swsusp only, and user.c contains the userland interface. If you want something to be run by the built-in swsusp only, place it in disk.c.

Would this patch suffice?

Cheers,
Con
---
Swsusp reclaims a lot of memory during the suspend cycle and can benefit from the aggressive_swap_prefetch mode immediately upon resuming.

Signed-off-by Con Kolivas <[email protected]>

---
 kernel/power/disk.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletion(-)

kernel/power/disk.c +4 -1

Index: linux-2.6.16-mm1/kernel/power/disk.c
===================================================================
@@ -19,6 +19,7 @@
 #include <linux/fs.h>
 #include <linux/mount.h>
 #include <linux/pm.h>
+#include <linux/swap-prefetch.h>
 
 #include "power.h"

@@ -138,8 +139,10 @@ int pm_suspend_disk(void)
 			unprepare_processes();
 			return error;
 		}
-	} else
+	} else {
 		pr_debug("PM: Image restored successfully.\n");
+		aggressive_swap_prefetch();
+	}
 
 	swsusp_free();
  Done:

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Rafael J. Wysocki

2006-03-24 15:05 UTC #9 lore raw

Re: [PATCH] swswsup: return correct load_image error

Quoted 7 lines · Con Kolivas

Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines
Basically, yes. swsusp.c and snapshot.c contain common functions, disk.c and swap.c contain the code used by the built-in swsusp only, and user.c contains the userland interface. If you want something to be run by the built-in swsusp only, place it in disk.c.
Would this patch suffice?

I'd change one thing (please see below).

Quoted 22 lines

---
Swsusp reclaims a lot of memory during the suspend cycle and can benefit from the aggressive_swap_prefetch mode immediately upon resuming.
Signed-off-by Con Kolivas <[email protected]>
---
 kernel/power/disk.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletion(-)
Index: linux-2.6.16-mm1/kernel/power/disk.c
===================================================================
kernel/power/disk.c +1 -0
@@ -19,6 +19,7 @@
 #include <linux/fs.h>
 #include <linux/mount.h>
 #include <linux/pm.h>
+#include <linux/swap-prefetch.h>
 
 #include "power.h"
 

kernel/power/disk.c int pm_suspend_disk(void) · line 147

Quoted 10 lines

kernel/power/disk.c +3 -1

@@ -138,8 +139,10 @@ int pm_suspend_disk(void)
 			unprepare_processes();
 			return error;
 		}
-	} else
+	} else {
 		pr_debug("PM: Image restored successfully.\n");
+		aggressive_swap_prefetch();
+	}
 
 	swsusp_free();

I'd put aggressive_swap_prefetch() here. Before swsusp_free() there may be no room for the prefetched pages. [I should have noticed it earlier. _This_ must have been the problem last time I tested it.]

kernel/power/disk.c int pm_suspend_disk(void) · line 148

Quoted 3 lines

kernel/power/disk.c +0 -0
  Done:

Greetings,
Rafael

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Rafael J. Wysocki

2006-03-24 14:51 UTC #10 lore raw

Re: [PATCH] swswsup: return correct load_image error

In reply to Con Kolivas

Quoted 9 lines · Con Kolivas

Quoted 4 lines · Rafael J. Wysocki
Basically, yes. swsusp.c and snapshot.c contain common functions, disk.c and swap.c contain the code used by the built-in swsusp only, and user.c contains the userland interface. If you want something to be run by the built-in swsusp only, place it in disk.c.
Ok. A quick look at the code in swap.c makes me wonder if we need this patch.
Rafael?

Yes, we do. Thanks.

Andrew, could you please pick it up?

Rafael

Quoted 21 lines

---
If there's an error in load_image() we should return that without checking snapshot_image_loaded.
Signed-off-by Con Kolivas <[email protected]>
---
 kernel/power/swap.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
Index: linux-2.6.16-mm1/kernel/power/swap.c
===================================================================
kernel/power/swap.c +0 -1
@@ -454,10 +454,11 @@ static int load_image(struct swap_map_ha
 			nr_pages++;
 		}
 	} while (ret > 0);
-	if (!error)

kernel/power/swap.c static int load_image(struct swap_map_ha · line 464

Quoted 10 lines

kernel/power/swap.c +4 -2

+	if (!error) {
 		printk("\b\b\b\bdone\n");
-	if (!snapshot_image_loaded(snapshot))
-		error = -ENODATA;
+		if (!snapshot_image_loaded(snapshot))
+			error = -ENODATA;
+	}
 	return error;
 }

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>