Message-ID: <87r0uy3hkw.fsf@esperi.org.uk>
Date:   Thu, 09 Feb 2023 16:54:23 +0000
From:   Nick Alcock <nick.alcock@...cle.com>
To:     Luis Chamberlain <mcgrof@...nel.org>
Cc:     Ingo Molnar <mingo@...nel.org>, Allen Webb <allenwebb@...gle.com>,
        Zhen Lei <thunder.leizhen@...wei.com>,
        Jeff Mahoney <jeffm@...e.com>, Omar Sandoval <osandov@...com>,
        Christoph Hellwig <hch@...radead.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Quentin Perret <qperret@...gle.com>, masahiroy@...nel.org,
        linux-modules@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
        linux-kernel@...r.kernel.org, arnd@...db.de,
        akpm@...ux-foundation.org, eugene.loh@...cle.com,
        kris.van.hees@...cle.com, elena.zannoni@...cle.com
Subject: Re: [PATCH modules-next v10 00/13] kallsyms: reliable
 symbol->address lookup with /proc/kallmodsyms

On 17 Jan 2023, Luis Chamberlain uttered the following:
[...]
> And please split out the driver conversions to remove module license per
> subsystem, and a new thread. The justification should be simple, commit
> 8b41fc4454e36fbf ("kbuild: create modules.builtin without Makefile.modbuiltin or
> tristate.conf") now relies on the module license tag to do simplify the
> build system. And as part of follow up work to that patch we want to
> correct false positives for modules.builtin where userspace may try to
> load a module which is built-in but such module can never be built in.
> You can add Suggested-by me to that set.

I understand what you are saying, and I have been working on this.

I am going to split this whole series into:

1. A series of patches (123 of them at present) Cc:ed to subsystem
maintainers as well as you, to comment out the MODULE_LICENSE usage.
These patches will have Suggested-by you. This series is rebased against
the latest modules-next and revalidated, and is ready to be mailed out;
will do so shortly.

2. A series of patches adding your new modules.builtin.objs and
kallmodsyms (with revised cover letter, etc, as you request). As part of
the second series I will make sure to involve the tracing maintainers
more and provide an example of usage with perf and hopefully ftrace.
(Note that the name "kallmodsym" is not something I am wedded to. We can
find a better one, if we can come up with one: it's more about
unambiguous symbol resolution now, and maximizing the number of module
names is largely a mechanism to accomplish that, so maybe /proc/ksym?)

This second series is not going out quite yet: I'm working on the perf
support now.

> The same applies to your other Makefile patch (except to the
> Suggested-by as I don't understand the goal there yet), I don't know what it is
> trying to do, but it should be a separate effort. You can feel free to
> Cc me on that too.

I have decided not to submit the tristate checker at this time, as it is
not essential and it made things too confused. The Makefile patch you
refer to and the tristate.conf reintroduction were only needed for the
checker, so are dropped, with nothing more than a reference to a branch
containing the checker in the kallmodsyms cover letter. (The checker
does need periodic rerunning to make sure that spurious MODULE_LICENSE
usages don't creep back in -- reintroductions seem to be running at
about one a month -- but that's easy to do ad-hoc and it doesn't need to
be upstream for that.)

> And lastly users. This cover letter should reflect clearly who are the
> new users who are dying for this feature, Cc them and hope to have them be
> actively engaged during review.

I do hope that adding some proof-of-concept usage of this in perf and
ftrace (emitting symbol names formatted like 'symbol@...name:module'
where possible rather than just unadorned symbols) might show the perf
and ftrace maintainers that this is not useless.

Thanks for your patience and feedback.

Powered by blists - more mailing lists