Re: [PATCH] mux-core: make it explicitly non-modular

[Paul Gortmaker]

[Re: [PATCH] mux-core: make it explicitly non-modular] On 08/03/2017 (Wed 16:32) Peter Rosin wrote:

> On 2017-03-08 15:38, Paul Gortmaker wrote:

[...]

> > 
> > Note that mux-core doesn't have a .ko (modular variant) and also you can
> > confirm with nm that everything in mux-core.o is in built-in.o (in this
> > case the nm outputs are virtually identical).
> 
> Yes, I was aware that the mux core was not set up to be built as a module,
> and that was intentional. But the only reason for that was that I thought
> subsystem cores were never modular. The module related remains that
> you removed were just that, remains from other code used as a template.

So, I used to think the same -- core subsystems were not modules.  But
what has happened is that we've got all these different ARM boards, and
they have board specific PCIe drivers/setup and board specific DMA
drivers.  Add to that the desire to have a distro like kernel for a
group of boards, and you get a situation where people want to boot the
absolute minimum vmlinux to support the core and RAM, and then load PCIe
and DMA drivers from a ramdisk/initrd as the specific board is detected.

The goal of not having a bloated vmlinux and a higher resource threshold
needed just for booting is reasonably valid I think.  But at the same
time, if the end target will never be part of a shared vmlinux image,
but always part of a board specific ".config" build, then the effort to
make such core subsystems modular is rather pointless.  Hence it isn't
sensible to have a blanket rule/policy -- it needs looking case by case.

If the mux subsystem isn't board specific (and at a glance, it seems
like it is not) then it might be the former and not the latter.  But I
leave that call up to you.

> > But if you do make that a module too, you will need the MODULE_LICENSE
> > tag.  The kernel also checks that license compatibility is maintained ;
> > i.e. a proprietary module can't use functions from another module doing
> > EXPORT_SYMBOL_GPL.
> > 
> >> Anyway, I'll add this to the queue, and fold it if I happen to rebase.
> > 
> > Oh, and speaking of EXPORT_SYMBOL, I should have added <linux/export.h>
> > to your file with the patch I sent, since it does do exports.
> 
> Ok, I assume that it should be included by the individual drivers as well?

Not really -- drivers, as leaf nodes in the overall system rarely do any
EXPORT_SYMBOL operations.  It is the exporters and not the consumers who
should explicitly include that header.

> > Hope that helps,
> > Paul.
> 
> Yup, it helps, but it doesn't really help with making the call if it
> should be possible to build the mux core as module or not. What things
> are not possible to build as modules? E.g. there's the mux_ida thing,

See above comments re: sharing vs. board specific.  You should then be
better able to make the call.

> will that work as expected if the mux core is modular (and possibly
> unloaded)? Isn't there a risk that names will be reused and confusing
> and cause bugs? Or is there a way to block a module from being unloaded?

Per the above use case (minimal vmlinux) the unload isn't so important,
and in many cases it may be racy or simply not make sense.  If that is
the case, you don't need to provide a module_exit, and in the past I
think we used to bump the module use count at a successful load to
prevent unloading ; there might be a more elegant method now; google is
your friend here.  Also, I don't think name reuse/confusion is an issue.

> Hmmm, there's also the ".owner = THIS_MODULE" line in mux-core.c that
> seems related, is that line valid for non-modular "units"?

That is largely a legacy bit of (repeatedly copied) boiler plate.  If I
recall correctly, in a non-module it becomes ".owner = NULL".  And even
if you are a proper module, I think the higher level wrappers like
module_platform_driver() do the right thing with .owner so that drivers
don't have to duplicate such mundane boilerplate.  If you search git
history for THIS_MODULE, I think you will find batch removals from
drivers for where the per-driver boilerplate is simply redundant.

Paul.
--

> 
> Cheers,
> peda
>