Please implement in-kernel suspend to both

[b3nmore]

From kernel-3.6 there is in-kernel support for suspend to both (AKA hybrid suspend). Working patches can be found at https://bugs.freedesktop.org/show_bug.cgi?id=52572 (or links).

[Impact]

 * Ubuntu 12.04 implements hybrid suspend differently. It suspends first and wakes up
   the computer for hibernation 15 minutes later. This is risky since the computer may be
   carried when the wakeup happens. The hard disk may experience physical shocks and get
   damaged.

 * Thus, it is desirable to have a real hybrid suspend implementation. In-kernel hybrid
   suspend has been supported since kernel 3.6+. pm-utils only needs a small patch to
   enable this feature.

[Test Case]

 * Ensure all Ubuntu 12.04 packages are up-to-date in the test environment.
   Install pm-utils 1.4.1-9ubuntu1
   Install linux-image-lts-raring

 * Reboot the computer with lts-raring kernel. Run the command: 'pm-suspend-hybrid' from a
   terminal. After the computer suspends, press the power button. It should be able to
   resume from suspension correctly.

 * Run the command above again. After it suspends, remove and reconnect its power supply
   (or its battery). Press the power button. It should be able to resume from hibernation
   correctly.

 * Reboot with the default 3.2 kernel, Run the command above.
   The computer should be able to suspend and then wake up for hibernation 15 minutes
   later.

[Regression Potential]

 * This patch won't affect users who still use 3.2 kernel. It only enables in-kernel
   hybrid suspend if the option 'suspend' is available from /sys/power/disk.

[Jskarvad (jskarvad)]

Created attachment 64764
Add support for in-kernel suspend to both.

From kernel-3.6 there is in-kernel support for suspend to both (AKA hybrid suspend).

Original report with patch from Bojan (also added as attachment):
https://bugzilla.redhat.com/show_bug.cgi?id=843657

[Jaroslav (jaroslav-redhat-bugs)]

Description of problem:
After pm-suspend-hybrid the hibernation method doesn't return to previous state

Version-Release number of selected component (if applicable):
pm-utils-1.4.1-21.fc18.x86_64

How reproducible:
Always

Steps to Reproduce:
1. pm-hibernate
2. pm-suspend-hybrid
3. pm-hibernate

Actual results:
On step 3 hybrid suspend is done instead of hibernation.

Expected results:
The hibernation on step 3.

Additional info:

[Jaroslav (jaroslav-redhat-bugs)]

Created attachment 627462
Proposed fix

[Jskarvad (jskarvad)]

Created attachment 68712
Improved patch to save previous hibernation method

https://bugzilla.redhat.com/show_bug.cgi?id=866487

[Jaroslav (jaroslav-redhat-bugs)]

Updated upstream ticket: https://bugs.freedesktop.org/show_bug.cgi?id=52572

[Fedora (fedora-redhat-bugs)]

pm-utils-1.4.1-22.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/pm-utils-1.4.1-22.fc18

[Fedora (fedora-redhat-bugs)]

Package pm-utils-1.4.1-22.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing pm-utils-1.4.1-22.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-16321/pm-utils-1.4.1-22.fc18
then log in and leave karma (feedback).

[Fedora (fedora-redhat-bugs)]

pm-utils-1.4.1-22.fc18 has been pushed to the Fedora 18 stable repository. If problems still persist, please make note of it in this bug report.

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Hao-Ran Liu (hzliu123)]

Please consider merge the patch. In-kernel suspend to both ram and disk has been supported since kernel 3.6. This patch adds the support and has been verified and works correctly on Ubuntu trusty. It has been included in Fedora 19.

OEM projects will also enable in-kernel suspend to both in 12.04.4.
https://bugs.launchpad.net/watauga/+bug/1177942

[Martin Pitt (pitti)]

http://anonscm.debian.org/gitweb/?p=collab-maint/pm-utils.git;a=commitdiff;h=43b4046af6fc81e93369e53b8d3d4f0475b5fd6b

[Martin Pitt (pitti)]

Uploaded to Debian unstable, will sync into trusty in ~ half a day when it got imported into Launchpad.

