On 21 Nov 2022, Luis Chamberlain spake thusly: > On Mon, Nov 21, 2022 at 11:12:52AM -0800, Luis Chamberlain wrote: >> On Mon, Nov 21, 2022 at 03:21:10PM +0000, Nick Alcock wrote: >> > 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)