bug#34902: guix cannot find a module on boot

bug#34902: guix cannot find a module on boot

[Julien Lepiller]

Hi!

I've installed the Guix system on my cubietruck yesterday, but had some 
difficulties. At first, the root partition was not available at boot, so 
I got a repl with a message saying that /dev/mmcblk0p1 was not 
available. I added the following to my guix config file:

(initrd-modules (cons "sunxi_mmc" %base-initrd-modules))

although the guix system command worked, the produced system still 
couldn't boot, but this time the message was that guix was unable to 
find sunxi_mmc.ko. Using the repl I could confirm that this module was 
indeed here, but under the name sunxi-mmc.ko. Using load-linux-module*, 
I was able to confirm that loading it made the filesystem available in 
/dev. In the end this line:

(initrd-modules (cons "sunxi-mmc" %base-initrd-modules))

was the right line to add, and I could properly boot my cubietruck!

The bug here is that guix should either be smarter and load sunxi-mmc.ko 
when it can't find sunxi_mmc.ko, or not allow me to build a system when 
I specify sunxi_mmc since it doesn't exist at boot time.

bug#34902: guix cannot find a module on boot

[Ludovic Courtès]

Hi Julien,

Julien Lepiller <julien@lepiller.eu> skribis:

> I've installed the Guix system on my cubietruck yesterday, but had
> some difficulties. At first, the root partition was not available at
> boot, so I got a repl with a message saying that /dev/mmcblk0p1 was
> not available. I added the following to my guix config file:
>
> (initrd-modules (cons "sunxi_mmc" %base-initrd-modules))

Didn’t ‘guix system init’ suggest adding this module?  Could it be that
this module was actually built-in in the kernel you booted?

> although the guix system command worked, the produced system still
> couldn't boot, but this time the message was that guix was unable to
> find sunxi_mmc.ko. Using the repl I could confirm that this module was
> indeed here, but under the name sunxi-mmc.ko. Using
> load-linux-module*, I was able to confirm that loading it made the
> filesystem available in /dev. In the end this line:
>
> (initrd-modules (cons "sunxi-mmc" %base-initrd-modules))
>
> was the right line to add, and I could properly boot my cubietruck!

Phewww.

> The bug here is that guix should either be smarter and load
> sunxi-mmc.ko when it can't find sunxi_mmc.ko, or not allow me to build
> a system when I specify sunxi_mmc since it doesn't exist at boot time.

This underscore vs. hyphen thing is terrible.

In commit fcd068e984078ab74c6842af2525bf88096cd262 we fixed the initrd
builder so it would try both file underscore and hyphen.

But now I suppose we need to do the same in ‘load-linux-module*’?

Thanks,
Ludo’.

bug#34902: guix cannot find a module on boot

[Julien Lepiller]

Le Mon, 18 Mar 2019 21:42:29 +0100,
Ludovic Courtès <ludo@gnu.org> a écrit :

> Hi Julien,
> 
> Julien Lepiller <julien@lepiller.eu> skribis:
> 
> > I've installed the Guix system on my cubietruck yesterday, but had
> > some difficulties. At first, the root partition was not available at
> > boot, so I got a repl with a message saying that /dev/mmcblk0p1 was
> > not available. I added the following to my guix config file:
> >
> > (initrd-modules (cons "sunxi_mmc" %base-initrd-modules))  
> 
> Didn’t ‘guix system init’ suggest adding this module?  Could it be
> that this module was actually built-in in the kernel you booted?

I couldn't find it in the output of lsmod, so I think it was builtin.

> 
> > although the guix system command worked, the produced system still
> > couldn't boot, but this time the message was that guix was unable to
> > find sunxi_mmc.ko. Using the repl I could confirm that this module
> > was indeed here, but under the name sunxi-mmc.ko. Using
> > load-linux-module*, I was able to confirm that loading it made the
> > filesystem available in /dev. In the end this line:
> >
> > (initrd-modules (cons "sunxi-mmc" %base-initrd-modules))
> >
> > was the right line to add, and I could properly boot my
> > cubietruck!  
> 
> Phewww.
> 
> > The bug here is that guix should either be smarter and load
> > sunxi-mmc.ko when it can't find sunxi_mmc.ko, or not allow me to
> > build a system when I specify sunxi_mmc since it doesn't exist at
> > boot time.  
> 
> This underscore vs. hyphen thing is terrible.
> 
> In commit fcd068e984078ab74c6842af2525bf88096cd262 we fixed the initrd
> builder so it would try both file underscore and hyphen.
> 
> But now I suppose we need to do the same in ‘load-linux-module*’?

I guess so :)

> 
> Thanks,
> Ludo’.

bug#34902: guix cannot find a module on boot

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]

> This underscore vs. hyphen thing is terrible.

Yeah.  But looking at kmod, it doesn't seem to be so bad for them.

They basically always use the version with underscores as the lookup
key for their internal hash tables.

