diff mbox series

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

Message ID 87zhawax60.fsf@m4x.org
State Accepted
Headers show
Series [bug#40927] Allow resume from swap device during boot | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job

Commit Message

Jean-Baptiste Note April 28, 2020, 7:26 a.m. UTC
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

Comments

Danny Milosavljevic May 1, 2020, 2:50 p.m. UTC | #1
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?
dziltener--- via Guix-patches via May 1, 2020, 4:35 p.m. UTC | #2
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
Tobias Geerinckx-Rice May 9, 2021, 7:34 p.m. UTC | #3
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
diff mbox series

Patch

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