Re: [PATCH 14/27] modpost: dump Module.symvers in the same order of modules.order

From: Nick Desaulniers <ndesaulniers@google.com>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	Michal Marek <michal.lkml@markovi.net>
Subject: Re: [PATCH 14/27] modpost: dump Module.symvers in the same order of modules.order
Date: Tue, 26 Apr 2022 15:22:44 -0700	[thread overview]
Message-ID: <CAKwvOdmH3pqf5WHtdv-6eJgQvWQOz1BTMA33QLL7t1wGzOxNnA@mail.gmail.com> (raw)
In-Reply-To: <20220424190811.1678416-15-masahiroy@kernel.org>

On Sun, Apr 24, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> modpost dumps the exported symbols into Module.symvers, but currently
> in random order because it iterates in the hash table.
>
> Add a linked list of exported symbols in struct module, so we can
> iterate on symbols per module.
>
> This commit makes Module.symvers much more readable; the outer loop in
> write_dump() iterates over the modules in the order of modules.order,
> and the inner loop dumps symbols in each module.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
>  scripts/mod/modpost.c | 27 ++++++++++++---------------
>  scripts/mod/modpost.h |  1 +
>  2 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index e23e416213bf..1793396e1323 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -185,6 +185,7 @@ static struct module *new_module(const char *modname)
>         mod = NOFAIL(malloc(sizeof(*mod) + strlen(modname) + 1));
>         memset(mod, 0, sizeof(*mod));
>
> +       INIT_LIST_HEAD(&mod->exported_symbols);
>         INIT_LIST_HEAD(&mod->unresolved_symbols);
>         INIT_LIST_HEAD(&mod->missing_namespaces);
>         INIT_LIST_HEAD(&mod->imported_namespaces);
> @@ -407,6 +408,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
>
>         if (!s) {
>                 s = new_symbol(name, mod, export);
> +               list_add_tail(&s->list, &mod->exported_symbols);
>         } else if (!external_module || s->module->is_vmlinux ||
>                    s->module == mod) {
>                 warn("%s: '%s' exported twice. Previous export was in %s%s\n",
> @@ -2452,22 +2454,17 @@ static void read_dump(const char *fname)
>  static void write_dump(const char *fname)
>  {
>         struct buffer buf = { };
> -       struct symbol *symbol;
> -       const char *namespace;
> -       int n;
> +       struct module *mod;
> +       struct symbol *sym;
>
> -       for (n = 0; n < SYMBOL_HASH_SIZE ; n++) {
> -               symbol = symbolhash[n];
> -               while (symbol) {
> -                       if (!symbol->module->from_dump) {
> -                               namespace = symbol->namespace;
> -                               buf_printf(&buf, "0x%08x\t%s\t%s\t%s\t%s\n",
> -                                          symbol->crc, symbol->name,
> -                                          symbol->module->name,
> -                                          export_str(symbol->export),
> -                                          namespace ? namespace : "");
> -                       }
> -                       symbol = symbol->next;
> +       list_for_each_entry(mod, &modules, list) {
> +               if (mod->from_dump)
> +                       continue;
> +               list_for_each_entry(sym, &mod->exported_symbols, list) {
> +                       buf_printf(&buf, "0x%08x\t%s\t%s\t%s\t%s\n",
> +                                  sym->crc, sym->name, mod->name,
> +                                  export_str(sym->export),
> +                                  sym->namespace ?: "");
>                 }
>         }
>         write_buf(&buf, fname);
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 4cb955dda83f..5922b0c39bb7 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -112,6 +112,7 @@ buf_write(struct buffer *buf, const char *s, int len);
>  struct module {
>         struct list_head list;
>         int gpl_compatible;
> +       struct list_head exported_symbols;
>         struct list_head unresolved_symbols;
>         bool from_dump;         /* true if module was loaded from *.symvers */
>         bool is_vmlinux;
> --
> 2.32.0
>

-- 
Thanks,
~Nick Desaulniers