Messages in this thread Patch in this message | | | Date | Mon, 7 Oct 2019 13:25:58 +0200 | From | Jessica Yu <> | Subject | Re: [PATCH] depmod: create and use System.map.no_namespaces |
| |
+++ Matthias Maennich [07/10/19 07:46 +0100]: >Hi! > >On Sat, Oct 05, 2019 at 04:53:56PM +0900, Masahiro Yamada wrote: >>On Sat, Oct 5, 2019 at 3:25 AM Lucas De Marchi >><lucas.de.marchi@gmail.com> wrote: >>> >>>On Fri, Oct 4, 2019 at 2:57 AM Matthias Maennich <maennich@google.com> 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@google.com/ >>>> >>>> Reported-by: Stefan Wahren <stefan.wahren@i2se.com> >>>> Fixes: 8651ec01daed ("module: add support for symbol namespaces.") >>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> >>>> Cc: Lucas De Marchi <lucas.de.marchi@gmail.com> >>>> Cc: Jessica Yu <jeyu@kernel.org> >>>> Cc: Martijn Coenen <maco@android.com> >>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>>> Cc: linux-modules@vger.kernel.org >>>> Signed-off-by: Matthias Maennich <maennich@google.com> >>>> --- >>>> >>>> Please note this depends on the new ksymtab entry format proposed in >>>> https://lore.kernel.org/lkml/20191003075826.7478-2-yamada.masahiro@socionext.com/ >>> >>>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. > >Thanks for the feedback! > >Based on the discussion from last year about changing kmod/depmod for >this change to the ksymtab entries, >(https://lore.kernel.org/lkml/CAKi4VA+wvVxPxVSP6ruSR++TYvavKPNAQ5XhREF_9ZxeOKQirg@mail.gmail.com/), >I assumed this approach would be acceptable. The workaround (yeah, it is >a hack) is a way to mitigate the issue for people that can't update >depmod so easily. System.map.no_namespaces was not intended to be part >of any distribution, but rather used locally in depmod.sh. I could have >made this more clear. > >> >>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. > >I will give this a try.
That sounds like a good idea. Maybe one way of going about it is to take the symbol value of __kstrtabns_##sym, subtract the section address (the section referred to by the st_shndx) from it to obtain the offset into __ksymtab_strings, and read in the namespace string from that offset.
| |