[PATCH v3 0/2] clarify limitations of hibernation

[PATCH v3 0/2] clarify limitations of hibernation

[Luigi Semenzato]

These patches aim to make it clearer under which circumstances
hibernation will function as expected.  They include one documentation
change, and one change which logs more information on failure to
enter hibernation.

---- comments on patch versions ----

As suggested, v3 makes the log messages more consistent, by including the term
"hibernation" in most of them (if not all).  (I think this was worth it, also
because this code doesn't change often, but we're quickly entering the region
of diminishing returns.)

v2 uses better terminology in the Documentation, and removes a suggestion
for a workaround.

-----------------------------------

Luigi Semenzato (2):
  Documentation: clarify limitations of hibernation
  pm: add more logging on hibernation failure

 Documentation/admin-guide/pm/sleep-states.rst | 12 +++++++++-
 kernel/power/hibernate.c                      | 23 +++++++++---------
 kernel/power/snapshot.c                       | 24 ++++++++++++-------
 3 files changed, 38 insertions(+), 21 deletions(-)

-- 
2.24.1.735.g03f4e72817-goog

[PATCH v3 1/2] Documentation: clarify limitations of hibernation

[Luigi Semenzato]

Entering hibernation (suspend-to-disk) will fail if the kernel
cannot allocate enough memory to create a snapshot of all pages
in use; i.e., if memory in use is over 1/2 of total RAM.  This
patch makes this limitation clearer in the documentation.  Without
it, users may assume that hibernation can replace suspend-to-RAM
when in fact its functionality is more limited.

Signed-off-by: Luigi Semenzato <semenzato@google.com>
---
 Documentation/admin-guide/pm/sleep-states.rst | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/pm/sleep-states.rst b/Documentation/admin-guide/pm/sleep-states.rst
index cd3a28cb81f4..a2d5632b7856 100644
--- a/Documentation/admin-guide/pm/sleep-states.rst
+++ b/Documentation/admin-guide/pm/sleep-states.rst
@@ -112,7 +112,9 @@ Hibernation
 This state (also referred to as Suspend-to-Disk or STD) offers the greatest
 energy savings and can be used even in the absence of low-level platform support
 for system suspend.  However, it requires some low-level code for resuming the
-system to be present for the underlying CPU architecture.
+system to be present for the underlying CPU architecture.  Additionally, the
+current implementation can enter the hibernation state only when memory
+usage is sufficiently low (see "Limitations" below).
 
 Hibernation is significantly different from any of the system suspend variants.
 It takes three system state changes to put it into hibernation and two system
@@ -149,6 +151,14 @@ Hibernation is supported if the :c:macro:`CONFIG_HIBERNATION` kernel
 configuration option is set.  However, this option can only be set if support
 for the given CPU architecture includes the low-level code for system resume.
 
+Limitations of Hibernation
+==========================
+
+When entering hibernation, the kernel tries to allocate a chunk of memory large
+enough to contain a copy of all pages in use, to use it for the system
+snapshot.  If the allocation fails, the system cannot hibernate and the
+operation fails with ENOMEM.  This will happen, for instance, when the total
+amount of anonymous pages (process data) exceeds 1/2 of total RAM.
 
 Basic ``sysfs`` Interfaces for System Suspend and Hibernation
 =============================================================
-- 
2.24.1.735.g03f4e72817-goog

[PATCH v3 2/2] pm: add more logging on hibernation failure

[Luigi Semenzato]

Hibernation fails when the kernel cannot allocate enough memory
to copy all pages in use.  This patch ensures that the failure
reason is clearly logged, and clearly attributable to the
hibernation module.

Signed-off-by: Luigi Semenzato <semenzato@google.com>
---
 kernel/power/hibernate.c | 23 ++++++++++++-----------
 kernel/power/snapshot.c  | 24 +++++++++++++++---------
 2 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 3c0a5a8170b0..6dbeedb7354c 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -9,7 +9,7 @@
  * Copyright (C) 2012 Bojan Smojver <bojan@rexursive.com>
  */
 
-#define pr_fmt(fmt) "PM: " fmt
+#define pr_fmt(fmt) "PM: hibernation: " fmt
 
 #include <linux/export.h>
 #include <linux/suspend.h>
@@ -106,7 +106,7 @@ EXPORT_SYMBOL(system_entering_hibernation);
 #ifdef CONFIG_PM_DEBUG
 static void hibernation_debug_sleep(void)
 {
-	pr_info("hibernation debug: Waiting for 5 seconds.\n");
+	pr_info("debug: Waiting for 5 seconds.\n");
 	mdelay(5000);
 }
 
