Subject: Re: [bug#42123] [PATCH] linux-libre: Enable module compression.

From: Ludovic Courtès <[email-masked]>
Date: Thu, 02 Jul 2020 12:23:01 +0200

Hi!

Mathieu Othacehe <[email-masked]> skribis:

> This commit enables GZIP compression for linux-libre kernel modules, reducing
> the size of linux-libre by 63% (165MB). The initrd modules are kept
> uncompressed as the initrd is already compressed as a whole.
>
> The linux-libre kernel also supports XZ compression, but as Guix does not have
> any available bindings for now, and the compression time is far more
> significant, GZIP seems to be a better option.
>
> * gnu/packages/aux-files/linux-libre/5.4-arm.conf: Enable GZ compression.
> * gnu/packages/aux-files/linux-libre/5.4-arm64.conf: Ditto.
> * gnu/packages/aux-files/linux-libre/5.4-i686.conf: Ditto.
> * gnu/packages/aux-files/linux-libre/5.4-x86_64.conf: Ditto.
> * gnu/build/linux-modules.scm (modinfo-section-contents): Use
> 'call-with-gzip-input-port' to read from a module file using '.gz' extension,
> (strip-extension): new procedure,
> (dot-ko): adapt to support compression,
> (ensure-dot-ko): ditto,
> (file-name->module-name): ditto,
> (find-module-file): ditto,
> (load-linux-module*): ditto,
> (module-name->file-name/guess): ditto,
> (module-name-lookup): ditto,
> (write-module-name-database): ditto,
> (write-module-alias-database): ditto,
> (write-module-device-database): ditto.
> * gnu/system/linux-initrd.scm (flat-linux-module-directory): Make sure that
> zlib bindings are available because they may be used in
> 'write-module-device-database'. Also make sure that the initrd only contains
> uncompressed module files.

Nice!

I do think that gzip is more appropriate than xz here, also in terms of
memory requirements.

Perhaps you can do this in two patches: first the linux-initrd bits, and
2nd the linux-libre changes.

> diff --git a/gnu/build/linux-modules.scm b/gnu/build/linux-modules.scm
> index aa1c7cfeae..7c6945a881 100644
> --- a/gnu/build/linux-modules.scm
> +++ b/gnu/build/linux-modules.scm
> @@ -21,6 +21,7 @@
>  (define-module (gnu build linux-modules)
>    #:use-module (guix elf)
>    #:use-module (guix glob)
> +  #:use-module (guix zlib)
>    #:use-module (guix build syscalls)
>    #:use-module ((guix build utils) #:select (find-files invoke))
>    #:use-module (guix build union)
> @@ -97,7 +98,16 @@ string list."
>  (define (modinfo-section-contents file)
>    "Return the contents of the '.modinfo' section of FILE as a list of
>  key/value pairs.."
> -  (let* ((bv      (call-with-input-file file get-bytevector-all))
> +  (define (get-bytevector file)
> +    (cond
> +     ((string-contains file ".ko.gz")

‘string-suffix?’ would be more accurate.

> +      (call-with-input-file file
> +        (lambda (port)
> +          (call-with-gzip-input-port port get-bytevector-all))))

‘call-with-input-file’ creates a buffered port, which could be
problematic, although ‘make-gzip-input-port’ checks that.

To be safe, you’d do (open-file file "r0") with a dynwind.

> +(define* (module-name->file-name/guess directory name
> +                                       #:key compression)
>    "Guess the file name corresponding to NAME, a module name.  That doesn't
>  always work because sometimes underscores in NAME map to hyphens (e.g.,
>  \"input-leds.ko\"), sometimes not (e.g., \"mac_hid.ko\")."

Please mention COMPRESSION in the docstring.

>  (define (write-module-alias-database directory)
> -  "Traverse the '.ko' files in DIRECTORY and create the corresponding
> +  "Traverse the '.ko[.gz|.xz]' files in DIRECTORY and create the corresponding
>  'modules.alias' file."
>    (define aliases
>      (map (lambda (file)
>             (cons (file-name->module-name file) (module-aliases file)))
> -         (find-files directory "\\.ko$")))
> +         (find-files directory "\\.ko.*$")))

Should we refine this regexp (there are a couple of places like this)?

There other Scheme bits LGTM!

(I don’t really know about Linux-libre but you do!)

Ludo’.