Please implement in-kernel suspend to both

Bug #1172692 reported by b3nmore on 2013-04-25
38
This bug affects 7 people
Affects Status Importance Assigned to Milestone
OEM Priority Project
High
Ara Pulido
Precise
High
Ara Pulido
pm-utils
Won't Fix
Medium
pm-utils (CentOS)
Fix Released
Undecided
pm-utils (Ubuntu)
Medium
Unassigned
Precise
Undecided
Unassigned
Quantal
Undecided
Unassigned
Trusty
Medium
Unassigned

Bug Description

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.

Related branches

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

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:

Created attachment 627462
Proposed fix

Created attachment 68712
Improved patch to save previous hibernation method

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

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

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

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

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

Changed in pm-utils (Ubuntu):
status: New → Confirmed
Daniel Hahler (blueyed) on 2013-08-23
Changed in pm-utils (Ubuntu):
status: Confirmed → Triaged
importance: Undecided → Medium
Changed in pm-utils:
importance: Unknown → Medium
status: Unknown → Confirmed
Hao-Ran Liu (hzliu123) wrote :

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

Changed in pm-utils (Ubuntu):
status: Triaged → Confirmed
Tim Chen (timchen119) on 2013-11-05
Changed in oem-priority:
importance: Undecided → High
Martin Pitt (pitti) wrote :
Changed in pm-utils (Ubuntu Trusty):
status: Confirmed → Fix Committed
Ara Pulido (ara) on 2013-11-06
Changed in oem-priority:
assignee: nobody → Ara Pulido (apulido)
Martin Pitt (pitti) wrote :

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

Martin Pitt (pitti) wrote :

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

Changed in pm-utils (Ubuntu Precise):
status: New → In Progress
Launchpad Janitor (janitor) wrote :

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

Changed in pm-utils (Ubuntu Trusty):
status: Fix Committed → Fix Released
Hao-Ran Liu (hzliu123) on 2013-11-07
description: updated

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!

Changed in pm-utils (Ubuntu Precise):
status: In Progress → Fix Committed
tags: added: verification-needed
Changed in pm-utils (Ubuntu Quantal):
status: New → Fix Committed
Stéphane Graber (stgraber) wrote :

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

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

Ara Pulido (ara) on 2013-11-14
tags: added: verification-done-precise
Launchpad Janitor (janitor) wrote :

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

Changed in pm-utils (Ubuntu Precise):
status: Fix Committed → Fix Released
CruelAngel (hendricha) wrote :

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

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)

tags: added: verification-done verification-done-quantal
removed: verification-needed
Ara Pulido (ara) on 2013-11-27
Changed in oem-priority:
status: New → Fix Released
Launchpad Janitor (janitor) wrote :

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

Changed in pm-utils (Ubuntu Quantal):
status: Fix Committed → Fix Released

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.

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

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().

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

See also bug 91497.

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

Changed in pm-utils (CentOS):
importance: Unknown → Undecided
status: Unknown → Fix Released

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

Changed in pm-utils:
status: Confirmed → Won't Fix
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.