unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
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 --]

  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).