For alias resolving, they use a resolver for [a-zxxx] AND they replace
dashes THAT ARE NOT IN THE "[...]" block by underscore before using it
for lookups.

modules.dep entries are always file names (no replacements whatsoever).

When a module is loaded as a result of an alias lookup, it has a special
key (string-append name "\" target-alias).  No idea what that's for.

I'd caution about complicating our version overly much by putting
just-in-case replacements everywhere.  Rather we should find out what
their original strategy is (if any :P) and do that as well.

Module aliases (the alias itself, not what it's resolved to) sometimes look
like that:

./arch/x86/crypto/des3_ede_glue.c:MODULE_ALIAS_CRYPTO("des3_ede-asm");

WTF...

But the thing the alias is resolved to is always the modname (which
has hyphens replaced by underscore and stops before a dot).

Their bin files (we don't use them) always have the final modname as key,
as opposed to their source files (for example "modules.alias" rather
than "modules.alias.bin").  (The source files sometimes have paths as key)

I think that they assume that each module, whether it has dependencies or
not, is listed in modules.dep .  They use modules.dep.bin for the lookup
of a module by modname, rather than by path.

So I think a pretty good fix is:

Every time we want to lookup a module by modname, check a hashtable that
is built from modules.dep by:
 
* let x-path be the first value of each modules.dep line.
* Then the key for the hashtable entry is (underscore (stop-at-dot (basename x-path)))
* And the value for the hashtable entry is x-path.

In this way we would look up modules in a way similar to them.

Why they couldn't just have used file names without mangling them beats me.
Sigh...

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#34902: guix cannot find a module on boot

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

Hi Danny,

Thanks for the explanation, I was unaware of all these subtleties…

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> Every time we want to lookup a module by modname, check a hashtable that
> is built from modules.dep by:
>  
> * let x-path be the first value of each modules.dep line.
> * Then the key for the hashtable entry is (underscore (stop-at-dot (basename x-path)))
> * And the value for the hashtable entry is x-path.
>
> In this way we would look up modules in a way similar to them.

Instead of guessing, we could also store a mapping from module name to
file name.  I had that in a previous patch series that we discussed some
time ago (see below.)

This a custom format, not used by kmod or any other tool, but the
advantage is that it solves the module/file name mapping without ugly
guess-hacks.

WDYT?

Thanks,
Ludo’.


[-- Attachment #2: Type: text/x-patch, Size: 5404 bytes --]

commit 370c4426aa94d99f8faafd3992d54169ff918fb1
Author: Ludovic Courtès <ludo@gnu.org>
Date:   Wed Mar 14 23:11:01 2018 +0100

    linux-modules: Define and use a module name database.
    
    * gnu/build/linux-modules.scm (module-formal-name): New procedure.
    (load-linux-modules-from-directory)[lookup-module]: Remove.
    [module-name->file-name]: New variable.  Use it.
    (module-name->file-name/guess, module-name-lookup)
    (write-module-name-database): New procedures.
    * gnu/system/linux-initrd.scm (flat-linux-module-directory): Call
    'write-module-name-database'.

diff --git a/gnu/build/linux-modules.scm b/gnu/build/linux-modules.scm
index 615e9e1d07..7e99f62941 100644
--- a/gnu/build/linux-modules.scm
+++ b/gnu/build/linux-modules.scm
@@ -31,8 +31,10 @@
   #:use-module (ice-9 match)
   #:use-module (ice-9 rdelim)
   #:use-module (ice-9 ftw)
+  #:autoload   (ice-9 pretty-print) (pretty-print)
   #:export (dot-ko
             ensure-dot-ko
+            module-formal-name
             module-aliases
             module-dependencies
             recursive-module-dependencies
@@ -46,6 +48,7 @@
             device-module-aliases
             known-module-aliases
             matching-modules
+            write-module-name-database
             write-module-alias-database
             write-module-device-database
             load-needed-linux-modules))
@@ -95,6 +98,14 @@ key/value pairs.."
 (define %not-comma
   (char-set-complement (char-set #\,)))
 
+(define (module-formal-name file)
+  "Return the module name of FILE as it appears in its info section.  Usually
+the module name is the same as the base name of FILE, modulo hyphens and minus
+the \".ko\" extension."
+  (match (assq 'name (modinfo-section-contents file))
+    (('name . name) name)
+    (#f #f)))
+
 (define (module-dependencies file)
   "Return the list of modules that FILE depends on.  The returned list
 contains module names, not actual file names."
@@ -240,12 +251,13 @@ appears in BLACK-LIST are not loaded."
   "Load MODULES and their dependencies from DIRECTORY, a directory containing
 the '.ko' files.  The '.ko' suffix is automatically added to MODULES if
 needed."
-  (define (lookup-module name)
-    (string-append directory "/" (ensure-dot-ko name)))
+  (define module-name->file-name
+    (module-name-lookup directory))
 
-  (for-each (cut load-linux-module* <>
-                 #:lookup-module lookup-module)
-            (map lookup-module modules)))
+  (for-each (lambda (module)
+              (load-linux-module* (module-name->file-name module)
+                                  #:lookup-module module-name->file-name))
+            modules))
 
 \f
 ;;;
@@ -401,6 +413,47 @@ ALIAS is a string like \"scsi:t-0x00\" as returned by
                       module)))
               known-aliases))
 
+(define (module-name->file-name/guess directory name)
+  "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\")."
+  (string-append directory "/" (ensure-dot-ko name)))
+
+(define (module-name-lookup directory)
+  "Return a one argument procedure that takes a module name (e.g.,
+\"input_leds\") and returns its absolute file name (e.g.,
+\"/.../input-leds.ko\")."
+  (catch 'system-error
+    (lambda ()
+      (define mapping
+        (call-with-input-file (string-append directory "/modules.name")
+          read))
+
+      (lambda (name)
+        (or (assoc-ref mapping name)
+            (module-name->file-name/guess directory name))))
+    (lambda args
+      (if (= ENOENT (system-error-errno args))
+          (cut module-name->file-name/guess directory <>)
+          (apply throw args)))))
+
+(define (write-module-name-database directory)
+  "Write a database that maps \"module names\" as they appear in the relevant
+ELF section of '.ko' files, to actual file names.  This format is
+Guix-specific.  It aims to deal with inconsistent naming, in particular
+hyphens vs. underscores."
+  (define mapping
+    (map (lambda (file)
+           (match (module-formal-name file)
+             (#f   (cons (basename file ".ko") file))
+             (name (cons name file))))
+         (find-files directory "\\.ko$")))
+
+  (call-with-output-file (string-append directory "/modules.name")
+    (lambda (port)
+      (display ";; Module name to file name mapping.\n" port)
+      (pretty-print mapping port))))
+
 (define (write-module-alias-database directory)
   "Traverse the '.ko' files in DIRECTORY and create the corresponding
 'modules.alias' file."
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index 4379fb461e..9160d94847 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -242,6 +242,9 @@ MODULES and taken from LINUX."
           (write-module-alias-database #$output)
           (write-module-device-database #$output)
 
+          ;; Hyphen or underscore?  This database tells us.
+          (write-module-name-database #$output)
+
           ;; Copy "modules.builtin", which we need to determine whether a
           ;; module needs to be loaded.
           (match (find-files #$linux (string->regexp "modules.builtin"))

bug#34902: guix cannot find a module on boot

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

Maybe I'm too paranoid but can we have "guix" in the file name "modules.name"
somewhere?  Otherwise I see it coming that upstream uses modules.name for an
incompatible purpose and then we'd be with a guix interface that's broken
and/or break their interface.

(So much complexity for something so silly.  Honestly, I feel like E-mailing
the upstream author and telling him what I think.  WTF :P)

Should we warn when we use the fallback?  I like the defensive programming
but I feel we shouldn't have it *silently* fall back when the database is
broken/missing.

Otherwise LGTM!

>(basename file ".ko")
               ^^^^^^

Nice.  Where was that all my life.  ;-)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#34902: guix cannot find a module on boot

[Ludovic Courtès]

Hello!

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> Maybe I'm too paranoid but can we have "guix" in the file name "modules.name"
> somewhere?  Otherwise I see it coming that upstream uses modules.name for an
> incompatible purpose and then we'd be with a guix interface that's broken
> and/or break their interface.
>
> (So much complexity for something so silly.  Honestly, I feel like E-mailing
> the upstream author and telling him what I think.  WTF :P)
>
> Should we warn when we use the fallback?  I like the defensive programming
> but I feel we shouldn't have it *silently* fall back when the database is
> broken/missing.
>
> Otherwise LGTM!

So I went ahead and pushed these patches, derived from our beautiful
patch set at <https://issues.guix.gnu.org/issue/30604>:

  c85ccf60bf linux-modules: Define and use a module name database.
  e1a9a7f275 linux-modules: Add 'load-linux-modules-from-directory'.
  2a693b69ca linux-modules: Add "modules.devname" writer.
  4f8b9d1a6f linux-modules: Add "modules.alias" writer.

The actual fix for the hyphen/underscore mismatch that Julien reported
is commit c85ccf60bf.  The “modules.devname” and “modules.alias” are
actually unused so far but (1) it was easier to preserve them, and (2)
that’ll give us an incentive to finish
<https://issues.guix.gnu.org/issue/30604>.  :-)

I added an explicit comment that “modules.name” uses a Guix-specific
format.  We can always rename it if the kernel folks decide to acquire
that file name.

Julien, could you please confirm that your initial issue is fixed?

Thanks,
Ludo’.

bug#34902: Guix cannot find a module on boot

[Julien Lepiller]

Hi Ludo,

Your patch has a LGTM, but I don't see it on master. Would you like to push it, or is there a reason why you didn't do it yet?

Thanks!