ZRAM default priority wrong

  • Done
  • quality assurance status badge
Details
3 participants
  • Stefan Baums
  • Josselin Poiret
  • Maxim Cournoyer
Owner
unassigned
Submitted by
Stefan Baums
Severity
normal
S
S
Stefan Baums wrote on 8 Apr 2022 04:27
(address . bug-guix@gnu.org)
878rsgfgxg.fsf@lmu.de
I have a swap file and zram configured without specifying
priority. The swap file was assigned -2, and zram -3, with the
effect that zram was never used. I had to manually change the zram
priority to something higher that that of the swap file.

The manual wrongly claims that the default priority for zram is
-1. The same paragraph of the manual also says:

‘swapon’ accepts values between -1 and 32767, with higher
values indicating higher priority.

which makes the -2 priority of the swap file look suspect (though
it seems to work).
J
J
Josselin Poiret wrote on 21 Apr 2022 09:57
87sfq6g96j.fsf@jpoiret.xyz
Hello Stefan and sorry for the late reply,

Stefan Baums <baums@stefanbaums.com> writes:

Toggle quote (9 lines)
> The manual wrongly claims that the default priority for zram is
> -1. The same paragraph of the manual also says:
>
> ‘swapon’ accepts values between -1 and 32767, with higher
> values indicating higher priority.
>
> which makes the -2 priority of the swap file look suspect (though
> it seems to work).

This issue comes from the inconsistency with which swap priorities are
specified on Linux: `swapon` the binary from util-linux (man 8 swapon)
has roughly the same description as above, but internally uses the
syscall wrapper `swapon` from libc (man 2 swapon), which has a better
description of priorities. The official interpretation of priorities
for the syscall is summarized in the "Swap space" part of the Guix
manual:

A swap space can have no priority, or a priority specified as an integer
between 0 and 32767. The kernel will first use swap spaces of higher
priority when paging, and use same priority spaces on a round-robin
basis. The kernel will use swap spaces without a set priority after
prioritized spaces, and in the order that they appeared in (not
round-robin).

Note that these are called high (for specified priority) or low
(unspecified) priority in `man 2 swapon` but that's a bit of a
misnomer. So, `swapon` the binary actually maps -1 to no (low)
priority, and there actually is no -1 priority in the Linux kernel ABI!
Since your actual swap file is swapon'd first, and also has no priority
set, according to the rules above it will be used before the ZRAM which
also has no priority.

On the Guix side of things, we should really be using the same interface
as swap-space, as I think the distinction between #f and 0 to 32767 is
clearer. I'll send some patches that adress this soon, along with the
zram-service-type documentation.

Best,
--
Josselin Poiret
S
S
Stefan Baums wrote on 21 Apr 2022 10:06
(name . Josselin Poiret)(address . dev@jpoiret.xyz)(address . 54783@debbugs.gnu.org)
874k2moo6n.fsf@stefanbaums.com
Dear Josselin,

thank you very much! That should clarify things. I have been
happily using ZRAM on Guix since I figured it out.

All best,
Stefan
J
J
Josselin Poiret wrote on 21 Apr 2022 15:49
[PATCH 2/2] doc: Remove double copyright.
(name . Josselin Poiret)(address . dev@jpoiret.xyz)
20220421134921.1505-2-dev@jpoiret.xyz
* doc/guix.texi: Remove doubled Josselin Poiret copyright line.
---
doc/guix.texi | 1 -
1 file changed, 1 deletion(-)

