Re: [PATCH v4] kallsyms: add names of built-in modules

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/19/2020 10:37 PM, Masahiro Yamada wrote:

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

Thanks again for that pointer. I'm looking at how that approach can be generalized to the functionality we need for the object-to-module mapping.

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.

Unfortunately we need to get the information from somewhere. I'll work on streamlining the implementation more, and prepare an updated patch.

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.

Thanks for the review.  In light of the value of the functionality and the
expressed interest in this feature, I'll work towards an updated patch for
further review.

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.



[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux