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(+)
Index: linux-2.6.16-rc6-mm2/kernel/power/swsusp.c
===================================================================
--- linux-2.6.16-rc6-mm2.orig/kernel/power/swsusp.c 2006-03-20 02:15:47.000000000 +1100
+++ linux-2.6.16-rc6-mm2/kernel/power/swsusp.c 2006-03-20 02:20:35.000000000 +1100
@@ -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;
}
Hi,
On Sunday 19 March 2006 16:34, Con Kolivas wrote:
>
> 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
Quoting "Rafael J. Wysocki" <[email protected]>:
> Hi,
>
> On Sunday 19 March 2006 16:34, Con Kolivas wrote:
> >
> > 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.
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
On Monday 20 March 2006 22:25, [email protected] wrote:
> Quoting "Rafael J. Wysocki" <[email protected]>:
>
> > Hi,
> >
> > On Sunday 19 March 2006 16:34, Con Kolivas wrote:
> > >
> > > 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.
>
> 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.]
> 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
Quoting "Rafael J. Wysocki" <[email protected]>:
> 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.
> > 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
On Tuesday 21 March 2006 01:44, [email protected] wrote:
> Quoting "Rafael J. Wysocki" <[email protected]>:
> > 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.
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.
> > > 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?
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
On Wed, 22 Mar 2006 05:11 am, Rafael J. Wysocki wrote:
> 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.
> > 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
On Tuesday 21 March 2006 10:22, Rafael J. Wysocki wrote:
> 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(-)
Index: linux-2.6.16-mm1/kernel/power/swap.c
===================================================================
--- linux-2.6.16-mm1.orig/kernel/power/swap.c 2006-03-24 15:04:13.000000000 +1100
+++ linux-2.6.16-mm1/kernel/power/swap.c 2006-03-24 15:55:30.000000000 +1100
@@ -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;
}
> On Tuesday 21 March 2006 10:22, Rafael J. Wysocki wrote:
> > 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(-)
Index: linux-2.6.16-mm1/kernel/power/disk.c
===================================================================
--- linux-2.6.16-mm1.orig/kernel/power/disk.c 2006-03-24 15:48:14.000000000 +1100
+++ linux-2.6.16-mm1/kernel/power/disk.c 2006-03-24 16:15:05.000000000 +1100
@@ -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:
On Friday 24 March 2006 06:00, Con Kolivas wrote:
> On Tuesday 21 March 2006 10:22, Rafael J. Wysocki wrote:
> > 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
> ---
> 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
> ===================================================================
> --- linux-2.6.16-mm1.orig/kernel/power/swap.c 2006-03-24 15:04:13.000000000 +1100
> +++ linux-2.6.16-mm1/kernel/power/swap.c 2006-03-24 15:55:30.000000000 +1100
> @@ -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;
> }
>
>
>
On Friday 24 March 2006 06:17, Con Kolivas wrote:
> > On Tuesday 21 March 2006 10:22, Rafael J. Wysocki wrote:
> > > 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).
> ---
> 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
> ===================================================================
> --- linux-2.6.16-mm1.orig/kernel/power/disk.c 2006-03-24 15:48:14.000000000 +1100
> +++ linux-2.6.16-mm1/kernel/power/disk.c 2006-03-24 16:15:05.000000000 +1100
> @@ -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();
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.]
> Done:
>
>
Greetings,
Rafael