Toggle diff (14 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 31f391357d..d9b26e440c 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -99,7 +99,6 @@ Copyright @copyright{} 2021 Alice Brenon@*
Copyright @copyright{} 2021, 2022 Josselin Poiret@*
Copyright @copyright{} 2021 Andrew Tropin@*
Copyright @copyright{} 2021 Sarah Morgensen@*
-Copyright @copyright{} 2021 Josselin Poiret@*
Copyright @copyright{} 2022 Remco van 't Veer@*
Copyright @copyright{} 2022 Aleksandr Vityazev@*
--
2.34.0
J
J
Josselin Poiret wrote on 21 Apr 2022 15:49
[PATCH 1/2] system: Align zram priority with swap-space spec to clarify.
(name . Josselin Poiret)(address . dev@jpoiret.xyz)
20220421134921.1505-1-dev@jpoiret.xyz
* gnu/services/linux.scm
(zram-device-configuration) [priority]: Adapt to use #f or an integer
from 0 to 32767. Add sanitizer to warn for the change and delay the
field.
(zram-device-configuration->udev-string): Adapt as above.
* doc/guix.texi (Zram Device Service): Change priority description to
refer to the Swap Space one, and suggest not leaving the default #f on
to properly use zram.
---
doc/guix.texi | 10 +++++-----
gnu/services/linux.scm | 26 +++++++++++++++++++++++---
2 files changed, 28 insertions(+), 8 deletions(-)

Toggle diff (92 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index b7005f0ef1..31f391357d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -96,7 +96,7 @@ Copyright @copyright{} 2021 Domagoj Stolfa@*
Copyright @copyright{} 2021 Hui Lu@*
Copyright @copyright{} 2021 pukkamustard@*
Copyright @copyright{} 2021 Alice Brenon@*
-Copyright @copyright{} 2021 Josselin Poiret@*
+Copyright @copyright{} 2021, 2022 Josselin Poiret@*
Copyright @copyright{} 2021 Andrew Tropin@*
Copyright @copyright{} 2021 Sarah Morgensen@*
Copyright @copyright{} 2021 Josselin Poiret@*
@@ -34650,11 +34650,11 @@ that compression will be 2:1, it is possible that uncompressable data
can be written to swap and this is a method to limit how much memory can
be used. It accepts a string and can be a number of bytes or use a
suffix, eg.: @code{"2G"}.
-@item @code{priority} (default @code{-1})
+@item @code{priority} (default @code{#f})
This is the priority of the swap device created from the zram device.
-@code{swapon} accepts values between -1 and 32767, with higher values
-indicating higher priority. Higher priority swap will generally be used
-first.
+@xref{Swap Space} for a description of swap priorities. You might want
+to set a specific priority for the zram device, otherwise it could end
+up not being used much for the reasons described there.
@end table
@end deftp
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index 2eb02ac5a3..9f598b2826 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -4,6 +4,7 @@
;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2021 raid5atemyhomework <raid5atemyhomework@protonmail.com>
;;; Copyright © 2021 B. Wilson <elaexuotee@wilsonb.com>
+;;; Copyright © 2022 Josselin Poiret <dev@jpoiret.xyz>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -21,9 +22,11 @@
;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
(define-module (gnu services linux)
+ #:use-module (guix diagnostics)
#:use-module (guix gexp)
#:use-module (guix records)
#:use-module (guix modules)
+ #:use-module (guix i18n)
#:use-module (gnu services)
#:use-module (gnu services base)
#:use-module (gnu services shepherd)
@@ -252,7 +255,19 @@ (define-record-type* <zram-device-configuration>
(memory-limit zram-device-configuration-memory-limit
(default 0)) ; string or integer
(priority zram-device-configuration-priority
- (default -1))) ; integer
+ (default #f) ; integer | #f
+ (delayed)
+ (sanitize warn-zram-priority-change)))
+
+(define-with-syntax-properties
+ (warn-zram-priority-change (priority properties))
+ (if (eqv? priority -1)
+ (begin
+ (warning (source-properties->location properties)
+ (G_ "Using -1 for zram priority is deprecated to align with \
+the corresponding swap-space field, please use #f from now on.~%"))
+ #f)
+ priority))
(define (zram-device-configuration->udev-string config)
"Translate a <zram-device-configuration> into a string which can be
@@ -278,8 +293,13 @@ (define (zram-device-configuration->udev-string config)
"")
"RUN+=\"/run/current-system/profile/sbin/mkswap /dev/zram0\" "
"RUN+=\"/run/current-system/profile/sbin/swapon "
- (if (not (equal? -1 priority))
- (string-append "--priority " (number->string priority) " ")
+ ;; XXX: The field is delayed while the deprecation warning remains in
+ ;; place, so we can't use match to fetch it (it would give a promise)
+ (if (zram-device-configuration-priority config)
+ (string-append "--priority "
+ (number->string
+ (zram-device-configuration-priority config))
+ " ")
"")
"/dev/zram0\"\n"))))
--
2.34.0
M
M
Maxim Cournoyer wrote on 24 May 2022 17:42
Re: bug#54783: ZRAM default priority wrong
(name . Josselin Poiret)(address . dev@jpoiret.xyz)
87o7zn3pkf.fsf_-_@gmail.com
Hi,

Josselin Poiret <dev@jpoiret.xyz> writes:

Toggle quote (74 lines)
> * gnu/services/linux.scm
> (zram-device-configuration) [priority]: Adapt to use #f or an integer
> from 0 to 32767. Add sanitizer to warn for the change and delay the
> field.
> (zram-device-configuration->udev-string): Adapt as above.
> * doc/guix.texi (Zram Device Service): Change priority description to
> refer to the Swap Space one, and suggest not leaving the default #f on
> to properly use zram.
> ---
> doc/guix.texi | 10 +++++-----
> gnu/services/linux.scm | 26 +++++++++++++++++++++++---
> 2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index b7005f0ef1..31f391357d 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -96,7 +96,7 @@ Copyright @copyright{} 2021 Domagoj Stolfa@*
> Copyright @copyright{} 2021 Hui Lu@*
> Copyright @copyright{} 2021 pukkamustard@*
> Copyright @copyright{} 2021 Alice Brenon@*
> -Copyright @copyright{} 2021 Josselin Poiret@*
> +Copyright @copyright{} 2021, 2022 Josselin Poiret@*
> Copyright @copyright{} 2021 Andrew Tropin@*
> Copyright @copyright{} 2021 Sarah Morgensen@*
> Copyright @copyright{} 2021 Josselin Poiret@*
> @@ -34650,11 +34650,11 @@ that compression will be 2:1, it is possible that uncompressable data
> can be written to swap and this is a method to limit how much memory can
> be used. It accepts a string and can be a number of bytes or use a
> suffix, eg.: @code{"2G"}.
> -@item @code{priority} (default @code{-1})
> +@item @code{priority} (default @code{#f})
> This is the priority of the swap device created from the zram device.
> -@code{swapon} accepts values between -1 and 32767, with higher values
> -indicating higher priority. Higher priority swap will generally be used
> -first.
> +@xref{Swap Space} for a description of swap priorities. You might want
> +to set a specific priority for the zram device, otherwise it could end
> +up not being used much for the reasons described there.
> @end table
>
> @end deftp
> diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
> index 2eb02ac5a3..9f598b2826 100644
> --- a/gnu/services/linux.scm
> +++ b/gnu/services/linux.scm
> @@ -4,6 +4,7 @@
> ;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
> ;;; Copyright © 2021 raid5atemyhomework <raid5atemyhomework@protonmail.com>
> ;;; Copyright © 2021 B. Wilson <elaexuotee@wilsonb.com>
> +;;; Copyright © 2022 Josselin Poiret <dev@jpoiret.xyz>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -21,9 +22,11 @@
> ;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
>
> (define-module (gnu services linux)
> + #:use-module (guix diagnostics)
> #:use-module (guix gexp)
> #:use-module (guix records)
> #:use-module (guix modules)
> + #:use-module (guix i18n)
> #:use-module (gnu services)
> #:use-module (gnu services base)
> #:use-module (gnu services shepherd)
> @@ -252,7 +255,19 @@ (define-record-type* <zram-device-configuration>
> (memory-limit zram-device-configuration-memory-limit
> (default 0)) ; string or integer
> (priority zram-device-configuration-priority
> - (default -1))) ; integer
> + (default #f) ; integer | #f
> + (delayed)

I'm curious, what does delaying the field buys us here? Is it to avoid
printing the warning multiple times when the record is evaluated?

Toggle quote (12 lines)
> + (sanitize warn-zram-priority-change)))
> +
> +(define-with-syntax-properties
> + (warn-zram-priority-change (priority properties))
> + (if (eqv? priority -1)
> + (begin
> + (warning (source-properties->location properties)
> + (G_ "Using -1 for zram priority is deprecated to align with \
> +the corresponding swap-space field, please use #f from now on.~%"))
> + #f)
> + priority))

By convention, a warning message should not be a complete sentence (no
capitalized first letter nor last period) and be short. To provide a
human friendly hint/message, you could use 'display-hint' (combined with
a more succinct warning).

The rest LGTM.

Maxim
J
J
Josselin Poiret wrote on 24 May 2022 19:16
[v2 0/2] Clarify zram priority
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20220524171631.25011-1-dev@jpoiret.xyz
Hello Maxim,

Thanks for looking at this. Here's a v2.

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
Toggle quote (3 lines)
> I'm curious, what does delaying the field buys us here? Is it to avoid
> printing the warning multiple times when the record is evaluated?

Right, and this would get worse if someone ended up inheriting from
another configuration. It would be bad UX IMO.

Toggle quote (5 lines)
> By convention, a warning message should not be a complete sentence (no
> capitalized first letter nor last period) and be short. To provide a
> human friendly hint/message, you could use 'display-hint' (combined with
> a more succinct warning).

Right, fixed!

Toggle quote (4 lines)
> The rest LGTM.
>
> Maxim

Josselin Poiret (2):
system: Align zram priority with swap-space spec to clarify.
doc: Remove double copyright.

doc/guix.texi | 11 +++++------
gnu/services/linux.scm | 29 ++++++++++++++++++++++++++---
2 files changed, 31 insertions(+), 9 deletions(-)

--
2.36.0
J
J
Josselin Poiret wrote on 24 May 2022 19:16
[PATCH v2 2/2] doc: Remove double copyright.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20220524171631.25011-3-dev@jpoiret.xyz
* doc/guix.texi: Remove doubled Josselin Poiret copyright line.
---
doc/guix.texi | 1 -
1 file changed, 1 deletion(-)

Toggle diff (14 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 5f0120a3bd..b960c546a8 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -99,7 +99,6 @@ Copyright @copyright{} 2021 Alice Brenon@*
Copyright @copyright{} 2021, 2022 Josselin Poiret@*
Copyright @copyright{} 2021 Andrew Tropin@*
Copyright @copyright{} 2021 Sarah Morgensen@*
-Copyright @copyright{} 2021 Josselin Poiret@*
Copyright @copyright{} 2022 Remco van 't Veer@*
Copyright @copyright{} 2022 Aleksandr Vityazev@*
Copyright @copyright{} 2022 Philip M@sup{c}Grath@*
--
2.36.0
J
J
Josselin Poiret wrote on 24 May 2022 19:16
[PATCH v2 1/2] system: Align zram priority with swap-space spec to clarify.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20220524171631.25011-2-dev@jpoiret.xyz
* gnu/services/linux.scm
(zram-device-configuration) [priority]: Adapt to use #f or an integer
from 0 to 32767. Add sanitizer to warn for the change and delay the
field.
(zram-device-configuration->udev-string): Adapt as above.
* doc/guix.texi (Zram Device Service): Change priority description to
refer to the Swap Space one, and suggest not leaving the default #f on
to properly use zram.
---
doc/guix.texi | 10 +++++-----
gnu/services/linux.scm | 29 ++++++++++++++++++++++++++---
2 files changed, 31 insertions(+), 8 deletions(-)

Toggle diff (95 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 184206bec8..5f0120a3bd 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -96,7 +96,7 @@ Copyright @copyright{} 2021 Domagoj Stolfa@*
Copyright @copyright{} 2021 Hui Lu@*
Copyright @copyright{} 2021 pukkamustard@*
Copyright @copyright{} 2021 Alice Brenon@*
-Copyright @copyright{} 2021 Josselin Poiret@*
+Copyright @copyright{} 2021, 2022 Josselin Poiret@*
Copyright @copyright{} 2021 Andrew Tropin@*
Copyright @copyright{} 2021 Sarah Morgensen@*
Copyright @copyright{} 2021 Josselin Poiret@*
@@ -35213,11 +35213,11 @@ that compression will be 2:1, it is possible that uncompressable data
can be written to swap and this is a method to limit how much memory can
be used. It accepts a string and can be a number of bytes or use a
suffix, eg.: @code{"2G"}.
-@item @code{priority} (default @code{-1})
+@item @code{priority} (default @code{#f})
This is the priority of the swap device created from the zram device.
-@code{swapon} accepts values between -1 and 32767, with higher values
-indicating higher priority. Higher priority swap will generally be used
-first.
+@xref{Swap Space} for a description of swap priorities. You might want
+to set a specific priority for the zram device, otherwise it could end
+up not being used much for the reasons described there.
@end table
@end deftp
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index 2eb02ac5a3..c6e460597a 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -4,6 +4,7 @@
;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2021 raid5atemyhomework <raid5atemyhomework@protonmail.com>
;;; Copyright © 2021 B. Wilson <elaexuotee@wilsonb.com>
+;;; Copyright © 2022 Josselin Poiret <dev@jpoiret.xyz>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -21,9 +22,12 @@
;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
(define-module (gnu services linux)
+ #:use-module (guix diagnostics)
#:use-module (guix gexp)
#:use-module (guix records)
#:use-module (guix modules)
+ #:use-module (guix i18n)
+ #:use-module (guix ui)
#:use-module (gnu services)
#:use-module (gnu services base)
#:use-module (gnu services shepherd)
@@ -252,7 +256,21 @@ (define-record-type* <zram-device-configuration>
(memory-limit zram-device-configuration-memory-limit
(default 0)) ; string or integer
(priority zram-device-configuration-priority
- (default -1))) ; integer
+ (default #f) ; integer | #f
+ (delayed) ; to avoid printing the deprecation
+ ; warning multiple times
+ (sanitize warn-zram-priority-change)))
+
+(define-with-syntax-properties
+ (warn-zram-priority-change (priority properties))
+ (if (eqv? priority -1)
+ (begin
+ (warning (source-properties->location properties)
+ (G_ "using -1 for zram priority is deprecated~%"))
+ (display-hint (G_ "Use #f or leave as default instead (@pxref{Linux \
+Services})."))
+ #f)
+ priority))
(define (zram-device-configuration->udev-string config)
"Translate a <zram-device-configuration> into a string which can be
@@ -278,8 +296,13 @@ (define (zram-device-configuration->udev-string config)
"")
"RUN+=\"/run/current-system/profile/sbin/mkswap /dev/zram0\" "
"RUN+=\"/run/current-system/profile/sbin/swapon "
- (if (not (equal? -1 priority))
- (string-append "--priority " (number->string priority) " ")
+ ;; XXX: The field is delayed while the deprecation warning remains in
+ ;; place, so we can't use match to fetch it (it would give a promise)
+ (if (zram-device-configuration-priority config)
+ (string-append "--priority "
+ (number->string
+ (zram-device-configuration-priority config))
+ " ")
"")
"/dev/zram0\"\n"))))
--
2.36.0
M
M
Maxim Cournoyer wrote on 11 Jun 2022 07:56
Re: bug#54783: ZRAM default priority wrong
(name . Josselin Poiret)(address . dev@jpoiret.xyz)
878rq3g2uh.fsf@gmail.com
Hi Josselin,

[...]
Toggle quote (6 lines)
>
> On the Guix side of things, we should really be using the same interface
> as swap-space, as I think the distinction between #f and 0 to 32767 is
> clearer. I'll send some patches that adress this soon, along with the
> zram-service-type documentation.

I've made this smallish change:

Toggle snippet (25 lines)
modified gnu/services/linux.scm
@@ -296,14 +296,12 @@ (define (zram-device-configuration->udev-string config)
"")
"RUN+=\"/run/current-system/profile/sbin/mkswap /dev/zram0\" "
"RUN+=\"/run/current-system/profile/sbin/swapon "
- ;; XXX: The field is delayed while the deprecation warning remains in
- ;; place, so we can't use match to fetch it (it would give a promise)
- (if (zram-device-configuration-priority config)
- (string-append "--priority "
- (number->string
- (zram-device-configuration-priority config))
- " ")
- "")
+ ;; TODO: Revert to simply use 'priority' after removing the deprecation
+ ;; warning and the delayed property of the field.
+ (let ((priority* (force priority)))
+ (if priority*
+ (format #f "--priority ~a " priority*)
+ ""))
"/dev/zram0\"\n"))))
(define %zram-device-config

Because the TODO comments seems more actionable for my future self, and
pushed as a99015c878.

Thanks Stefan for reporting the issue and for Josselin for fixing it!

Closing.

Maxim
Closed
?