On Tue, Oct 10, 2023 at 01:33:09PM +0200, Petr Mladek wrote: > On Mon 2023-10-09 15:14:28, Alessandro Carminati wrote: > > Hello Kris, > > > > Thank you for your contribution and for having your thought shared with me. > > > > Allow me to begin this conversation by explaining what came to mind when > > I decided to propose a patch that creates aliases. > > > > The objective was to address a specific problem I was facing while > > minimizing any potential impact on other aspects. > > My initial consideration was the existence of numerous tools, both in the > > kernel and in userspace, that rely on the current kallsyms implementation. > > Both Nick and I shared the concern that making changes to elements upon > > which these tools depend on could have significant consequences. > > > > To the best of my knowledge, Nick's strategy has been to duplicate kallsyms > > with something new - a new, improved kallsyms file. > > > > However, even if Nick's patch were to be accepted, it wouldn't fully meet > > my personal requirements. > > This is because my goal was to utilize kprobe on a symbol that shares its > > name with others. Nick's work wouldn't allow me to do this, and that's why, > > I proposed an alternative. > > > > As a result, my strategy was more modest and focused solely on creating > > aliases for duplicate symbols. > > By adding these aliases, existing tools would remain unaffected, and the > > current system state and ecosystem would be preserved. > > For instance, mechanisms like live patching could continue to use the > > symbol hit count. > > > > On the flip side, introducing these new symbols would enable tracers to > > directly employ the new names without any modifications, and humans could > > easily identify the symbol they are dealing with just by examining the > > name. > > These are the fundamental principles behind my patch - introducing aliases. > > > > Il giorno gio 5 ott 2023 alle ore 18:25 Kris Van Hees > > <kris.van.hees@xxxxxxxxxx> ha scritto: > > > > > > On Wed, Sep 27, 2023 at 05:35:16PM +0000, Alessandro Carminati (Red Hat) wrote: > > > > It is not uncommon for drivers or modules related to similar peripherals > > > > to have symbols with the exact same name. > > > > While this is not a problem for the kernel's binary itself, it becomes an > > > > issue when attempting to trace or probe specific functions using > > > > infrastructure like ftrace or kprobe. > > > > > > > > The tracing subsystem relies on the `nm -n vmlinux` output, which provides > > > > symbol information from the kernel's ELF binary. However, when multiple > > > > symbols share the same name, the standard nm output does not differentiate > > > > between them. This can lead to confusion and difficulty when trying to > > > > probe the intended symbol. > > > > > > > > ~ # cat /proc/kallsyms | grep " name_show" > > > > ffffffff8c4f76d0 t name_show > > > > ffffffff8c9cccb0 t name_show > > > > ffffffff8cb0ac20 t name_show > > > > ffffffff8cc728c0 t name_show > > > > ffffffff8ce0efd0 t name_show > > > > ffffffff8ce126c0 t name_show > > > > ffffffff8ce1dd20 t name_show > > > > ffffffff8ce24e70 t name_show > > > > ffffffff8d1104c0 t name_show > > > > ffffffff8d1fe480 t name_show > > > > > > One problem that remains as far as I can see is that this approach does not > > > take into consideration that there can be duplicate symbols in the core > > > kernel, but also between core kernel and loadable modules, and even between > > > loadable modules. So, I think more is needed to also ensure that this > > > approach of adding alias symbols is also done for loadable modules. > > > > To identify which symbols are duplicated, including those contained in > > modules, it requires exploring all the objects. If I were to propose a > > complementary tool to kas_alias that operates on modules, it would need to > > run on all objects to ascertain the state of the names. > > Only after this assessment could it produce its output. > > This would entail postponing the second kallsyms pass until after all > > modules have been processed. > > Additionally, modules would need to be processed twice: once to assess the > > names and a second time to generate aliases for duplicated symbols. > > I am uncertain if the community would be willing to accept such a delay in > > the build process to introduce this feature. > > >From the livepatching POV: > > + It needs a way to distinguish duplicate symbols within a module. > > + It does _not_ need to distinguish symbols which have the same name > in two modules or in a module and vmlinux. > > Background: The livepatch contains a structure where the livepatched > symbols are already split per-livepatched objects: vmlinux or modules. > I has to know whether a later loaded or removed module is livepatched > or not and what functions need some tweaking. Thank you for sharing the POV for livepatching. That is cery helpful. My follow-up email to my original response shows an example that demonstrates this remaining problem... a loadable module that contains a duplicate symbol, so this issue does pop up (and is why I raise it here): # grep ' metadata_show' /proc/kallsyms ffffffffc05659c0 t metadata_show [md_mod] ffffffffc05739f0 t metadata_show [md_mod] This is certainly a harder problem to deal with because of how kallsyms data for a module is handled, but unfortunately, there is a need because this situation does occur. > > > I'd be happy to work on something like this as a contribution to your work. > > > I would envision the alias entry not needing to have the typical [module] added > > > to it because that will already be annotated on the actual symbol entry. So, > > > the alias could be extended to be something like: > > > > > > ffffffffc0533720 t floppy_open [floppy] > > > ffffffffc0533720 t floppy_open@floppy:drivers_block_floppy_c_3988 > > > > > > (absence of a name: prefix to the path would indicate the symbol is not > > > associated with any module) > > > > > > Doing this is more realistic now as a result of the clean-up patches that > > > Nick introduced, e.g. > > > > > > https://lore.kernel.org/lkml/20230302211759.30135-1-nick.alcock@xxxxxxxxxx/ > > > > > > > Personally, I don't perceive any specific benefit in including the module name > > as part of the decoration. Please don't get me wrong; I do recognize that it > > enhances clarity in Nick's work. > > In that context, a human can easily discern that a duplicated name originates > > from a module, aiding in understanding which symbol they require as it is > > already for duplicates coming from modules. > > > > However, when it comes to my current implementation, I don't see a compelling > > reason to include module name into the decoration I append to names aliases. > > Please accept my apologies if I may not be taking into account any obvious use > > cases, but as it stands, I don't find a strong rationale for incorporating > > module names into the symbol decoration. > > > > In any case, as you've pointed out, duplicates can arise from names in code > > that is not intended to be a module. > > Therefore, relying solely on the module name would not fully address the > > problem you initially aimed to solve. > > >From my POV: > > The source path and the line number is enough to distinguish duplicate > symbols even in modules. > > The added module name would just add extra complexity into the kernel > and tools parsing and using the alias. The tracing tools would need to > handle the source path and line number anyway for symbols duplicated > within same module/vmlinux. > > Adding module name for builtin modules might be misleading. It won't > be clear which symbols are in vmlinux binary and which are in > real modules. The merit of having module names even for symbols that are in a module that is built into the kernel image has been discussed before and there certainly is a benefit for tracers. Also, it is easy to avoid misleading information bu making it easy to distinguish whether a module name is for a builtin module vs a loaded module (e.g. Nick proposed {modname} vs [modname]). But I will defer that topic to a different mail thread, picking up from the earlier discussions on this feature, and proposing a minimal patch to make this data available in a way that is completely independent from this work (and won't impact kallsyms data presentation in any way). Thanks, Kris
