Date:   Mon, 20 Mar 2023 14:04:47 -0700
From:   Luis Chamberlain <mcgrof@kernel.org>
To:     jim.cromie@gmail.com, linux-modules@vger.kernel.org,
        Nick Alcock <nick.alcock@oracle.com>,
        Aaron Tomlin <atomlin@redhat.com>
Cc:     Jason Baron <jbaron@akamai.com>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-kbuild@vger.kernel.org, Greg KH <gregkh@linuxfoundation.org>
Subject: Re: RFC - KBUILD_MODNAME is misleading in builtins, as seen in
 /proc/dynamic_debug/control
Message-ID: <ZBjKb8fXHOxnHuHD@bombadil.infradead.org>
References: <CAJfuBxyeKz3bsc=WfjJZDKgAHScC80_irQvmsecxPukjM-J8gw@mail.gmail.com>
 <6af9da81-7a7b-9f47-acb1-d0350bae7f3f@akamai.com>
 <CAJfuBxyoeuurDoUe2tLs=JbX=BbxGdYpf2yBEP6bkhtFh2XTtQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAJfuBxyoeuurDoUe2tLs=JbX=BbxGdYpf2yBEP6bkhtFh2XTtQ@mail.gmail.com>
Sender: Luis Chamberlain <mcgrof@infradead.org>
Precedence: bulk

On Mon, Mar 20, 2023 at 01:59:28PM -0600, jim.cromie@gmail.com wrote:
> On Mon, Mar 20, 2023 at 12:35 PM Jason Baron <jbaron@akamai.com> wrote:
> >
> >
> >
> > On 3/20/23 1:05 AM, jim.cromie@gmail.com wrote:
> > > dynamic-debug METADATA uses KBUILD_MODNAME as:
> > >
> > > #define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt)       \
> > >          static struct _ddebug  __aligned(8)                     \
> > >          __section("__dyndbg") name = {                          \
> > >                  .modname = KBUILD_MODNAME,                      \
> > >
> > > This is going amiss for some builtins, ie those enabled here, by:
> > >
> > >      echo module main +pmf > /proc/dynamic_debug_control
> > >      grep =pmf /proc/dynamic_debug/control
> > >
> > > init/main.c:1187 [main]initcall_blacklist =pmf "blacklisting initcall %s\n"
> > > init/main.c:1226 [main]initcall_blacklisted =pmf "initcall %s blacklisted\n"
> > > init/main.c:1432 [main]run_init_process =pmf "  with arguments:\n"
> > > init/main.c:1434 [main]run_init_process =pmf "    %s\n"
> > > init/main.c:1435 [main]run_init_process =pmf "  with environment:\n"
> > > init/main.c:1437 [main]run_init_process =pmf "    %s\n"
> >
> >
> > Hi Jim,
> >
> > So if I'm following correctly, this is not a new issue, the 'module'
> > name for dynamic debug has always been this way for builtin.
> 
> It is not a new issue - both PM and init-main have been in [main] for some time.
> 
> I believe that with
> cfc1d277891e module: Move all into module/
> 
> module's module-name joined them, changing from [module] to [main]

If there was a regression due to this, we'd be very interested in
hearing about it. Aaron he did the work to move the code to its own directory. 

> We could do
> > something simple and just normalize it when we initially create the
> > table, but setting the 'module name' to 'core' or 'builtin' or something
> > for all these?
> 
> core and builtin would both lump all those separate modules together,
> making it less meaningful.
> 
> having stable names independent of M vs Y config choices is imperative, ISTM.
> 
> Also, I dont think "only builtins are affected" captures the whole problem.
> I dont recall amdgpu or other modules changing when built with =y
> 
> Theres some subtlety in how KBUILD_MODNAME is set,
> and probably many current users who like its current behavior.
> A new var ?
> 
> 1st, I think that anything tristate gets a sensible value,
> but at least some of the builtin-only "modules" get basenames, by default.

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.

To that end I had suggested the current state of affairs & current difficulty
in trying to get us a name for this here:

https://lore.kernel.org/all/Y/kXDqW+7d71C4wz@bombadil.infradead.org/

I ended up suggesting perhaps we need a -DPOSSIBLE_MODULE then if we
could *somehow* pull that off perhaps then we could instead use
-DPOSSIBLE_KBUILD_MODNAME which would ensure a consistent symbol when
a module is built-in as well.

That still leaves the difficulty in trying to gather possible-obj-m as
a future challenge.

  Luis