Skip to contentSkip to conversation

s2disk / hibernate: Bug 47931 / Bug 83821

PATCH New Rainer Fiebig 1 participant 1 message Nov 8, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 s2disk / hibernate: Bug 47931 / Bug 83821

Hi!

I think I’ve found a solution for Bug 47931 and Bug 83821.
It makes s2disk almost 100% reliable on my x86_64-system.
Only one line of code needs to be modified and placed higher up in the code.

Background
s2disk sometimes does not succeed at the first attempt but needs two or even more attempts (Bug 47931, https://bugzilla.kernel.org/show_bug.cgi?id=47931).
This happens if more than 50% of memory is used by applications and buffers/cache.
As buffers/cache can take up a lot of memory quickly, Bug 47931 may occur even if only a few applications are running.

Bug 83821 (https://bugzilla.kernel.org/show_bug.cgi?id=83821.) causes a similar behavior if s2disk is executed immediately after a memory-intensive application was closed or if it is executed more than 2 times in a row after a fresh system start.
That means it can occur even if less than 50% of memory is used.

For the user this is quite bewildering and time-consuming. It makes s2disk unreliable and basically unusable.

I found out that both bugs can be prevented by issuing "drop_caches=1" before s2disk.

In function "int hibernate_preallocate_memory(void)" in kernel/power/snapshot.c "shrink_all_memory()" is called to free memory. Unfortunately, it is placed too far down in the code to take effect. This is why we run into Bug 47931.
If memory-usage is low, it may not be called at all. This probably causes Bug 83821.

Solution
The solution is simple: free memory early and do it with every s2disk.

Only one line of code needs to be modified and and put before "saveable = count_data_pages();":

shrink_all_memory(count_data_pages() / 2);.

Freeing 50% of used data-pages seems sufficient without applying too much pressure which could have a negative effect on resumes and initial system- responsiveness.

The "shrink_all_memory(saveable - size);"-statement further down in the code must be deleted.

Results
This modification has made s2disk almost 100% reliable on my system.

Testing was done with kernels 3.16.2 and 3.14.22 on an x86_64-system with 12 GB RAM.
I have not tested it on a 32-bit-system.

Here's an example of a case where s2disk now works but surely would have failed before:

Quoted 1 line

free

total used free shared buffers cached
Mem: 11973244 11734724 238520 0 6107076 1995700
-/+ buffers/cache: 3631948 8341296
Swap: 10714108 0 10714108

I have found only two conditions where s2disk still fails:

1.) If "used - buffers/cache" is > 50% of memory.
In this case s2disk stops and returns to the desktop.
That's not a big problem and is to be expected due to its current limit of 50% of RAM.
Example:

Quoted 1 line

free

total used free shared buffers cached

Mem:      11973244   10786352    1186892          0    1436860    2341156
-/+ buffers/cache:    7008336    4964908
Swap:     10714108    1003124    9710984

2.) If "used - buffers/cache" is very close to 50% of memory. Here one may run into Bug 75101
(https://bugzilla.kernel.org/show_bug.cgi?id=75101). Which is bad, because it requires a hard-reset.
I have encountered this only once when testing the limits.
But it may be a problem on systems with sparse memory.
Example:

Quoted 1 line

free

total used free shared buffers cached

Mem:      11997572    6490540    5507032          0      32600     577632
-/+ buffers/cache:    5880308    6117264
Swap:     10714108     739580    9974528

Other than 2.) I have not encountered any major negative side-effects. I use s2both and if memory usage was high before s2disk, resumes may take a little longer than after a plain suspend to RAM.
In those cases the system also may be a little less responsive initially. For me this is negligible and clearly outweighed by the benefit of almost total reliability.
In addition, the whole procedure is at least not slower than before.

More test-results in the thread for Bug 47931 (posted by "Jay").

I am not sure whether my post or the patch conform to conventions here but in view of the potential improvement in s2disk I thought I'd give it a try.

Reviews/comments/advice welcome.

Rainer Fiebig

--- linux-3.16.2/kernel/power/snapshot.orig 2014-08-04 00:25:02.000000000
+0200
+++ linux-3.16.2/kernel/power/snapshot.c 2014-10-28 20:49:32.000000000
+0100
@@ -1332,6 +1332,13 @@ int hibernate_preallocate_memory(void)

        /* Count the number of saveable data pages. */
        save_highmem = count_highmem_pages();
+
+       /* 
+        * Free memory. About 50% of used data pages seems sufficient 
+        * without exerting too much pressure. This is done on every s2disk.
+        */
+       shrink_all_memory(count_data_pages() / 2);
+
        saveable = count_data_pages();
 
        /*
@@ -1389,14 +1396,6 @@ int hibernate_preallocate_memory(void)
                size = min_t(unsigned long, pages, max_size);
 
        /*
-        * Let the memory management subsystem know that we're going to need a
-        * large number of page frames to allocate and make it free some 
memory.
-        * NOTE: If this is not done, performance will be hurt badly in some
-        * test cases.
-        */

- shrink_all_memory(saveable - size);
-
- /*
* The number of saveable pages in memory was too high, so apply some
* pressure to decrease it. First, make room for the largest possible
* image and fail if that doesn't work. Next, try to decrease the
size
--