| Message ID | 20220424190811.1678416-17-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: yet another series of cleanups (modpost and LTO) | expand |
On Sun, Apr 24, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > This is currently a warning, but I think modpost should stop building > in this case. > > If the same symbol is exported multiple times and we let it keep going, > the sanity check becomes difficult. > > Only the legitimate case is that an external module overrides the > corresponding in-tree module to provide a different implementation > with the same interface. Could the same module export a weak version of a symbol, and a strong one? Can kernel modules override in-kernel strong symbols? > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > scripts/mod/modpost.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 14044cd94aaa..73f0b98e3b5a 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -411,9 +411,9 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod, > 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", > - mod->name, name, s->module->name, > - s->module->is_vmlinux ? "" : ".ko"); > + error("%s: '%s' exported twice. Previous export was in %s%s\n", > + mod->name, name, s->module->name, > + s->module->is_vmlinux ? "" : ".ko"); > return s; > } > > -- > 2.32.0 >
On Tue, Apr 26, 2022 at 3:48 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Sun, Apr 24, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > This is currently a warning, but I think modpost should stop building > > in this case. > > > > If the same symbol is exported multiple times and we let it keep going, > > the sanity check becomes difficult. > > > > Only the legitimate case is that an external module overrides the > > corresponding in-tree module to provide a different implementation > > with the same interface. > > Could the same module export a weak version of a symbol, and a strong one? No. There is no concept like EXPORT_SYMBOL_WEAK. I am talking about kmod things. You can modprobe an external module instead of the in-kernel one. > > Can kernel modules override in-kernel strong symbols? Yes, I think so. > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > scripts/mod/modpost.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > index 14044cd94aaa..73f0b98e3b5a 100644 > > --- a/scripts/mod/modpost.c > > +++ b/scripts/mod/modpost.c > > @@ -411,9 +411,9 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod, > > 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", > > - mod->name, name, s->module->name, > > - s->module->is_vmlinux ? "" : ".ko"); > > + error("%s: '%s' exported twice. Previous export was in %s%s\n", > > + mod->name, name, s->module->name, > > + s->module->is_vmlinux ? "" : ".ko"); > > return s; > > } > > > > -- > > 2.32.0 > > > > > -- > Thanks, > ~Nick Desaulniers
On Mon, Apr 25, 2022 at 9:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Tue, Apr 26, 2022 at 3:48 AM Nick Desaulniers > <ndesaulniers@google.com> wrote: > > > > On Sun, Apr 24, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > This is currently a warning, but I think modpost should stop building > > > in this case. > > > > > > If the same symbol is exported multiple times and we let it keep going, > > > the sanity check becomes difficult. > > > > > > Only the legitimate case is that an external module overrides the > > > corresponding in-tree module to provide a different implementation > > > with the same interface. > > > > Could the same module export a weak version of a symbol, and a strong one? > > No. There is no concept like EXPORT_SYMBOL_WEAK. > > I am talking about kmod things. > You can modprobe an external module instead of the in-kernel one. Ok, this patch seems fine to me. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
On Wed, Apr 27, 2022 at 1:40 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Mon, Apr 25, 2022 at 9:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Tue, Apr 26, 2022 at 3:48 AM Nick Desaulniers > > <ndesaulniers@google.com> wrote: > > > > > > On Sun, Apr 24, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > This is currently a warning, but I think modpost should stop building > > > > in this case. > > > > > > > > If the same symbol is exported multiple times and we let it keep going, > > > > the sanity check becomes difficult. > > > > > > > > Only the legitimate case is that an external module overrides the > > > > corresponding in-tree module to provide a different implementation > > > > with the same interface. > > > > > > Could the same module export a weak version of a symbol, and a strong one? > > > > No. There is no concept like EXPORT_SYMBOL_WEAK. > > > > I am talking about kmod things. > > You can modprobe an external module instead of the in-kernel one. > > Ok, this patch seems fine to me. > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > -- > Thanks, > ~Nick Desaulniers Nick, If useful, I can add more commits to the commit description. I know one example in the tree that exploits this feature. $ make allmodconfig all You will get drivers/nvdimm/libnvdimm.ko, then $ make M=tools/testing/nvdimm You will get tools/testing/nvdimm/libnvdimm.ko The latter is a mocked one that exported the same symbols as drivers/nvdimm/libnvdimm.ko
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 14044cd94aaa..73f0b98e3b5a 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -411,9 +411,9 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod, 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", - mod->name, name, s->module->name, - s->module->is_vmlinux ? "" : ".ko"); + error("%s: '%s' exported twice. Previous export was in %s%s\n", + mod->name, name, s->module->name, + s->module->is_vmlinux ? "" : ".ko"); return s; }
This is currently a warning, but I think modpost should stop building in this case. If the same symbol is exported multiple times and we let it keep going, the sanity check becomes difficult. Only the legitimate case is that an external module overrides the corresponding in-tree module to provide a different implementation with the same interface. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/mod/modpost.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)