Re: [PATCH v9 2/8] kbuild: add modules_thick.builtin

On 21 Nov 2022, Luis Chamberlain spake thusly:

On Mon, Nov 21, 2022 at 11:12:52AM -0800, Luis Chamberlain wrote:

quoted

On Mon, Nov 21, 2022 at 03:21:10PM +0000, Nick Alcock wrote:

quoted

One question: do you think it's worthwhile me submitting patches to
de-MODULE_* things that need it?

100% yes.

Yes please remove all that module declration helpers for things that are
not modules, and after you add your helper which will nag at build time
when it finds new ones.

For justification just mention in the commit log that after commit
8b41fc4454e ("kbuild: create modules.builtin without Makefile.modbuiltin or
tristate.conf") we rely on the module license tag to generate the
modules.builtin file and so built-in code which uses module helpers
just need to be removed.

You should also mention what modules.builtin is used for as per our
Documentation/kbuild/kbuild.rst, and that it is only used for
modprobe to *not* fail when trying to load a module which is
built-in.

Yep! (Which is an extremely damn useful improvement, I must say. I'm
relying on it already.)

Only two remaining problems in your patch that I can see (hacked around
in the checker, but consumers shouldn't have to hack around this sort of
thing). First, some very strange lines like this in modules.builtin.objs:

drivers/hid/hid-uclogic.o: drivers/hid/hid-uclogic-core.o drivers/hid/hid-uclogic-params.o drivers/hid/hid-uclogic-rdesc.o
drivers/hid/hid-uclogic
drivers/hid/hid-uclogic-test.o: 

(note the line with no .o or colon at all.)

This seems to be a consequence of lines like

hid-uclogic-objs                := hid-uclogic-core.o \
                                   hid-uclogic-rdesc.o \
                                   hid-uclogic-params.o
obj-$(CONFIG_HID_UCLOGIC)       += hid-uclogic.o

i.e. use of -objs as a completely random variable for holding object
files which might or might not be in a module. This seems a bit.. risky
to me. Looking for a fix... maybe we can just ignore *-objs on the
grounds that if it matters it will always land in some other variable
too? ... maybe?


One other definite problem: drivers/staging/media/atomisp/Makefile says:

obj-$(CONFIG_VIDEO_ATOMISP) += pci/atomisp_gmin_platform.o

This subdirectory is lost from KBUILD_MODOBJS, leading to the file entry
in modinfo and thus the resulting modules.builtin.objs pointing to a
file named drivers/staging/media/atomisp/atomisp_gmin_platform.o, which
does not exist. (This is also seen in some non-staging directories, e.g.
kernel/trace/rv.)

How many of these are we talking about? I'm happy to take them
via modules-next. I'd hope to not run accross many conflicts against
other trees.

Going by an x86 allyesconfig run, 169 total (probably plus a few given
errors like the one above), plus no doubt a few more for other arches.
So not a vast number, but enough that hacking up a checker was clearly
not a waste of time.

If there turn out to be any conflicts that aren't spurious I'd be very
surprised. Hardly anyone ever even adjusts their email address in
MODULE_AUTHOR when they change it :P

-- 
NULL && (void)