On Sat, Oct 5, 2019 at 3:25 AM Lucas De Marchi <lucas.de.marchi@xxxxxxxxx> wrote: > > On Fri, Oct 4, 2019 at 2:57 AM Matthias Maennich <maennich@xxxxxxxxxx> wrote: > > > > depmod in its current version is not aware of symbol namespace in > > ksymtab entries introduced with 8651ec01daed ("module: add support for > > symbol namespaces."). They have the form > > > > __ksymtab_NAMESPACE.symbol_name > > > > A fix for kmod's depmod has been proposed [1]. In order to support older > > versions of depmod as well, create a System.map.no_namespaces during > > scripts/depmod.sh that has the pre-namespaces format. That way users do > > not immediately upgrade the userspace tool. > > > > [1] https://lore.kernel.org/linux-modules/20191004094136.166621-1-maennich@xxxxxxxxxx/ > > > > Reported-by: Stefan Wahren <stefan.wahren@xxxxxxxx> > > Fixes: 8651ec01daed ("module: add support for symbol namespaces.") > > Cc: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> > > Cc: Lucas De Marchi <lucas.de.marchi@xxxxxxxxx> > > Cc: Jessica Yu <jeyu@xxxxxxxxxx> > > Cc: Martijn Coenen <maco@xxxxxxxxxxx> > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > Cc: linux-modules@xxxxxxxxxxxxxxx > > Signed-off-by: Matthias Maennich <maennich@xxxxxxxxxx> > > --- > > > > Please note this depends on the new ksymtab entry format proposed in > > https://lore.kernel.org/lkml/20191003075826.7478-2-yamada.masahiro@xxxxxxxxxxxxx/ > > I don't really agree with that thought, more below. > > > > > That is likely to be merged soon as well as it fixes problems in 5.4-rc*, hence > > this patch depends on it. > > > > Cheers, > > Matthias > > > > .gitignore | 1 + > > scripts/depmod.sh | 8 +++++++- > > 2 files changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/.gitignore b/.gitignore > > index 70580bdd352c..5ed58a7cb433 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -59,6 +59,7 @@ modules.order > > /vmlinux-gdb.py > > /vmlinuz > > /System.map > > +/System.map.no_namespaces > > /Module.markers > > /modules.builtin.modinfo > > > > diff --git a/scripts/depmod.sh b/scripts/depmod.sh > > index e083bcae343f..602e1af072c7 100755 > > --- a/scripts/depmod.sh > > +++ b/scripts/depmod.sh > > @@ -39,7 +39,13 @@ if $depmod_hack_needed; then > > KERNELRELEASE=99.98.$KERNELRELEASE > > fi > > > > -set -- -ae -F System.map > > +# Older versions of depmod do not support symbol namespaces in ksymtab entries, > > +# hence create an alternative System.map with namespace patched out to use for > > +# depmod. I.e. transform entries as follows: > > +# __ksymtab_NAMESPACE.symbol_name -> __ksymtab_symbol_name > > +sed 's/__ksymtab_.*\./__ksymtab_/' System.map > System.map.no_namespaces > > So people with old kmod will have to know they need to pass > System.map.no_namespaces rather than the usual > System.map. Also, distros will need to be update to also copy the new > file to the kernel package (or upgrade/patch kmod). > > I'd rather maintain the current format and fix the bug that patch is > fixing. The namespace > in the end IMO is just a small annoyance with a reason to exist. I agree, this fix is bad. We should not bother kmod or any tools. And System.map.no_namespaces is a cheesy workaround. BTW, I expressed my negative opinion in the review process for the patch set. I am still not convinced with the namespace feature, but anyway it was merged (with poor review and test). Get back on track, probably the right fix would be to stop using __ksymtab_<namespace>.<symbol>. It is not used for any purposes but passing <namespace> / <symbol> pairs to modpost. For example, __kstrtabns_##sym points to the namespace string, so it would be possible to parse it from modpost? Then, asm("__ksymtab_" #ns NS_SEPARATOR #sym) will go away. Masahiro > Lucas De Marchi > > > + > > +set -- -ae -F System.map.no_namespaces > > if test -n "$INSTALL_MOD_PATH"; then > > set -- "$@" -b "$INSTALL_MOD_PATH" > > fi > > -- > > 2.23.0.581.g78d2f28ef7-goog > > > > > -- > Lucas De Marchi -- Best Regards Masahiro Yamada