@@ -277,7 +277,7 @@ static int create_image(int platform_mode)
 
 	error = dpm_suspend_end(PMSG_FREEZE);
 	if (error) {
-		pr_err("Some devices failed to power down, aborting hibernation\n");
+		pr_err("Some devices failed to power down, aborting\n");
 		return error;
 	}
 
@@ -295,7 +295,7 @@ static int create_image(int platform_mode)
 
 	error = syscore_suspend();
 	if (error) {
-		pr_err("Some system devices failed to power down, aborting hibernation\n");
+		pr_err("Some system devices failed to power down, aborting\n");
 		goto Enable_irqs;
 	}
 
@@ -310,7 +310,7 @@ static int create_image(int platform_mode)
 	restore_processor_state();
 	trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false);
 	if (error)
-		pr_err("Error %d creating hibernation image\n", error);
+		pr_err("Error %d creating image\n", error);
 
 	if (!in_suspend) {
 		events_check_enabled = false;
@@ -680,7 +680,7 @@ static int load_image_and_restore(void)
 	if (!error)
 		hibernation_restore(flags & SF_PLATFORM_MODE);
 
-	pr_err("Failed to load hibernation image, recovering.\n");
+	pr_err("Failed to load image, recovering.\n");
 	swsusp_free();
 	free_basic_memory_bitmaps();
  Unlock:
@@ -743,7 +743,7 @@ int hibernate(void)
 		else
 		        flags |= SF_CRC32_MODE;
 
-		pm_pr_dbg("Writing image.\n");
+		pm_pr_dbg("Writing hibernation image.\n");
 		error = swsusp_write(flags);
 		swsusp_free();
 		if (!error) {
@@ -755,7 +755,7 @@ int hibernate(void)
 		in_suspend = 0;
 		pm_restore_gfp_mask();
 	} else {
-		pm_pr_dbg("Image restored successfully.\n");
+		pm_pr_dbg("Hibernation image restored successfully.\n");
 	}
 
  Free_bitmaps:
@@ -894,7 +894,7 @@ static int software_resume(void)
 		goto Close_Finish;
 	}
 
-	pm_pr_dbg("Preparing processes for restore.\n");
+	pm_pr_dbg("Preparing processes for hibernation restore.\n");
 	error = freeze_processes();
 	if (error)
 		goto Close_Finish;
@@ -903,7 +903,7 @@ static int software_resume(void)
  Finish:
 	__pm_notifier_call_chain(PM_POST_RESTORE, nr_calls, NULL);
 	pm_restore_console();
-	pr_info("resume from hibernation failed (%d)\n", error);
+	pr_info("resume failed (%d)\n", error);
 	atomic_inc(&snapshot_device_available);
 	/* For success case, the suspend path will release the lock */
  Unlock:
