Re: [RFC PATCH 1/1] module: speed modprobe by adding name_crc to struct module

["Christophe Leroy (CS GROUP)" <chleroy@kernel.org>]

Le 23/01/2026 à 00:46, Jim Cromie a écrit :
> [Vous ne recevez pas souvent de courriers de jim.cromie@gmail.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> "modprobe foo" currently does strcmp on the name, this can be improved.
> 
> So this commit:
> 
> 1. adds name_crc to struct module
> 2. modpost.c computes the value and
> 3. outputs it for "modinfo foo" to see/use.
> 
> 4. adds hotpath to find_module_all()
>     this uses name_crc to do quick "name-check"
>     falls back to strcmp only to guard against collisions.
> 
> This should significantly reduce modprobe workload, and shorten module
> load-time.

Any numbers of how significant is the reduction ?

> 
> Since it alters struct module, its binary incompatible. This means:
> 
> 1. RFC for its wide "blast radius".
> 2. suitable for major version bump *only*
> 
> 3. it opens door for further struct module reorg, to:
>     a. segregate fields by "temperature"
>     b. pack out paholes.
>     c. improve cache locality (by reordering coldest on bottom)
>        name should be cold now.
>        bikeshedding is appropriate here.
> 
> NB: this isn't a substitute for CONFIG_MODULE_SIG.
> It reimplements crc_le(), doesn't reuse kernel's version.

Why not use the kernel's version ?

> 
> CC: Luis Chamberlain <mcgrof@kernel.org>
> CC: Petr Pavlu <petr.pavlu@suse.com>
> CC: Daniel Gomez <da.gomez@kernel.org>
> CC: Sami Tolvanen <samitolvanen@google.com>
> CC: Aaron Tomlin <atomlin@atomlin.com>
> CC: linux-modules@vger.kernel.org
> 
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> 
>   '#' will be ignored, and an empty message aborts the commit.
> ---
>   include/linux/module.h | 15 ++++++++-------
>   kernel/module/main.c   |  8 ++++++--
>   scripts/mod/modpost.c  | 18 ++++++++++++++++++
>   scripts/mod/modpost.h  |  6 +++++-
>   4 files changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index d80c3ea57472..4ea6c5ae3374 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -402,10 +402,18 @@ struct klp_modinfo {
> 
>   struct module {
>          enum module_state state;
> +       u32 name_hash;

In the subject you say "name_crc"

> 
>          /* Member of list of modules */
>          struct list_head list;
> 
> +       /* Sysfs stuff. */
> +       struct module_kobject mkobj;
> +       struct module_attribute *modinfo_attrs;
> +       const char *version;
> +       const char *srcversion;
> +       struct kobject *holders_dir;
> +

Shouldn't this move be another patch ?

>          /* Unique handle for this module */
>          char name[MODULE_NAME_LEN];
> 
> @@ -414,13 +422,6 @@ struct module {
>          unsigned char build_id[BUILD_ID_SIZE_MAX];
>   #endif
> 
> -       /* Sysfs stuff. */
> -       struct module_kobject mkobj;
> -       struct module_attribute *modinfo_attrs;
> -       const char *version;
> -       const char *srcversion;
> -       struct kobject *holders_dir;
> -
>          /* Exported symbols */
>          const struct kernel_symbol *syms;
>          const u32 *crcs;
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index d855f43a2be3..685218b2c5ef 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -39,6 +39,7 @@
>   #include <linux/mutex.h>
>   #include <linux/rculist.h>
>   #include <linux/uaccess.h>
> +#include <linux/crc32.h>
>   #include <asm/cacheflush.h>
>   #include <linux/set_memory.h>
>   #include <asm/mmu_context.h>
> @@ -431,13 +432,16 @@ struct module *find_module_all(const char *name, size_t len,
>                                 bool even_unformed)
>   {
>          struct module *mod;
> +       u32 incoming_name_hash = crc32_le(0, name, len);
> 
>          list_for_each_entry_rcu(mod, &modules, list,
>                                  lockdep_is_held(&module_mutex)) {
>                  if (!even_unformed && mod->state == MODULE_STATE_UNFORMED)
>                          continue;
> -               if (strlen(mod->name) == len && !memcmp(mod->name, name, len))
> -                       return mod;
> +               if (mod->name_hash == incoming_name_hash) {
> +                       if (strlen(mod->name) == len && !memcmp(mod->name, name, len))
> +                               return mod;
> +               }

Why not just adding the following instead of modifing existing test:

	if (mod->name_hash != incoming_name_hash)
		continue;

>          }
>          return NULL;
>   }
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 755b842f1f9b..ae90e0bf9330 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -21,6 +21,22 @@
>   #include <stdbool.h>
>   #include <errno.h>
> 
> +/* Local CRC32 implementation for modpost.c */
> +#define CRCPOLY_LE 0xEDB88320
> +
> +typedef uint32_t u32;
> +
> +static u32 crc32_le(u32 crc,  char  *p, size_t len)
> +{
> +       int i;
> +       while (len--) {
> +               crc ^= *p++;
> +               for (i = 0; i < 8; i++)
> +                       crc = (crc >> 1) ^ ((crc & 1) ? CRCPOLY_LE : 0);
> +       }
> +       return crc;
> +}
> +

Why do you re-implement crc32_le() ?

>   #include <hash.h>
>   #include <hashtable.h>
>   #include <list.h>
> @@ -1581,6 +1597,7 @@ static void read_symbols(const char *modname)
> 
>          /* strip trailing .o */
>          mod = new_module(modname, strlen(modname) - strlen(".o"));
> +       mod->name_hash = crc32_le(0, mod->name, strlen(mod->name));
> 
>          /* save .no_trim_symbol section for later use */
>          if (info.no_trim_symbol_len) {
> @@ -1834,6 +1851,7 @@ static void add_header(struct buffer *b, struct module *mod)
>          buf_printf(b, "#include <linux/compiler.h>\n");
>          buf_printf(b, "\n");
>          buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
> +       buf_printf(b, "MODULE_INFO(name_crc, \"0x%08x\");\n", mod->name_hash);
>          buf_printf(b, "\n");
>          buf_printf(b, "__visible struct module __this_module\n");
>          buf_printf(b, "__section(\".gnu.linkonce.this_module\") = {\n");
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 2aecb8f25c87..3fc3cfd0a039 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -11,11 +11,14 @@
>   #include <fcntl.h>
>   #include <unistd.h>
>   #include <elf.h>
> +#include <stdint.h>
>   #include "../../include/linux/module_symbol.h"
> 
>   #include <list_types.h>
>   #include "elfconfig.h"
> 
> +typedef uint32_t u32;
> +
>   /* On BSD-alike OSes elf.h defines these according to host's word size */
>   #undef ELF_ST_BIND
>   #undef ELF_ST_TYPE
> @@ -126,7 +129,8 @@ struct module {
>          bool seen;
>          bool has_init;
>          bool has_cleanup;
> -       char         srcversion[25];
> +               char         srcversion[25];
> +               u32          name_hash;
>          // Missing namespace dependencies
>          struct list_head missing_namespaces;
>          // Actual imported namespaces
> --
> 2.52.0
> 
>