Re: [PATCH 1/8] kbuild, PCI: generic,versatile: comment out MODULE_LICENSE in non-modules

From: Nick Alcock <nick.alcock@oracle.com>
To: Rob Herring <robh@kernel.org>
Cc: mcgrof@kernel.org, linux-modules@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/8] kbuild, PCI: generic,versatile: comment out MODULE_LICENSE in non-modules
Date: Fri, 10 Feb 2023 18:43:45 +0000	[thread overview]
Message-ID: <87a61l1hum.fsf@esperi.org.uk> (raw)
In-Reply-To: <CAL_JsqJ_VhesDZO336tw=KAp88jCLdW9C6y6QDkTF7WpLkr3+w@mail.gmail.com> (Rob Herring's message of "Fri, 10 Feb 2023 11:36:16 -0600")

On 10 Feb 2023, Rob Herring uttered the following:

> On Fri, Feb 10, 2023 at 11:05 AM Nick Alcock <nick.alcock@oracle.com> wrote:
>>
>> Since commit 8b41fc4454e ("kbuild: create modules.builtin without
>> Makefile.modbuiltin or tristate.conf"), MODULE_LICENSE declarations
>> are used to identify modules. As a consequence, uses of the macro
>> in non-modules will cause modprobe to misidentify their containing
>> object file as a module when it is not (false positives), and modprobe
>> might succeed rather than failing with a suitable error message.
>
> How is there an issue when any given module could be built-in instead?

"modprobe module-that-might-be-built-in" is always meant to succeed if
the module is built-in; but "modprobe thing-that-can't-be-a-module-at-all"
is meant to fail. e.g. on a system in which ext4 is built in, I see

loom:~# modprobe ext4
loom:~# lsmod | grep ext4

(with either reporting any answer, and the modprobe returning exitcode 0).

But trying to modprobe something that cannot be a module says, e.g.
(sorry for old kernel, just happens to be what I can lay my hands on
easily right now):

loom:~# modprobe slab
modprobe: FATAL: Module slab not found in directory /lib/modules/5.16.19-00037-ge8dfda4e77fb-dirty
[exitcode nonzero]

This is what is expected, even though slab is built in. It's not a
module, it cannot be a module, so trying to modprobe it should fail.

But right now we have things like this:

silk:~# modprobe zswap
[nothing, exitcode 0]

zswap cannot be built as a module, so this output is wrong (and
inconsistent with the slab attempt above). (Sure, this isn't exactly a
disastrous consequence, but I have other things I'm going to contribute
after this series that depend on this being got right consistently.)

> The general trend is to make all PCI host drivers modules, the primary
> reason this one, IIRC, is not a module is because it is missing
> remove() hook to de-init the PCI bus. Not too hard to add, but I
> wanted to do a common devm hook to do that instead of an explicit
> .remove() hook in each driver. I suppose we could just ignore that and
> allow building as a module. Unloading is optional anyways.

That's perfectly acceptable for me -- I'm not saying that these things
should not be modular, only that *as long as* they are not modular, they
should not have a MODULE_LICENSE. Making it possible to build them as
modules again is fine!

-- 
NULL && (void)