@@ -1068,7 +1068,8 @@ static ssize_t resume_store(struct kobject *kobj, struct kobj_attribute *attr,
 	lock_system_sleep();
 	swsusp_resume_device = res;
 	unlock_system_sleep();
-	pm_pr_dbg("Configured resume from disk to %u\n", swsusp_resume_device);
+	pm_pr_dbg("Configured hibernation resume from disk to %u\n",
+		  swsusp_resume_device);
 	noresume = 0;
 	software_resume();
 	return n;
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 26b9168321e7..dcd1376e8fdf 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -8,7 +8,7 @@
  * Copyright (C) 2006 Rafael J. Wysocki <rjw@sisk.pl>
  */
 
-#define pr_fmt(fmt) "PM: " fmt
+#define pr_fmt(fmt) "PM: hibernation: " fmt
 
 #include <linux/version.h>
 #include <linux/module.h>
@@ -1705,16 +1705,20 @@ int hibernate_preallocate_memory(void)
 	ktime_t start, stop;
 	int error;
 
-	pr_info("Preallocating image memory... ");
+	pr_info("Preallocating image memory\n");
 	start = ktime_get();
 
 	error = memory_bm_create(&orig_bm, GFP_IMAGE, PG_ANY);
-	if (error)
+	if (error) {
+		pr_err("Cannot allocate original bitmap\n");
 		goto err_out;
+	}
 
 	error = memory_bm_create(&copy_bm, GFP_IMAGE, PG_ANY);
-	if (error)
+	if (error) {
+		pr_err("Cannot allocate copy bitmap\n");
 		goto err_out;
+	}
 
 	alloc_normal = 0;
 	alloc_highmem = 0;
@@ -1804,8 +1808,11 @@ int hibernate_preallocate_memory(void)
 		alloc -= pages;
 		pages += pages_highmem;
 		pages_highmem = preallocate_image_highmem(alloc);
-		if (pages_highmem < alloc)
+		if (pages_highmem < alloc) {
+			pr_err("Image allocation is %lu pages short\n",
+				alloc - pages_highmem);
 			goto err_out;
+		}
 		pages += pages_highmem;
 		/*
 		 * size is the desired number of saveable pages to leave in
@@ -1836,13 +1843,12 @@ int hibernate_preallocate_memory(void)
 
  out:
 	stop = ktime_get();
-	pr_cont("done (allocated %lu pages)\n", pages);
+	pr_info("Allocated %lu pages for shapshot\n", pages);
 	swsusp_show_speed(start, stop, pages, "Allocated");
 
 	return 0;
 
  err_out:
-	pr_cont("\n");
 	swsusp_free();
 	return -ENOMEM;
 }
@@ -1976,7 +1982,7 @@ asmlinkage __visible int swsusp_save(void)
 {
 	unsigned int nr_pages, nr_highmem;
 
-	pr_info("Creating hibernation image:\n");
+	pr_info("Creating image:\n");
 
 	drain_local_pages(NULL);
 	nr_pages = count_data_pages();
@@ -2010,7 +2016,7 @@ asmlinkage __visible int swsusp_save(void)
 	nr_copy_pages = nr_pages;
 	nr_meta_pages = DIV_ROUND_UP(nr_pages * sizeof(long), PAGE_SIZE);
 
-	pr_info("Hibernation image created (%d pages copied)\n", nr_pages);
+	pr_info("Image created (%d pages copied)\n", nr_pages);
 
 	return 0;
 }
-- 
2.24.1.735.g03f4e72817-goog

Re: [PATCH v3 1/2] Documentation: clarify limitations of hibernation

[Rafael J. Wysocki]

On Fri, Jan 3, 2020 at 12:19 AM Luigi Semenzato <semenzato@google.com> wrote:
>
> Entering hibernation (suspend-to-disk) will fail if the kernel
> cannot allocate enough memory to create a snapshot of all pages
> in use; i.e., if memory in use is over 1/2 of total RAM.  This
> patch makes this limitation clearer in the documentation.  Without
> it, users may assume that hibernation can replace suspend-to-RAM
> when in fact its functionality is more limited.
>
> Signed-off-by: Luigi Semenzato <semenzato@google.com>
> ---
>  Documentation/admin-guide/pm/sleep-states.rst | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/pm/sleep-states.rst b/Documentation/admin-guide/pm/sleep-states.rst
> index cd3a28cb81f4..a2d5632b7856 100644
> --- a/Documentation/admin-guide/pm/sleep-states.rst
> +++ b/Documentation/admin-guide/pm/sleep-states.rst
> @@ -112,7 +112,9 @@ Hibernation
>  This state (also referred to as Suspend-to-Disk or STD) offers the greatest
>  energy savings and can be used even in the absence of low-level platform support
>  for system suspend.  However, it requires some low-level code for resuming the
> -system to be present for the underlying CPU architecture.
> +system to be present for the underlying CPU architecture.  Additionally, the
> +current implementation can enter the hibernation state only when memory
> +usage is sufficiently low (see "Limitations" below).

This really isn't about memory usage being "sufficiently low" (I told
you I could hibernate systems with almost 100% of RAM allocated before
hibernation), but about specific memory allocation patterns that may
prevent the hibernation code from being able to get enough memory on
demand.

So I would prefer the following alternative statement: "Additionally,
the hibernation state cannot be entered if the current memory usage
pattern of the system prevents the hibernation code from acquiring
enough memory (see "Limitations" below)."

>  Hibernation is significantly different from any of the system suspend variants.
>  It takes three system state changes to put it into hibernation and two system
> @@ -149,6 +151,14 @@ Hibernation is supported if the :c:macro:`CONFIG_HIBERNATION` kernel
>  configuration option is set.  However, this option can only be set if support
>  for the given CPU architecture includes the low-level code for system resume.
>
> +Limitations of Hibernation
> +==========================
> +
> +When entering hibernation, the kernel tries to allocate a chunk of memory large
> +enough to contain a copy of all pages in use, to use it for the system
> +snapshot.

This isn't precise enough, because "all pages in use" may be read as
"all pages of virtual memory in use" and "a chunk of memory" may be
misunderstood as "a contiguous region".

The following describes what the code really does more precisely IMO:

"When entering hibernation, the kernel tries to allocate enough memory
to store a copy of every physical page frame (in RAM) that is not
free, except for some special regions of physical memory explicitly
marked as "not to be saved". These allocations are made one page at a
time with the expectation that the memory management subsystem will
push out memory to the swap when it is not able to find a free
physical page frame. However, in some cases that expectation is not
met: for example when there is not enough swap space in the system or
when the total amount of anonymous pages (process data) exceeds 1/2 of
total RAM. In those cases the operation fails with ENOMEM."

>+  If the allocation fails, the system cannot hibernate and the

> +operation fails with ENOMEM.  This will happen, for instance, when the total
> +amount of anonymous pages (process data) exceeds 1/2 of total RAM.
>
>  Basic ``sysfs`` Interfaces for System Suspend and Hibernation
>  =============================================================
> --
> 2.24.1.735.g03f4e72817-goog
>

Re: [PATCH v3 2/2] pm: add more logging on hibernation failure

[Rafael J. Wysocki]

On Fri, Jan 3, 2020 at 12:19 AM Luigi Semenzato <semenzato@google.com> wrote:
>
> Hibernation fails when the kernel cannot allocate enough memory
> to copy all pages in use.  This patch ensures that the failure
> reason is clearly logged, and clearly attributable to the
> hibernation module.
>
> Signed-off-by: Luigi Semenzato <semenzato@google.com>

Applied as 5.6 material with some minor changes in the subject and
changelog, thanks!

> ---
>  kernel/power/hibernate.c | 23 ++++++++++++-----------
>  kernel/power/snapshot.c  | 24 +++++++++++++++---------
>  2 files changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 3c0a5a8170b0..6dbeedb7354c 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -9,7 +9,7 @@
>   * Copyright (C) 2012 Bojan Smojver <bojan@rexursive.com>
>   */
>
> -#define pr_fmt(fmt) "PM: " fmt
> +#define pr_fmt(fmt) "PM: hibernation: " fmt
>
>  #include <linux/export.h>
>  #include <linux/suspend.h>
> @@ -106,7 +106,7 @@ EXPORT_SYMBOL(system_entering_hibernation);
>  #ifdef CONFIG_PM_DEBUG
>  static void hibernation_debug_sleep(void)
>  {
> -       pr_info("hibernation debug: Waiting for 5 seconds.\n");
> +       pr_info("debug: Waiting for 5 seconds.\n");
>         mdelay(5000);
>  }
>
> @@ -277,7 +277,7 @@ static int create_image(int platform_mode)
>
>         error = dpm_suspend_end(PMSG_FREEZE);
>         if (error) {
> -               pr_err("Some devices failed to power down, aborting hibernation\n");
> +               pr_err("Some devices failed to power down, aborting\n");
>                 return error;
>         }
>
> @@ -295,7 +295,7 @@ static int create_image(int platform_mode)
>
>         error = syscore_suspend();
>         if (error) {
> -               pr_err("Some system devices failed to power down, aborting hibernation\n");
> +               pr_err("Some system devices failed to power down, aborting\n");
>                 goto Enable_irqs;
>         }
>
> @@ -310,7 +310,7 @@ static int create_image(int platform_mode)
>         restore_processor_state();
>         trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false);
>         if (error)
> -               pr_err("Error %d creating hibernation image\n", error);
> +               pr_err("Error %d creating image\n", error);
>
>         if (!in_suspend) {
>                 events_check_enabled = false;
> @@ -680,7 +680,7 @@ static int load_image_and_restore(void)
>         if (!error)
>                 hibernation_restore(flags & SF_PLATFORM_MODE);
>
> -       pr_err("Failed to load hibernation image, recovering.\n");
> +       pr_err("Failed to load image, recovering.\n");
>         swsusp_free();
>         free_basic_memory_bitmaps();
>   Unlock:
> @@ -743,7 +743,7 @@ int hibernate(void)
>                 else
>                         flags |= SF_CRC32_MODE;
>
> -               pm_pr_dbg("Writing image.\n");
> +               pm_pr_dbg("Writing hibernation image.\n");
>                 error = swsusp_write(flags);
>                 swsusp_free();
>                 if (!error) {
> @@ -755,7 +755,7 @@ int hibernate(void)
>                 in_suspend = 0;
>                 pm_restore_gfp_mask();
>         } else {
> -               pm_pr_dbg("Image restored successfully.\n");
> +               pm_pr_dbg("Hibernation image restored successfully.\n");
>         }
>
>   Free_bitmaps:
> @@ -894,7 +894,7 @@ static int software_resume(void)
>                 goto Close_Finish;
>         }
>
> -       pm_pr_dbg("Preparing processes for restore.\n");
> +       pm_pr_dbg("Preparing processes for hibernation restore.\n");
>         error = freeze_processes();
>         if (error)
>                 goto Close_Finish;
> @@ -903,7 +903,7 @@ static int software_resume(void)
>   Finish:
>         __pm_notifier_call_chain(PM_POST_RESTORE, nr_calls, NULL);
>         pm_restore_console();
> -       pr_info("resume from hibernation failed (%d)\n", error);
> +       pr_info("resume failed (%d)\n", error);
>         atomic_inc(&snapshot_device_available);
>         /* For success case, the suspend path will release the lock */
>   Unlock:
> @@ -1068,7 +1068,8 @@ static ssize_t resume_store(struct kobject *kobj, struct kobj_attribute *attr,
>         lock_system_sleep();
>         swsusp_resume_device = res;
>         unlock_system_sleep();
> -       pm_pr_dbg("Configured resume from disk to %u\n", swsusp_resume_device);
> +       pm_pr_dbg("Configured hibernation resume from disk to %u\n",
> +                 swsusp_resume_device);
>         noresume = 0;
>         software_resume();
>         return n;
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 26b9168321e7..dcd1376e8fdf 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -8,7 +8,7 @@
>   * Copyright (C) 2006 Rafael J. Wysocki <rjw@sisk.pl>
>   */
>
> -#define pr_fmt(fmt) "PM: " fmt
> +#define pr_fmt(fmt) "PM: hibernation: " fmt
>
>  #include <linux/version.h>
>  #include <linux/module.h>
> @@ -1705,16 +1705,20 @@ int hibernate_preallocate_memory(void)
>         ktime_t start, stop;
>         int error;
>
> -       pr_info("Preallocating image memory... ");
> +       pr_info("Preallocating image memory\n");
>         start = ktime_get();
>
>         error = memory_bm_create(&orig_bm, GFP_IMAGE, PG_ANY);
> -       if (error)
> +       if (error) {
> +               pr_err("Cannot allocate original bitmap\n");
>                 goto err_out;
> +       }
>
>         error = memory_bm_create(&copy_bm, GFP_IMAGE, PG_ANY);
> -       if (error)
> +       if (error) {
> +               pr_err("Cannot allocate copy bitmap\n");
>                 goto err_out;
> +       }
>
>         alloc_normal = 0;
>         alloc_highmem = 0;
> @@ -1804,8 +1808,11 @@ int hibernate_preallocate_memory(void)
>                 alloc -= pages;
>                 pages += pages_highmem;
>                 pages_highmem = preallocate_image_highmem(alloc);
> -               if (pages_highmem < alloc)
> +               if (pages_highmem < alloc) {
> +                       pr_err("Image allocation is %lu pages short\n",
> +                               alloc - pages_highmem);
>                         goto err_out;
> +               }
>                 pages += pages_highmem;
>                 /*
>                  * size is the desired number of saveable pages to leave in
> @@ -1836,13 +1843,12 @@ int hibernate_preallocate_memory(void)
>
>   out:
>         stop = ktime_get();
> -       pr_cont("done (allocated %lu pages)\n", pages);
> +       pr_info("Allocated %lu pages for shapshot\n", pages);
>         swsusp_show_speed(start, stop, pages, "Allocated");
>
>         return 0;
>
>   err_out:
> -       pr_cont("\n");
>         swsusp_free();
>         return -ENOMEM;
>  }
> @@ -1976,7 +1982,7 @@ asmlinkage __visible int swsusp_save(void)
>  {
>         unsigned int nr_pages, nr_highmem;
>
> -       pr_info("Creating hibernation image:\n");
> +       pr_info("Creating image:\n");
>
>         drain_local_pages(NULL);
>         nr_pages = count_data_pages();
> @@ -2010,7 +2016,7 @@ asmlinkage __visible int swsusp_save(void)
>         nr_copy_pages = nr_pages;
>         nr_meta_pages = DIV_ROUND_UP(nr_pages * sizeof(long), PAGE_SIZE);
>
> -       pr_info("Hibernation image created (%d pages copied)\n", nr_pages);
> +       pr_info("Image created (%d pages copied)\n", nr_pages);
>
>         return 0;
>  }
> --
> 2.24.1.735.g03f4e72817-goog
>