unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Jean-Baptiste Note <jean-baptiste.note@m4x.org>
To: Danny Milosavljevic <dannym@scratchpost.org>
Cc: 40927@debbugs.gnu.org
Subject: [bug#40927] [PATCH] Allow resume from swap device during boot
Date: Tue, 05 May 2020 18:07:15 +0000	[thread overview]
Message-ID: <87d07ixnl8.fsf@m4x.org> (raw)
In-Reply-To: <20200501165045.3d845900@scratchpost.org> (Danny Milosavljevic's message of "Fri, 1 May 2020 16:50:45 +0200")


[-- 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 --]

  reply	other threads:[~2020-05-05 18:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-28  7:26 [bug#40927] [PATCH] Allow resume from swap device during boot Jean-Baptiste Note
2020-05-01 14:50 ` Danny Milosavljevic
2020-05-05 18:07   ` Jean-Baptiste Note [this message]
2020-05-05 19:29     ` Jean-Baptiste Note
2020-05-01 16:35 ` Tobias Geerinckx-Rice via Guix-patches via
2020-05-03 11:05   ` Tobias Geerinckx-Rice via Guix-patches via
2020-05-07 20:58 ` [bug#40927] Subject=[bug#40927] " Jean-Baptiste Note
2021-05-09 19:34 ` bug#40927: Close hibernation bugs Tobias Geerinckx-Rice via Guix-patches via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d07ixnl8.fsf@m4x.org \
    --to=jean-baptiste.note@m4x.org \
    --cc=40927@debbugs.gnu.org \
    --cc=dannym@scratchpost.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).