Message-ID: <20170308143847.GA22171@windriver.com>
Date:   Wed, 8 Mar 2017 09:38:47 -0500
From:   Paul Gortmaker <paul.gortmaker@...driver.com>
To:     Peter Rosin <peda@...ntia.se>
CC:     <linux-kernel@...r.kernel.org>, Jonathan Cameron <jic23@...nel.org>
Subject: Re: [PATCH] mux-core: make it explicitly non-modular

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

> On 2017-03-07 23:41, Paul Gortmaker wrote:
> > The Kconfig currently controlling compilation of this code is:
> > 
> > drivers/mux/Kconfig:menuconfig MULTIPLEXER
> > drivers/mux/Kconfig:    bool "Multiplexer subsystem"
> > 
> > ...meaning that it currently is not being built as a module by anyone.
> > 
> > Lets remove the couple traces of modular infrastructure use, so that
> > when reading the driver there is no doubt it is builtin-only.
> > 
> > Hence we delete the MODULE_LICENSE tag etc. since all that information
> > is already contained at the top of the file in the comments.
> 
> Hi Paul,
> 
> Yup, it is confirmed, I don't really know what I'm doing. In particular
> when in comes to modules... I did wonder about this when I wrote the
> code and one specific thing I wondered about was how module loading is
> triggered.
> 
> I can imagine that calling a function that happens to be exported from
> a module triggers its loading and that failure to load the module leads
> to an oops. But I don't know if that is even remotely correct? Is it?

No, that would be pretty user unfriendly.  When you "insmod" a module,
the kernel checks just like a linker, that all the functions it wants to
use are available.  If they are not, then the kernel fails to load it.
But it fails gracefully with a list of the unresolved symbols.

Obviously managing all the module interdependencies manually would be
tedious, so that is what what "depmod" does.  Then loading of a module
is usually done with "modprobe", which will consult the depmod data and
then "insmod" the required modules in the needed order.

> Is there a short answer? Or what should I read for a longer one?

Well, now that you know it won't oops from a module needing a module,
perhaps you do now want to make the core support modular?  If I look in
my ARM build, I see:

    paul@...-lpgnfs-02:~/git/arm-build/drivers/mux$ ls -l
    total 96
    -rw-rw-r-- 1 paul paul   399 Mar  7 16:41 built-in.mod.c
    -rw-rw-r-- 1 paul paul 15999 Mar  7 16:41 built-in.o
    -rw-rw-r-- 1 paul paul     0 Mar  7 16:12 modules.builtin
    -rw-rw-r-- 1 paul paul    65 Mar  7 16:41 modules.order
    -rw-rw-r-- 1 paul paul  8324 Mar  7 16:49 mux-adg792a.ko
    -rw-rw-r-- 1 paul paul  1566 Mar  7 16:46 mux-adg792a.mod.c
    -rw-rw-r-- 1 paul paul  3552 Mar  7 16:47 mux-adg792a.mod.o
    -rw-rw-r-- 1 paul paul  6824 Mar  7 16:41 mux-adg792a.o
    -rw-rw-r-- 1 paul paul 20472 Mar  7 16:41 mux-core.o
    -rw-rw-r-- 1 paul paul  7680 Mar  7 16:49 mux-gpio.ko
    -rw-rw-r-- 1 paul paul  1550 Mar  7 16:46 mux-gpio.mod.c
    -rw-rw-r-- 1 paul paul  3408 Mar  7 16:47 mux-gpio.mod.o
    -rw-rw-r-- 1 paul paul  6360 Mar  7 16:41 mux-gpio.o
    paul@...-lpgnfs-02:~/git/arm-build/drivers/mux$ 

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).

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.

Hope that helps,
Paul.
--

> Thanks!
> 
> Cheers,
> peda
> 
> > Cc: Peter Rosin <peda@...ntia.se>
> > Cc: Jonathan Cameron <jic23@...nel.org>
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@...driver.com>
> > ---
> > 
> > [feel free to fold this change into the original addition commit if
> >  you happen to be rebasing for some other reason... ]
> > 
> >  drivers/mux/mux-core.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > diff --git a/drivers/mux/mux-core.c b/drivers/mux/mux-core.c
> > index 46088a0f9677..7b4af6370e37 100644
> > --- a/drivers/mux/mux-core.c
> > +++ b/drivers/mux/mux-core.c
> > @@ -15,7 +15,7 @@
> >  #include <linux/device.h>
> >  #include <linux/err.h>
> >  #include <linux/idr.h>
> > -#include <linux/module.h>
> > +#include <linux/init.h>
> >  #include <linux/mux.h>
> >  #include <linux/of.h>
> >  #include <linux/of_platform.h>
> > @@ -408,7 +408,3 @@ void devm_mux_control_put(struct device *dev, struct mux_control *mux)
> >  EXPORT_SYMBOL_GPL(devm_mux_control_put);
> >  
> >  subsys_initcall(mux_init);
> > -
> > -MODULE_DESCRIPTION("Multiplexer subsystem");
> > -MODULE_AUTHOR("Peter Rosin <peda@...ntia.se");
> > -MODULE_LICENSE("GPL v2");
> > 
> 

Powered by blists - more mailing lists