Message-ID: <87mt46q9po.fsf@esperi.org.uk>
Date:   Tue, 21 Mar 2023 17:45:55 +0000
From:   Nick Alcock <nick.alcock@...cle.com>
To:     Luis Chamberlain <mcgrof@...nel.org>
Cc:     Masahiro Yamada <masahiroy@...nel.org>, jim.cromie@...il.com,
        linux-modules@...r.kernel.org,
        Nick Alcock <nick.alcock@...cle.com>,
        Aaron Tomlin <atomlin@...hat.com>,
        Jason Baron <jbaron@...mai.com>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-kbuild@...r.kernel.org, Greg KH <gregkh@...uxfoundation.org>
Subject: Re: RFC - KBUILD_MODNAME is misleading in builtins, as seen in
 /proc/dynamic_debug/control

On 21 Mar 2023, Luis Chamberlain spake thusly:

> On Tue, Mar 21, 2023 at 06:03:01PM +0900, Masahiro Yamada wrote:
>> On Tue, Mar 21, 2023 at 6:04 AM Luis Chamberlain <mcgrof@...nel.org> wrote:
>> > In general we could all benefit from an enhancement for a shortname for
>> > things which could be modules being built-in. We're now seeing requests
>> > for dynamic debug, but it could also be usefulf for Nick's future work
>> > to help userspace tools / tracing map kallsysms to specific modules when
>> > built-in.
>> 
>> I think I rejected it some years ago.
>
> Good to know.

Indeed! It was rejected because it used too much space in the kernel at
runtime (it really did, it was like a megabyte, it was ridiculous).

The new approach consumes only about 12K even in a make allyesconfig
kernel. Any realistic kernel has much lower overhead, usually about 4KiB
total and often even less, but once we're below a page I doubt anyone
cares much any more. Load two modules and you've lost more RAM on pure
page-granularity slack than that.

(This is true to the point where I stopped trying to optimize it further
because all the data to determine which module and object file every
symbol comes from, to sufficient accuracy to make it possible for every
symbol to have a unique (name, module, objname) triplet, was becoming
smaller than the size of the code needed to read it!)

>> He comes back again and again with almost the same approaches,
>> until he finds a "sponsor" (it's you) who will get it in.
>
> Actually I also rejected the same approach too.

... and that approach is indeed gone, surviving only in a patch series
serving as a *validator* to ensure that Luis's approach for tracking
built-in modules gives the right result in every case.

The validator patch series hasn't been upstreamed. I've given up on that
because whenever I tried to upstream it people seem to have assumed that
it was incurring costs at build time or in some way being used by the
kallsyms code at runtime. It doesn't, it isn't, it's purely a validator,
it isn't even executed unless you do make tristatecheck. But not
upstreaming it isn't too problematic: even out of tree it still serves
its purpose. It's only a developer tool after all.

>> When "modprobe foo" succeeds, the user is sure that the kernel
>> provides the feature "foo" (but he does not care if
>> "foo" is built-in or modular).
>
> You are thinking about the modprobe situation. That is in no way shape
> or form the end goal. Nick suggests he has tooling enhancements which
> allow userspace to disambiguate symbols from kallsyms which are built-in
> to come things which are modules.

DTrace has had this for about a decade now, using a much older and less
capable version of the kallmodsyms patch series. I'm working on adding
the same sort of thing to perf and ftrace right now. systemtap can
already do it, using hacks that would make your hair turn white:
hopefully it can move to using /proc/kallmodsyms in time :)

>                                   His hacks bring back the tristate crap
> which both you and I have rejected.

Nah, those are gone and will not come back (see above). In hindsight I
actually have a good reason for rejecting them -- they relied on
uppercasing CONFIG_ symbols to a y/n value, but a lot of the makefiles
contain code that relies on those symbols' values being lowercase! So
doing the modules.builtin construction that way leads to wrong results
unless apply rather a large pile of tweaks to ugly hacks in makefiles
across the tree, and the errors are silent (as witness the fact that
nobody had ever fixed it in over a decade). The new approach avoids
those errors and gives results precisely matching the tristate values in
Kconfig, *as long as* .modinfo is present only in things that can be
built as modules. Hence the validator.

(Also the makefile trickery in the old scripts/Makefile.modbuiltin is
just *unreadable*.)

>         Having something which could never be a module still use
> MODULE_LICENSE() and have modprobe succeed is not fatal.

Actually I have since found code in dracut that relies on modprobe
failing only if a module it tries to load genuinely does not exist, and
succeeding if it's built-in.

There is probably more. It seems like something scripts loading modules
would be likely to care about.

>                                                          However, if
> future tooling *is* going to be relied upon to display to tracer /
> debuggers where symbols come from, it becomes more useful if that
> information is a bit more deterministic.

I'd say it's even more valuable to use it for tracers/debuggers to
arrange to only *trace* things that you want to: so this is input side,
deciding what to trace, as much as it is output side in trace/stackwalk
output etc.

Things like locking primitives are often traceable, but there are many
duplicates all with the same names because they're static functions in
headers, and you really don't want to trace all the duplicates, only the
specific instances you're interested in. (If you trace all of them you
likely cause an overwhelming flood of output and possibly crash the
kernel due to tracing some bit of locking that it was critical nobody
ever trace, which was almost certainly not the specific instance you
were concerned with). Function specialization and hot/cold partitioning
makes this even worse.

> Part of the issue with Nick's patches all along has been proper
> justification / documentation. Try to take a step back and think about
> the possible *value-add* of userspace having more concrete information
> for traces / debugging.

... I hope what I wrote above helps a bit?

I'm about to take another look at the cover letter and commit logs for
the whole kallmodsyms side of things and try (again) to improve on them.

Powered by blists - more mailing lists