On Tuesday, July 19, 2016 08:36:44 AM Chen Yu wrote:
> test_resume mode is to verify if the snapshot data
> written to swap device can be successfully restored
> to memory. It is useful to ease the debugging process
> on hibernation, since this mode can not only bypass
> the BIOSes/bootloader, but also the system re-initialization.
> 
> For example:
> echo test_resume > /sys/power/disk
> echo disk > /sys/power/state
> 
> [  187.306470] PM: Image saving progress:  70%
> [  187.395298] PM: Image saving progress:  80%
> [  187.476697] PM: Image saving progress:  90%
> [  187.554641] PM: Image saving done.
> [  187.558896] PM: Wrote 594600 kbytes in 0.90 seconds (660.66 MB/s)
> [  187.566000] PM: S|
> [  187.589742] PM: Basic memory bitmaps freed
> [  187.594694] PM: Checking hibernation image
> [  187.599865] PM: Image signature found, resuming
> [  187.605209] PM: Loading hibernation image.
> [  187.665753] PM: Basic memory bitmaps created
> [  187.691397] PM: Using 3 thread(s) for decompression.
> [  187.691397] PM: Loading and decompressing image data (148650 pages)...
> [  187.889719] PM: Image loading progress:   0%
> [  188.100452] PM: Image loading progress:  10%
> [  188.244781] PM: Image loading progress:  20%
> [  189.057305] PM: Image loading done.
> [  189.068793] PM: Image successfully loaded
> 
> Suggested-by: Rafael J. Wysocki <rafael.j.wyso...@intel.com>
> Signed-off-by: Chen Yu <yu.c.c...@intel.com>
> ---
> v5:
>  - Introduce a new function to be shared with software_resume().
> v4:
>  - Fix some errors and modify the comment for software_resume_unthaw.
> v3:
>  - As Pavel mentioned, there was a potential risk in previous
>    version that might break the filesystem. According to Rafael's suggestion,
>    this version avoids that issue by restoring the pages with user/kernel
>    threads kept in frozen. Also updated the patch on top of linux-next.
> ---
>  kernel/power/hibernate.c | 61 
> ++++++++++++++++++++++++++++++++----------------
>  kernel/power/swap.c      |  6 +++++
>  2 files changed, 47 insertions(+), 20 deletions(-)
> 
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 5f3523e..73ec63c 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -52,6 +52,7 @@ enum {
>  #ifdef CONFIG_SUSPEND
>       HIBERNATION_SUSPEND,
>  #endif
> +     HIBERNATION_TEST_RESUME,
>       /* keep last */
>       __HIBERNATION_AFTER_LAST
>  };
> @@ -647,12 +648,39 @@ static void power_down(void)
>               cpu_relax();
>  }
>  
> +static int load_image_and_restore(void)
> +{
> +     int error;
> +     unsigned int flags;
> +
> +     pr_debug("PM: Loading hibernation image.\n");
> +
> +     lock_device_hotplug();
> +     error = create_basic_memory_bitmaps();
> +     if (error)
> +             goto Unlock;
> +
> +     error = swsusp_read(&flags);
> +     swsusp_close(FMODE_READ);
> +     if (!error)
> +             hibernation_restore(flags & SF_PLATFORM_MODE);
> +
> +     printk(KERN_ERR "PM: Failed to load hibernation image, recovering.\n");
> +     swsusp_free();
> +     free_basic_memory_bitmaps();
> + Unlock:
> +     unlock_device_hotplug();
> +
> +     return error;
> +}
> +
>  /**
>   * hibernate - Carry out system hibernation, including saving the image.
>   */
>  int hibernate(void)
>  {
>       int error, nr_calls = 0;
> +     bool snapshot_test = false;
>  
>       if (!hibernation_available()) {
>               pr_debug("PM: Hibernation not available.\n");
> @@ -704,7 +732,9 @@ int hibernate(void)
>               pr_debug("PM: writing image.\n");
>               error = swsusp_write(flags);
>               swsusp_free();
> -             if (!error)
> +             if (hibernation_mode == HIBERNATION_TEST_RESUME)
> +                     snapshot_test = true;
> +             if (!error && !snapshot_test)

The above change isn't correct IMO.

If swsusp_write() returns an error, snapshot_test shouldn't be set (that
basically means "no image", so nothing to test).

So this code should look like

        if (!error) {
                if (hibernation_mode == HIBERNATION_TEST_RESUME)
                        snapshot_test = true;
                else
                        power_down();
        }

The rest of the patch looks OK to me.

Thanks,
Rafael

Reply via email to