From: Marius Bakke <marius@gnu.org>
To: Efraim Flashner <efraim@flashner.co.il>, 42478@debbugs.gnu.org
Cc: Efraim Flashner <efraim@flashner.co.il>
Subject: [bug#42478] [PATCH] services: Add zram-device-service.
Date: Sat, 25 Jul 2020 19:00:54 +0200 [thread overview]
Message-ID: <87eeozsf5l.fsf@gnu.org> (raw)
In-Reply-To: <20200722181025.29413-1-efraim@flashner.co.il>
[-- Attachment #1: Type: text/plain, Size: 3361 bytes --]
Efraim Flashner <efraim@flashner.co.il> writes:
> * gnu/services/linux.scm (<zram-device-configuration>): New record.
> (zram-device-service-type): New variable.
> * doc/guix.texi (Linux Services): Document it.
> * tests/services/linux.scm (zram-device-configuration->udev-string): New
> test.
[...]
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 8696a9b554..f656c31fab 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -27127,6 +27127,51 @@ parameters, can be done as follow:
> @end lisp
> @end deffn
>
> +@cindex zram
> +@cindex compressed swap
> +@cindex Compressed RAM-based block devices
> +@subsubheading Zram Device Service
> +
> +The Zram device service provides a compressed swap device in system
> +memory. The Linux Kernel documentation has more information about
> +@uref{https://www.kernel.org/doc/html/latest/admin-guide/blockdev/zram.html,zram}
> +devices.
> +
> +@deffn {Scheme Variable} zram-device-service-type
> +This service creates the zram block device, formats it as swap and
> +enables it as a swap device. The service's value is a
> +@code{zram-device-configuration} record.
> +
> +@deftp {Data Type} zram-device-configuration
> +This is the data type representing the configuration for the zram-device
> +service.
> +
> +@table @asis
> +@item @code{disksize} (default @var{"0"})
> +This is the amount of space you wish to provide for the zram device. It
> +accepts a string and can be a number of bytes or use a suffix, eg.:
> +@var{2G}.
Perhaps this could accept both an integer and a string? What does a
size 0 device do, would it make sense to not have a default here?
Also, would it make sense to name it "size" instad of "disksize"?
> +@item @code{comp_algorithm} (default @var{"lzo"})
> +This is the compression algorithm you wish to use. It is difficult to
> +list all the possible compression options, but common ones supported by
> +Guix's Linux Libre Kernel include @var{lzo}, @var{lz4} and @var{zstd}.
"comp_algorithm" is not very idiomatic :-)
Either "compression-algorithm" or just "compression" IMO. I'd also
prefer a symbol instead of a string (or both!), but no strong opinion.
> +@item @code{mem_limit} (default @var{"0"})
"mem_limit" should instead be "memory-limit" or just "limit".
Accepting an integer here too would be nice! :-)
> +This is the maximum amount of memory which the zram device can use.
> +Setting it to '0' disables the limit. While it is generally expected
> +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.: @var{2G}.
> +@item @code{priority} (default @var{"-1"})
Just an integer I suppose?
> +\f
> +;;;
> +;;; Zram swap device.
> +;;;
> +
> +(define zram-device-configuration->udev-string
> + (@@ (gnu services linux) zram-device-configuration->udev-string))
Would it make sense to export this, or is it strictly for internal use?
Great that you were able to add unit tests for the functionality. I
think in this case we could also have a system test that checks that a
zram device was created? But it can come later.
Other than the cosmetic/idiomatic issues looks great to me!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]
next prev parent reply other threads:[~2020-07-25 17:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-22 18:10 [bug#42478] [PATCH] services: Add zram-device-service Efraim Flashner
2020-07-25 17:00 ` Marius Bakke [this message]
2020-07-27 12:01 ` Efraim Flashner
2020-07-27 12:03 ` [bug#42478] [PATCH v2] " Efraim Flashner
2020-08-02 12:56 ` bug#42478: " Efraim Flashner
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=87eeozsf5l.fsf@gnu.org \
--to=marius@gnu.org \
--cc=42478@debbugs.gnu.org \
--cc=efraim@flashner.co.il \
/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).