[Patch] s2disk / hibernate: Bug 47931 / Bug 83821

[Patch] s2disk / hibernate: Bug 47931 / Bug 83821

[Rainer Fiebig]

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:


> 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:


> 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:


> 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



Signed-off-by: J. Rainer Fiebig <j.r.f@t-online.de>

--- 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
--