[bug#40927] [PATCH] Allow resume from swap device during boot

[bug#40927] [PATCH] Allow resume from swap device during boot

[Jean-Baptiste Note]

[-- Attachment #1.1: Type: text/plain, Size: 1163 bytes --]


Dear GUIX maintainers,

Current the GUIX SD boot process does not allow resuming from sleep,
even thought sleep options are available through loginctl, eg:

loginctl hybrid-sleep
loginctl hibernate

This is a very important feature for people like me using GUIX SD on a
laptop (yes, it is possible, mine is a corebooted X230 running
linux-libre!)

This patch is based on a patch floating around. The core functionality
has been isolated, the resume function isolated, the patch rebased and
tested. I'm not taking credit for it, even though tracing the exact
origin is hard.

The resume hook is called if the resume= kernel argument is provided,
which one can do during system configuration.

My scheme level is zero, so please bear with me. In particular, some
conditionals could maybe be moved within the function, or the function
itself called within some already-available hooks. Also it is not clear
if the commit log is adequate for such a change.

Please let me know how to improve this and get this merged; I can also
write some documentation (probably once the mechanism is in place) to
explain how the feature can be used.

Kind regards,
Jean-Baptiste


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: allow resume from swap --]
[-- Type: text/x-patch, Size: 3609 bytes --]

From 2531d1d08dabb53ff15020aedcec2ad5d8e6c600 Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Note <jean-baptiste.note@m4x.org>
Date: Mon, 27 Apr 2020 20:42:03 +0000
Subject: [PATCH] linux-boot: Add support for resuming from swap device.

* gnu/build/linux-boot.scm (resume-from-device): Add function.

* gnu/build/linux-boot.scm (boot-system): Add hook calling resume-from-device
  if specified on commandline, before mounting any actual disk filesystems.
---
 gnu/build/linux-boot.scm | 43 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 4fb711b8f2..907c84276f 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -357,6 +357,37 @@ the last argument of `mknod'."
                      (compose (cut string=? program <>) basename))))
           (filter-map string->number (scandir "/proc")))))
 
+(define (resume-from-device resume-device)
+  "Resume from hibernation state on device DEVICE. This *must* happen before
+we mount any filesystems on disk. See
+linux-libre/Documentation/swsusp.txt. Please note that this function will not
+return if resume happens successfully, and will return if swap device does not
+contain a valid resume signature."
+  (false-if-exception
+   (let* ((device-base-name
+           ;; The base name of the device file, after resolving
+           ;; symlinks.
+           (let loop ((file resume-device))
+             (match (stat:type (lstat file))
+               ('symlink
+                (let ((target (readlink file)))
+                  (if (string-prefix? "/" target)
+                      (loop target)
+                      (loop (string-append (dirname file) "/" target)))))
+               (_ (basename file)))))
+          (major+minor
+           ;; The major:minor string (e.g. "8:2") corresponding
+           ;; to the resume device.
+           (call-with-input-file (string-append "/sys/class/block/"
+                                                device-base-name
+                                                "/dev")
+             read-line)))
+     ;; Write the major:minor string to /sys/power/resume
+     ;; to attempt resume from hibernation.
+     (when major+minor
+       (call-with-output-file "/sys/power/resume"
+         (cut display major+minor <>))))))
+
 (define* (mount-root-file-system root type
                                  #:key volatile-root? (flags 0) options)
   "Mount the root file system of type TYPE at device ROOT. If VOLATILE-ROOT? is
@@ -493,9 +524,10 @@ upon error."
   (call-with-error-handling
     (lambda ()
       (mount-essential-file-systems)
-      (let* ((args    (linux-command-line))
-             (to-load (find-long-option "--load" args))
-             (root    (find-long-option "--root" args)))
+      (let* ((args          (linux-command-line))
+             (to-load       (find-long-option "--load" args))
+             (root          (find-long-option "--root" args))
+             (resume-device (find-long-option "resume" args)))
 
         (when (member "--repl" args)
           (start-repl))
@@ -528,6 +560,11 @@ upon error."
           (unless (pre-mount)
             (error "pre-mount actions failed")))
 
+        (when (and resume-device
+                   (file-exists? resume-device)
+                   (file-exists? "/sys/power/resume"))
+          (resume-from-device resume-device))
+
         (setenv "EXT2FS_NO_MTAB_OK" "1")
 
         (if root
-- 
2.26.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[bug#40927] [PATCH] Allow resume from swap device during boot

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

Hi Jean-Baptiste Note,

On Tue, 28 Apr 2020 07:26:15 +0000
Jean-Baptiste Note <jean-baptiste.note@m4x.org> wrote:

> linux-libre/Documentation/swsusp.txt

Should be linux-libre/Documentation/power/swsusp.txt .

Otherwise OK.

Please try to find the names of the actual authors so we can commit the parts
they wrote with them as author.

Who provides the "resume=" argument eventually?

Also, if we want to support this, what would the Guix installer have to do?

Add a swap partition?  Does it have to be exactly as big as the amount of RAM
or does it need to be bigger because of overhead?

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#40927] [PATCH] Allow resume from swap device during boot

[Tobias Geerinckx-Rice via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 4148 bytes --]

Jean-Baptiste,

Jean-Baptiste Note 写道：
> This is a very important feature for people like me using GUIX 
> SD on a
> laptop (yes, it is possible, mine is a corebooted X230 running
> linux-libre!)

Greetings, brother in hardware.

Note that hibernation and resumption already work fine if you rely 
on the kernel's built-in support.  I've been hibernating my X230T 
for years without patches.

This explicit initramfs support is only needed if your storage 
drivers aren't available when the kernel itself tries to resume, 
and the initramfs has to retry later.  That's slower but allows 
things like ahci to be modular instead of built-in.

> This patch is based on a patch floating around. The core 
> functionality
> has been isolated, the resume function isolated, the patch 
> rebased and
> tested. I'm not taking credit for it, even though tracing the 
> exact
> origin is hard.

Heh.  It's certainly very similar to a patch of mine that's ‘out 
there’ although it's probably not the only one.

> The resume hook is called if the resume= kernel argument is 
> provided,
> which one can do during system configuration.

This patch ignores the ‘noresume’ flag, which is bad.  If it's 
present anywhere on the command line resumption should be skipped:

+        (when (and (not (member "noresume" args))
+                   resume-device
+                   (file-exists? resume-device)
+                   (file-exists? "/sys/power/resume"))
+          (resume-from-device resume-device))

> My scheme level is zero, so please bear with me. In particular, 
> some
> conditionals could maybe be moved within the function,

I'd like to see everything moved into a self-contained DTRT 
[try-to-]resume procedure, except for the ‘noresume’ check (so 
callers are free to explicitly resume if they so please):

+        (unless (member "noresume" args)
+                (resume-if-hibernated  (find-long-option "resume" 
args)))

This is what the last iteration of my patch does.

If (find-long-option "resume" args) is #f, fall back to 
CONFIG_PM_STD_PARTITION.  This is what the kernel does: even if 
‘resume=’ is missing, it will try to resume from that device if 
specified.  We should match that behaviour if we can.

Problem is, I forgot how to get that value from user space, or if 
it's even possible.  I gave up on Linux's built-in hibernation 
(swsusp) years ago.  It's poorly written and maintained, and the 
author fiercely defends it from all improvement.

Instead I use TuxOnIce, which exposes it under 
/sys/power/tuxonice/….  That's obviously not an option here, 
although it would be friendly to fall back to it for us ToI users 
:-)

*user.

I think ToI even exposes the ‘last used hibernation device’ 
somewhere, so user space can just use that instead of 
CONFIG_PM_STD_PARTITION.  This is obviously the right thing to do. 
Again, not sure if swsusp does.

> or the function itself called within some already-available 
> hooks.

We don't have a concept of ‘initramfs hooks’ and I think that's a 
good thing.  Gives me dracut flashbacks.  Don't lets bother with 
them until we need them, which is never.

> Also it is not clear if the commit log is adequate for such a 
> change.

It's all right :-)  If anything, it's too long:

  linux-boot: Add support for resuming from swap device.

  * gnu/build/linux-boot.scm (resume-from-device): New procedure.
  * gnu/build/linux-boot.scm (boot-system): Call it, unless 
  ‘noresume’
    is present on the kernel command line.

> Please let me know how to improve this and get this merged; I 
> can also
> write some documentation (probably once the mechanism is in 
> place) to
> explain how the feature can be used.

If this works properly no documentation is needed.  The kernel by 
default writes to the first swap device; we should magically 
resume from it.  Forcing users to know (or care) about part 2 is 
asymmetrical and wrong.

Not sure if that's possible with vanilla Linux-Libre…

Will stop shilling ToI for now,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[bug#40927] [PATCH] Allow resume from swap device during boot

[Tobias Geerinckx-Rice via Guix-patches via]

[-- Attachment #1.1: Type: text/plain, Size: 691 bytes --]

Hullo again,

Tobias Geerinckx-Rice 写道：
> This is what the last iteration of my patch does.

I managed to find it!  Long live dusty back-ups.

It uses native procedures to divine the device number, instead of 
— clever! — /sys/blockery that supports only a subset of specs 
(and reminds me too much of ‘look what I found lying around’ shell 
scripting).  If any are missing we can add them to 
CANONICALIZE-DEVICE-SPEC to the benefit of all.

Tested with both built-in & modular ATA drivers × mainline swsusp 
& TuxOnIce.  More welcome.

These copyright dates are downright embarrassing.  Let's Get 
(something like) This Merged!

Kind regards,

T G-R


[-- Attachment #1.2: 0001-linux-boot-Resume-from-hibernation.patch --]
[-- Type: text/x-patch, Size: 4652 bytes --]

From 46c4c1010d9257f3d1d1ddb201dc7f7519d42ba0 Mon Sep 17 00:00:00 2001
From: Tobias Geerinckx-Rice <me@tobias.gr>
Date: Fri, 26 Feb 2016 17:57:07 +0100
Subject: [PATCH] linux-boot: Resume from hibernation.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* gnu/build/linux-boot.scm (resume-if-hibernated): New procedure.
(boot-system): Call it unless ‘noresume’ was specified.
---
 gnu/build/linux-boot.scm | 52 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 05e833c0c6..74e76b6a31 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2016, 2017, 2019 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2019 Guillaume Le Vaillant <glv@posteo.net>
 ;;;
@@ -110,6 +111,51 @@ OPTION doesn't appear in ARGUMENTS."
                        (substring arg (+ 1 (string-index arg #\=)))))
                 arguments)))
 
+(define (resume-if-hibernated device)
+  "Resume from hibernation if possible.  This is safe ONLY if no on-disk file
+systems have been mounted; calling it later risks severe file system corruption!
+See <Documentation/swsusp.txt> in the kernel source directory.  This is the
+caller's responsibility, as is catching exceptions if resumption was supposed to
+happen but didn't.
+
+Resume only from DEVICE if it's a string.  If it's #f, use the kernel's default
+hibernation device (CONFIG_PM_STD_PARTITION).  Never return if resumption
+succeeds.  Return nothing otherwise.  The kernel logs any details to dmesg."
+
+  (define (string->major:minor string)
+    "Return a string with MAJOR:MINOR numbers of the device specified by STRING"
+
+    ;; The "resume=" kernel command-line option always provides a string, which
+    ;; can represent a device, a UUID, or a label.  Check for all three.
+    (let* ((spec (cond ((string-prefix? "/" string) string)
+                       ((uuid string) => identity)
+                       (else (file-system-label string))))
+           ;; XXX kernel's swsusp_resume_can_resume() waits if ‘resumewait’ is
+           ;; found on the command line; our canonicalize-device-spec gives up
+           ;; after 20 seconds.  We could loop (ew!) if someone relies on it…
+           (device (canonicalize-device-spec spec))
+           (rdev (stat:rdev (stat device)))
+           (minor (modulo rdev 256))
+           (major (/ (- rdev minor) 256)))
+      (format #f "~a:~a" major minor)))
+
+  ;; Write the MAJOR:MINOR numbers of the specified or default resume DEVICE to
+  ;; this magic file.  The kernel will immediately try to resume from it.
+  (let ((resume "/sys/power/resume"))
+    (when (file-exists? resume)         ; this kernel supports hibernation
+      ;; Honour the kernel's default device (only) if none other was given.
+      (let ((major:minor (if device
+                             (string->major:minor device)
+                             (let ((default (call-with-input-file resume
+                                              read-line)))
+                               ;; Don't waste time echoing ‘nothing’ to /sys.
+                               (if (string=? "0:0" default)
+                                   #f
+                                   default)))))
+        (when major:minor
+          (call-with-output-file resume ; may throw an ‘Invalid argument’
+            (cut display major:minor <>))))))) ; may never return
+
 (define* (make-disk-device-nodes base major #:optional (minor 0))
   "Make the block device nodes around BASE (something like \"/root/dev/sda\")
 with the given MAJOR number, starting with MINOR."
@@ -504,6 +550,12 @@ upon error."
         (load-linux-modules-from-directory linux-modules
                                            linux-module-directory)
 
+        (unless (member "noresume" args)
+          ;; Try to resume immediately after loading (storage) modules
+          ;; but before any on-disk file systems have been mounted.
+          (false-if-exception           ; failure is not fatal
+           (resume-if-hibernated (find-long-option "resume" args))))
+
         (when keymap-file
           (let ((status (system* "loadkeys" keymap-file)))
             (unless (zero? status)
-- 
2.25.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[bug#40927] [PATCH] Allow resume from swap device during boot

[Jean-Baptiste Note]

[-- Attachment #1.1: Type: text/plain, Size: 3378 bytes --]

Hi Danny,

Thanks for being so guix in answering the original report.
Your questions prompted me to take a bit of time before answering.

Regarding attribution, i've dug two instances of this patch:

https://git.pantherx.org/mirror/guix/commit/af8d58efa05927b24694c87379cccc378f3fdde7

https://issues.guix.gnu.org/issue/37290
https://lists.gnu.org/archive/html/bug-guix/2019-09/msg00017.html

Both instances of the patch are given by author Mark H Weaver <mhw@netris.org>

As you can see there's already an open issue in GUIX regarding resume;
i'm sorry i missed that. Maybe for attribution it would make sense for
me to resend the patch there; let me know how to proceed.

Regarding the patch itself and your questions:

  - for comparison to the original patch: elogind currently has the
    configurability required to make the second half of the original
    patch moot, and I think it would be better to split the work
    anyways;

  - i've addressed the reference to swsusp.txt

  - the resume argument is provided by (in my case) the system
    definition, so I basically have something like that:

#+begin_src scheme
(operating-system
  (kernel-arguments
  (cons
   (string-append "modprobe.blacklist="
		  (comma-separated
		   %redundant-linux-modules))
  '("--resume=/dev/sdb3" "quiet"))))
#end_src

  - in order to support this in the GUIX installer, I guess adding such
    a line in the generated system definition, along with a swap
    partition, would be sufficient. My knowledge of the workings of the
    guix installer is null, tough :)

  - please note that if you have a suspend signature on your swap
    partition, and fail to resume from it (for instance because of a
    missing kernel commandline argument...), then shepherd will fail
    bringing up the swap device, and you will need to handle this
    situation by hand currently;

  - My knowledge of the details of swsusp is rather limited: I don't
    know, for instance, how this would translate in case of a more
    complex setup of multi-swap devices;

  - However, it seems very clear from the documentation that a mere
    resume= argument syntax can trigger specific initialization paths
    within the kernel itself leading to a resume. I find it better
    style, ultimately, to properly separate the responsibilities of
    kernel and initrd, follow the GUIX conventions with a --resume
    argument.

  - therefore I propose the following, updated patch, with a more
    GUIX-like --resume argument; I've tested it on my laptop, which
    happens to have an encrypted /home partition (but only one swap
    device).

  - as far as I can understand the documentation, the swap partition
    does not have to be as big as the RAM; by default the kernel will
    strive to make the suspend image 2/5 of the physical RAM, but this
    can be tuned more aggressively. There's actually less than overhead:
    some parts of the memory can be discarded (shared MMAPd files, the
    buffer cache); and even then you can hope compression will reduce
    the needs. If the swap space is not large enough, suspend will
    simply fail.

This probably raises more issues than it solves, however i'd rather you
have the whole picture. Let me know how to proceed; I'm willing to
follow-up with other patches to get the feature up to the required
standards of quality.

Kind regards,
Jean-Baptiste

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: updated resume patch --]
[-- Type: text/x-patch, Size: 3617 bytes --]

From 6cf0833a93bede5fe41e0f126267876fefbb3672 Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Note <jean-baptiste.note@m4x.org>
Date: Mon, 27 Apr 2020 20:42:03 +0000
Subject: [PATCH] linux-boot: Add support for resuming from swap device.

* gnu/build/linux-boot.scm (resume-from-device): Add function.

* gnu/build/linux-boot.scm (boot-system): Add hook calling resume-from-device
  if specified on commandline, before mounting any actual disk filesystems.
---
 gnu/build/linux-boot.scm | 43 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 05e833c0c6..2febe61ec1 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -357,6 +357,37 @@ the last argument of `mknod'."
                      (compose (cut string=? program <>) basename))))
           (filter-map string->number (scandir "/proc")))))
 
