Hi Eugene, On Thu, Jan 9, 2020 at 3:32 AM Eugene Loh <eugene.loh@xxxxxxxxxx> wrote: > > On 12/18/2019 08:28 PM, Masahiro Yamada wrote: > > > On Thu, Dec 19, 2019 at 12:29 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > >> Couple of notes: > >> 1) this affects code that doesn't really have a maintainer. I could > >> take it in my tree, but I would like to have acks from other > >> maintainers. Perhaps Jessica Yu (Module maintainer), and probably one > >> from Linus himself. > >> > >> 2) Do not send new versions of a patch as a reply to the old version. I > >> and many other maintainers sort our inbox by threads, and I look at the > >> top of the thread for patches. That is, if there's another version of a > >> patch that is a reply to a previous version, it is basically off my > >> radar, unless I happen to notice it by chance (which I did with this > >> email). > >> > >> You can send your v4 patch again, but please send it as its own thread, > >> that way it will be on the radar of other maintainers. Hopefully we can > >> get some acks on this as well. > > Sorry. I misunderstood some process doc. But before I resend... > > > I do not like this patch. > > > > scripts/Makefile.modbuiltin is really ugly. > > It traverses all the directories once again. > > > > This patch makes it even worse, > > Kbuild would traverse the > > whole directories three times. > > > > I was thinking to remove scripts/Makefile.modbuiltin > > and Kconfig's tristate.conf entirely > > because it is possible to generate modules.builtin more simply. > > Sorry about the delayed response, due in part to holidays. Thank you > for your on-going review and the pointer to > https://lore.kernel.org/patchwork/project/lkml/list/?series=423205 > > I agree your proposed patch simplifies some build code, but this is > long-standing code. Also, the build time -- either that would be saved > by your patch or that would be incurred by a third traversal -- is > miniscule. > > Further, I do not see how to add object-to-module information to your > proposed scheme. Can you suggest something? If not, then it seems the > proposed code simplification is limiting functionality. The object-to-module information can be retrieved by a similar way as I did in https://lore.kernel.org/patchwork/project/lkml/list/?series=423205 But, even if modules_think.builtin is produced in a new way, there would make no difference in the fact that the build system needs to generate modules_think.builtin and .tmp_vmlinux.range, and kallsyms must integrate a big parser of them. So, I think this patch lacks the taste as overall. > > > As I said, the name of builtin module is not fixed info. > > And, this makes kallsyms fat just for less important info. > > The name of the builtin module can be ambiguous in some cases, but in > most cases it is not. Indeed, the extra information is typically > useful, and comments from, e.g., Linus and Steve were positive about > adding that information to kallsyms. Further, we have even heard > favorable feedback for adding such built-in-module information to > available_filter_functions as well. In my opinion, this should be determined by the balance between the added value and the ugliness of the code. (Real) module names are obvious, but as I stated, built-in module names are somewhat subtle, so I do not like to extend it too much. Perhaps, I was the only person who reviewed the code in detail. After looking at how this feature is integrated, I do not believe this should go in. Sorry. Masahiro Yamada > > >>> On 12/10/2019 09:48 AM, eugene.loh@xxxxxxxxxx wrote: > >>>> From: Eugene Loh <eugene.loh@xxxxxxxxxx> > >>>> > >>>> /proc/kallsyms is very useful for tracers and other tools that need > >>>> to map kernel symbols to addresses. > >>>> > >>>> It would be useful if there were a mapping between kernel symbol and > >>>> module name that only changed when the kernel source code is changed. > >>>> This mapping should not vanish simply because a module becomes built > >>>> into the kernel. -- Best Regards Masahiro Yamada