[Martin Pitt (pitti)]

Uploaded the precise SRU to the unapproved queue for SRU review. Please update the bug according to https://wiki.ubuntu.com/StableReleaseUpdates#Procedure . Thanks!

[Launchpad Janitor (janitor)]

This bug was fixed in the package pm-utils - 1.4.1-13

---------------
pm-utils (1.4.1-13) unstable; urgency=low

  [ Martin Pitt ]
  * debian/sleep.d/50unload_alx: Unload the alx module during suspend.
    (LP: #1173952)
  * Drop obsolete debian/pm-utils.preinst.
  * Drop obsolete Conflicts/Replaces to pm-utils-powersave-policy and
    laptop-mode-tools.
  * debian/copyright: Move to copyright format 1.0.
  * Move to canonical Vcs-* fields (anonscm.debian.org).
  * Bump to Standards-Version 3.9.5.

  [ Hao-Ran Liu ]
  * Add 28-suspend-hybrid.patch: Support in-kernel hybrid suspend
    (Linux >= 3.6) (LP: #1172692)

 -- Martin Pitt <email address hidden> Wed, 06 Nov 2013 09:26:45 +0100

[Stéphane Graber (stgraber)]

Hello b3nmore, or anyone else affected,

Accepted pm-utils into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/pm-utils/1.4.1-9fix.ubuntu12.04 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Stéphane Graber (stgraber)]

Hello b3nmore, or anyone else affected,

Accepted pm-utils into quantal-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/pm-utils/1.4.1-9fix.ubuntu12.10 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[b3nmore (b3nmore)]

pm-utils 1.4.1-9fix.ubuntu12.04 works for me on precise. Thanks.

[Launchpad Janitor (janitor)]

This bug was fixed in the package pm-utils - 1.4.1-9fix.ubuntu12.04

---------------
pm-utils (1.4.1-9fix.ubuntu12.04) precise-proposed; urgency=low

  (Weird version number to stay smaller than the 13.04 version)
  * Add support for hybrid in-kernel suspend (Linux >= 3.6) (LP: #1172692)
 -- Hao-Ran Liu (Joseph Liu) <email address hidden> Mon, 04 Nov 2013 15:39:45 +0800

[CruelAngel (hendricha)]

Checked it out with the 12.04 kernel. pm-suspend-hybrid made my screen flash black for a second then I saw my desktop, but frozen. Weirdly enough it did hibernate in the background, since when I reset the machine manually it booted back to the point where I suspended it.
Random fact: after installing TuxOnIce, pm-suspend-hybrid does work, so they seem to have a workaround for the issue,

[Hao-Ran Liu (hzliu123)]

pm-utils 1.4.1-9fix.ubuntu12.10 works good in a clean quantal installation with all packages updated under kvm.

pm-suspend: good.
pm-hibernate: good.
pm-suspend-hybrid: good. (Since the kernel is 3.5, it suspend first and hibernate 15 mins later)

[Launchpad Janitor (janitor)]

This bug was fixed in the package pm-utils - 1.4.1-9fix.ubuntu12.10

---------------
pm-utils (1.4.1-9fix.ubuntu12.10) quantal-proposed; urgency=low

  (Weird version number to stay smaller than the 13.04 version)
  * Add support for hybrid in-kernel suspend (Linux >= 3.6) (LP: #1172692)
 -- Hao-Ran Liu (Joseph Liu) <email address hidden> Mon, 04 Nov 2013 15:39:45 +0800

[Stéphane Graber (stgraber)]

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

[Dirk F (fieldhouse)]

Comment on attachment 68712
Improved patch to save previous hibernation method

Review of attachment 68712:
-----------------------------------------------------------------

In do_hibernate() the attempt to save and restore the active mode in /sys/power/disk fails, causing "sh: I/O error" message in pm log (attempting to write something that isn't one of the modes in /sys/power/disk, namely an empty string). Instrumenting the function I found that HIBERNATE_MODE_SAVE was never set.

The characters [] are special in a shell pattern (which is what follows the ## and %% shell variable expansion modifiers) and have to be escaped: \[ \].

The following works as you intended:

do_hibernate()
 {
  [ -n "${HIBERNATE_MODE}" ] && \
  grep -qw "${HIBERNATE_MODE}" /sys/power/disk && \
  #df 2016-02-07 Shell patterns have to be escaped \[ \]! Fixes sh: I/O error when -z $HIBERNATE_MODE_SAVE
  HIBERNATE_MODE_SAVE=$(cat /sys/power/disk) && \
  HIBERNATE_MODE_SAVE="${HIBERNATE_MODE_SAVE##*\[}" && \
  HIBERNATE_MODE_SAVE="${HIBERNATE_MODE_SAVE%%\]*}" && \
  echo -n "${HIBERNATE_MODE}" > /sys/power/disk
  echo -n "disk" > /sys/power/state
  RET=$?
  echo -n "$HIBERNATE_MODE_SAVE" > /sys/power/disk
  return "$RET"
 }

Although you could make the penultimate line as follows I don't recommend it because it would hide any problems like the escaping issue that could cause HIBERNATE_MODE_SAVE to be invalid:

  [ -n "$HIBERNATE_MODE_SAVE" ] && echo -n "$HIBERNATE_MODE_SAVE" > /sys/power/disk

[Dirk F (fieldhouse)]

In comment #2 I wrote:
> Comment on attachment 68712 [details] [review]
>...
> Although you could make the penultimate line as follows I don't recommend it
> because it would hide any problems like the escaping issue that could cause
> HIBERNATE_MODE_SAVE to be invalid:
>
> [ -n "$HIBERNATE_MODE_SAVE" ] && echo -n "$HIBERNATE_MODE_SAVE" >
> /sys/power/disk

In fact there are 2 cases as the code is used now:

1 HIBERNATE_MODE unset => normal hibernate

2 HIBERNATE_MODE = suspend => suspend-hybrid

Given which, I've revised my comment above and propose a new version of the modified do_hibernate() as follows:

 do_hibernate()
 {
  local hibernate_mode_save ret

  [ -n "${HIBERNATE_MODE}" ] && \
   grep -qw "${HIBERNATE_MODE}" /sys/power/disk && \
   hibernate_mode_save=$(cat /sys/power/disk) && \
   hibernate_mode_save="${hibernate_mode_save##*\[}" && \
   hibernate_mode_save="${hibernate_mode_save%%\]*}" && \
   [ "$hibernate_mode_save" != "${HIBERNATE_MODE}" ] || \
   hibernate_mode_save=""
  [ -n "$hibernate_mode_save" ] && \
   echo -n "${HIBERNATE_MODE}" > /sys/power/disk
  echo -n "disk" > /sys/power/state
  ret=$?
  [ -n "$hibernate_mode_save" ] && \
   echo -n "$hibernate_mode_save" > /sys/power/disk
  return $ret
 }

The key points:
- hibernate_mode_save is only set if the current HIBERNATE_MODE is being changed (which only happens, if it does, in the suspend-hybrid case);
- on resume the hibernate mode is only restored if hibernate_mode_save was set.

This fixes:
- the failure to restore the hibernate mode with suspend-hybrid;
- "sh: I/O error" on resume from suspend-hybrid
- "sh: I/O error" on resume from hibernate.

Finally, the pm_functions script uses "echo -n" (from line 318, including the above patch) and local declarations while the comment against function log() implies that the script is aiming for POSIX conformance and yet other functions use non-POSIX local declarations. local and "echo -n" usages are fine for Debian and derived environments. To achieve POSIX conformance such usages would have to be reviewed and modified; "echo -n" could be replaced with printf (the parameters to be echoed in each case being plain text not containing formatting commands); or a shell function echo() can be added based on log().

[Dirk F (fieldhouse)]

In comment #3 I wrote:
>...
> Finally, the pm_functions script uses "echo -n" (from line 318, including
> the above patch) ...

See also bug 91497.

[Dirk F (fieldhouse)]

Created attachment 122010
patch based on review of patch 68712, taken from running pm-functions

[Ajax-a (ajax-a)]

pm-utils hasn't been touched in eight years, none of this is likely to get addressed. Closing bugs and disabling the bz product.