+(define (resume-from-device resume-device)
+  "Resume from hibernation state on RESUME-DEVICE. This *must* happen before
+we mount any filesystems on disk. See
+linux-libre/Documentation/power/swsusp.txt. Please note that this function
+will not return if resume happens successfully, and will return if swap device
+does not contain a valid resume signature."
+  (false-if-exception
+   (let* ((device-base-name
+           ;; The base name of the device file, after resolving
+           ;; symlinks.
+           (let loop ((file resume-device))
+             (match (stat:type (lstat file))
+               ('symlink
+                (let ((target (readlink file)))
+                  (if (string-prefix? "/" target)
+                      (loop target)
+                      (loop (string-append (dirname file) "/" target)))))
+               (_ (basename file)))))
+          (major+minor
+           ;; The major:minor string (e.g. "8:2") corresponding
+           ;; to the resume device.
+           (call-with-input-file (string-append "/sys/class/block/"
+                                                device-base-name
+                                                "/dev")
+             read-line)))
+     ;; Write the major:minor string to /sys/power/resume
+     ;; to attempt resume from hibernation.
+     (when major+minor
+       (call-with-output-file "/sys/power/resume"
+         (cut display major+minor <>))))))
+
 (define* (mount-root-file-system root type
                                  #:key volatile-root? (flags 0) options)
   "Mount the root file system of type TYPE at device ROOT. If VOLATILE-ROOT? is
@@ -493,9 +524,10 @@ upon error."
   (call-with-error-handling
     (lambda ()
       (mount-essential-file-systems)
-      (let* ((args    (linux-command-line))
-             (to-load (find-long-option "--load" args))
-             (root    (find-long-option "--root" args)))
+      (let* ((args          (linux-command-line))
+             (to-load       (find-long-option "--load" args))
+             (root          (find-long-option "--root" args))
+             (resume-device (find-long-option "--resume" args)))
 
         (when (member "--repl" args)
           (start-repl))
@@ -528,6 +560,11 @@ upon error."
           (unless (pre-mount)
             (error "pre-mount actions failed")))
 
+        (when (and resume-device
+                   (file-exists? resume-device)
+                   (file-exists? "/sys/power/resume"))
+          (resume-from-device resume-device))
+
         (setenv "EXT2FS_NO_MTAB_OK" "1")
 
         (if root
-- 
2.26.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[bug#40927] [PATCH] Allow resume from swap device during boot

[Jean-Baptiste Note]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

Hi there,

As i'm not subscribed to the list, I completely missed on Tobias'
messages -- and my previous one must seem strange in hindsight. Sorry
about this.

I'm now subscribed, but still can't reply directly to his email.

Though I still won't be able to answer directly to Tobias' latest email,
I will test the patch, and /somehow/ report back on Thursday.

For now I must say that on my kernel -- official guix linux-libre-5.4 --
the built-in support does not seem to be working (I'm pretty sure i've
tested this); I will double-check however.

Kind regards,
Jean-Baptiste

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[bug#40927] Subject=[bug#40927] [PATCH] Allow resume from swap device during boot

[Jean-Baptiste Note]

[-- Attachment #1.1: Type: text/plain, Size: 3243 bytes --]


Dear Tobias,

I hopefully found a way to reply to your message directly. I've checked
resume with your patch -- actually a slightly reworked version of it; a
few remarks:

- I've amended it as per Danny's suggestions regarding documentation location

- I've adjusted it to take the guixy --resume and --noresume options
  rather than the kernel norm resume and noresume. In particular this
  allows me to test the 'kernel' resume path and the 'initrd' resume
  path with the same os definition quickly, only adjusting the GRUB
  cmdline within GRUB.

- I find the added copyright years on top strange -- shouldn't it be
  2020 ? 

The results of my tests are attached as a separate file. In a nutshell:

- I can confirm that on my kernel (default guix linux-libre-5.4) the
  resume= option by itself does not resume at all. This seems to have been
  reported already:
  https://lists.gnu.org/archive/html/bug-guix/2016-11/msg00060.html

- resume-by-uuid from userspace does not work
- resume-by-label from userspace does not work either
(as much as I'd like them to).

(please note that, on top of this, I found no way to specify the swap
devices in the os definition by UUID or LABEL either -- so the
limitation does not bother me in the current state).

Each time these loop for 20 seconds, waiting for the partition to appear
(the partition is there, from dmesg, but somehow is not in the database
looked for in (canonicalize-device-spec spec) -- weird).

Regarding the interface, I'm not sure we should feel bound by the
argument naming convention of the kernel for resume handling from
userspace -- we could leave its own namespace for the kernel to handle,
for those kernels that can, and separate the resume handling as done by
initrd in a separate, more guixy namespace.

Actually I don't really understand why we would want overlapping of
names for two different codepaths; I think separating the names brings
more flexibility (you can control from the GRUB commandline which resume
path you want to take) and less confusion (it is clearer from the
commandline where the resuming will/should be handled).

To summarize:

* I still think that this feature is greatly needed: we can hibernate
  with the current software stack, but the default kernel won't resume,
  leaving the system in a bad state (swap is not activated by shepherd,
  see logs), and we need manual swapon to leave this bad state;

* This version of the patch looks like it can handle UUID and LABEL,
  while it can't -- for reasons that i've not dug;

* Meanwhile I find resume-by-uuid or resume-by-label currently
  pointless, given the limited ways we have to specify swap devices in
  the OS definition file.

I'd be in favor of doing the following:

* Dropping support for UUID and LABEL in the code *OR* signalling
  clearly in some comment that the path is currently non-functional;

* Including the patch ASAP to avoid the really bad state we're in
  currently after a suspend.

* Using the guixy --resume and --noresume options rather than the kernel
  ones

Please let me know how to proceed, and let me know how to handle the
copyright notice.

Kind regards,
Jean-Baptiste


[-- Attachment #1.2: resume tests logs --]
[-- Type: text/plain, Size: 2701 bytes --]

# by device node -- does work
jb@guixrules ~$ sudo loginctl hibernate
jb@guixrules ~$ cat /proc/cmdline 
BOOT_IMAGE=/gnu/store/bkpkbms3mp8s45kpir4f2cnvxvgdvssp-linux-libre-5.4.39/bzImage --root=ed73cc09-aff3-41e4-90b6-51fb41a7d225 --system=/gnu/store/xcdn9naivwxhr4x0cq669zzn9f02x706-system --load=/gnu/store/xcdn9naivwxhr4x0cq669zzn9f02x706-system/boot modprobe.blacklist=pcspkr,snd_pcsp --resume=/dev/sda3 quiet

# by UUID -- does not work
jb@guixrules ~$ sudo swaplabel /dev/sdb3
swaplabel: /dev/sdb3: not a valid swap partition
jb@guixrules ~$ sudo swapon /dev/sdb3
swapon: /dev/sdb3: software suspend data detected. Rewriting the swap signature.
jb@guixrules ~$ sudo swaplabel /dev/sdb3
LABEL: resume_device
UUID:  446c81d4-efcf-4508-a9ab-bb38f74ff77d
jb@guixrules ~$ cat /proc/cmdline 
BOOT_IMAGE=/gnu/store/bkpkbms3mp8s45kpir4f2cnvxvgdvssp-linux-libre-5.4.39/bzImage --root=ed73cc09-aff3-41e4-90b6-51fb41a7d225 --system=/gnu/store/xcdn9naivwxhr4x0cq669zzn9f02x706-system --load=/gnu/store/xcdn9naivwxhr4x0cq669zzn9f02x706-system/boot modprobe.blacklist=pcspkr,snd_pcsp --resume=446c81d4-efcf-4508-a9ab-bb38f74ff77d quiet
jb@guixrules ~$ diff <(echo 446c81d4-efcf-4508-a9ab-bb38f74ff77d) <(echo 446c81d4-efcf-4508-a9ab-bb38f74ff77d)
jb@guixrules ~$ echo $?
0

# by LABEL -- does not work
jb@guixrules ~$ sudo swaplabel /dev/sdb3
swaplabel: /dev/sdb3: not a valid swap partition
jb@guixrules ~$ sudo swapon /dev/sdb3
swapon: /dev/sdb3: software suspend data detected. Rewriting the swap signature.
jb@guixrules ~$ sudo swaplabel 
swaplabel: no device specified
Try 'swaplabel --help' for more information.
jb@guixrules ~$ sudo swaplabel /dev/sdb3
LABEL: resume_device
UUID:  446c81d4-efcf-4508-a9ab-bb38f74ff77d
jb@guixrules ~$ cat /proc/cmdline 
BOOT_IMAGE=/gnu/store/bkpkbms3mp8s45kpir4f2cnvxvgdvssp-linux-libre-5.4.39/bzImage --root=ed73cc09-aff3-41e4-90b6-51fb41a7d225 --system=/gnu/store/xcdn9naivwxhr4x0cq669zzn9f02x706-system --load=/gnu/store/xcdn9naivwxhr4x0cq669zzn9f02x706-system/boot modprobe.blacklist=pcspkr,snd_pcsp --resume=resume_device quiet

# leaving it to the kernel -- does not work
jb@guixrules ~$ sudo swaplabel /dev/sdb3
swaplabel: /dev/sdb3: not a valid swap partition
jb@guixrules ~$ sudo swapon /dev/sdb3
swapon: /dev/sdb3: software suspend data detected. Rewriting the swap signature.
jb@guixrules ~$ cat /proc/cmdline 
BOOT_IMAGE=/gnu/store/bkpkbms3mp8s45kpir4f2cnvxvgdvssp-linux-libre-5.4.39/bzImage --root=ed73cc09-aff3-41e4-90b6-51fb41a7d225 --system=/gnu/store/xcdn9naivwxhr4x0cq669zzn9f02x706-system --load=/gnu/store/xcdn9naivwxhr4x0cq669zzn9f02x706-system/boot modprobe.blacklist=pcspkr,snd_pcsp resume=/dev/sdb3 quiet

[-- Attachment #1.3: resume patch with longopt --]
[-- Type: text/x-patch, Size: 4728 bytes --]

From 889fc647a84289747ae840778f0313193eca1316 Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Note <jean-baptiste.note@m4x.org>
Date: Wed, 6 May 2020 20:38:21 +0000
Subject: [PATCH] linux-boot: Resume from hibernation.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* gnu/build/linux-boot.scm (resume-if-hibernated): New procedure.
(boot-system): Call it unless ‘noresume’ was specified.
---
 gnu/build/linux-boot.scm | 53 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 05e833c0c6..be1cda0181 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2016, 2017, 2019, 2020 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2019 Guillaume Le Vaillant <glv@posteo.net>
 ;;;
@@ -110,6 +111,52 @@ OPTION doesn't appear in ARGUMENTS."
                        (substring arg (+ 1 (string-index arg #\=)))))
                 arguments)))
 
+(define (resume-if-hibernated device)
+  "Resume from hibernation if possible.  This is safe ONLY if no on-disk file
+systems have been mounted; calling it later risks severe file system
+corruption!  See <Documentation/power/swsusp.txt> in the kernel source
+directory.  This is the caller's responsibility, as is catching exceptions if
+resumption was supposed to happen but didn't.
+
+Resume only from DEVICE if it's a string.  If it's #f, use the kernel's default
+hibernation device (CONFIG_PM_STD_PARTITION).  Never return if resumption
+succeeds.  Return nothing otherwise.  The kernel logs any details to dmesg."
+
+  (define (string->major:minor string)
+    "Return a string with MAJOR:MINOR numbers of the device specified by STRING."
+
+    ;; The "resume=" kernel command-line option always provides a string, which
+    ;; can represent a device, a UUID, or a label.  Check for all three.
+    (let* ((spec (cond ((string-prefix? "/" string) string)
+                       ((uuid string) => identity)
+                       (else (file-system-label string))))
+           ;; XXX kernel's swsusp_resume_can_resume() waits for the resume
+           ;; device to appear if ‘resumewait’ is found on the command line;
+           ;; our canonicalize-device-spec gives up after 20 seconds.  We
+           ;; could loop (ew!) if someone relies on it…
+           (device (canonicalize-device-spec spec))
+           (rdev (stat:rdev (stat device)))
+           (minor (modulo rdev 256))
+           (major (/ (- rdev minor) 256)))
+      (format #f "~a:~a" major minor)))
+
+  ;; Write the MAJOR:MINOR numbers of the specified or default resume DEVICE to
+  ;; this magic file.  The kernel will immediately try to resume from it.
+  (let ((resume "/sys/power/resume"))
+    (when (file-exists? resume)         ; this kernel supports hibernation
+      ;; Honour the kernel's default device (only) if none other was given.
+      (let ((major:minor (if device
+                             (string->major:minor device)
+                             (let ((default (call-with-input-file resume
+                                              read-line)))
+                               ;; Don't waste time echoing ‘nothing’ to /sys.
+                               (if (string=? "0:0" default)
+                                   #f
+                                   default)))))
+        (when major:minor
+          (call-with-output-file resume ; may throw an ‘Invalid argument’
+            (cut display major:minor <>))))))) ; may never return
+
 (define* (make-disk-device-nodes base major #:optional (minor 0))
   "Make the block device nodes around BASE (something like \"/root/dev/sda\")
 with the given MAJOR number, starting with MINOR."
@@ -504,6 +551,12 @@ upon error."
         (load-linux-modules-from-directory linux-modules
                                            linux-module-directory)
 
+        (unless (member "--noresume" args)
+          ;; Try to resume immediately after loading (storage) modules
+          ;; but before any on-disk file systems have been mounted.
+          (false-if-exception           ; failure is not fatal
+           (resume-if-hibernated (find-long-option "--resume" args))))
+
         (when keymap-file
           (let ((status (system* "loadkeys" keymap-file)))
             (unless (zero? status)
-- 
2.26.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

bug#40927: Close hibernation bugs

[Tobias Geerinckx-Rice via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 235 bytes --]

Hibernation has been supported on Guix Systems for a very long 
time now.

I'm not aware of any, but any failures to hibernate are probably 
configuration or hardware-specific and should be reported as new 
bugs.

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]