Differences between builtins and modules

Differences between builtins and modules

[Lucas De Marchi]

Changing the subject because this is unrelated to the patch to kmod. It was:
[PATCH] libkmod-module: Remove directory existence check for KMOD_MODULE_BUILTIN

CC'ing Michael Marek who created the modules.builtin file a while ago.

On Thu, Feb 19, 2015 at 12:25 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Rusty, thinking again if we fallback to "coming" instead of "builtin"
>> everything should be fine, no? Because the decision about builtin has
>> already been taken by looking at the modules.builtin index. If we
>> return "coming" here the second call to modprobe would call
>> init_module() again which would wait for the first one to complete (or
>> return EEXIST if it's already live) since we only shortcut the
>> init_module() call if the module is live or builtin
>
> It's weird that your code should care about this at all.  Ideally,
> userspace would see builtin modules as simply unremovable ones.
> Historically, it hasn't; it was only module parameters for builtins
> which caused us to expose built modules.

While integrating the patch above in kmod I noticed there are more differences.

/sys/module/<modname>/ may exist and modname not be present in
modules.builtin index. Looking in the kernel tree, this is because
Makefile.modbuiltin adds only those that can be tristate and not those
that can be only boolean. It may make sense because since a "module"
can never be compiled as a "module", there would be no reason to put
it in the index.

right now in kmod if we do this:

"modprobe --show-depends vt" it reports as "builtin" since there is a
directory in /sys/module

However if vt had no arguments, it would have been reported as "not found".

This could be particularly bad if in a kernel version an option was
tristate and in a new version it changed to boolean. I'm not sure if
this is common to happen in kernel. Any code that did "modprobe
<module>" would start to fail.

My questions are:
1) should we put *all* the "modules" in the builtin index?
2) should we actually check /sys/module/<modulename> to report a
module as builtin or just stop doing that and rely solely in the
index? Initially I'd like to do the opposite, but given the race in
deciding this I'm favoring the index.

thanks

-- 
Lucas De Marchi

Re: Differences between builtins and modules

[Michal Marek]

On 2015-02-23 15:30, Lucas De Marchi wrote:
> This could be particularly bad if in a kernel version an option was
> tristate and in a new version it changed to boolean. I'm not sure if
> this is common to happen in kernel. Any code that did "modprobe
> <module>" would start to fail.

I think it's quite uncommon (*) and also the use case for loading
builtin modules is not that common. I can think of:
1) building the initramfs, to determine which *.ko files need to be
   copied to it. Since such tools are often updated for other reasons,
   it's not a big deal.
2) Hardcoded module names in things like softdep -- hopefully not that
   common either, plus the kernel-provided soft dependencies can be
   fixed together with the change.

Until not so long ago, the kernel would return EINVAL if passed a
non-existent (renamed, removed) module option to init_module, yet there
were no attempts at preserving the module options for compatibility reasons.

(*) I now did a quick search:
$ git log -p origin/master --no-merges -- '*/Kconfig*' | grep -C3 '^-
*tristate' | grep '^+ *bool'
+       bool "Intel P state control"
+       bool "Intel microcode patch loading support"
+       bool "AMD microcode patch loading support"
+        bool "STI text console"
+       bool "Enable DDC2 Support"
+       bool "Enable Console Acceleration"

That's only 6 cases in the whole git history. Maybe there are a few more
hidden outside the three-line context as part of larger edits, but I'm
sure more modules have been *removed* entirely from the kernel over this
period.


> My questions are:
> 1) should we put *all* the "modules" in the builtin index?

You mean all *.o files that do not end up in some *.ko? That won't work,
because unlike module names, the names of object files are not global.
Plus, there was IIRC an idea to teach lsmod to print builtin modules --
listing all *.o would make it rather useless.


> 2) should we actually check /sys/module/<modulename> to report a
> module as builtin or just stop doing that and rely solely in the
> index? Initially I'd like to do the opposite, but given the race in
> deciding this I'm favoring the index.

If the race between the creation of /sys/module/<modulename> and
/sys/module/<modulename>/initstate is inevitable, then I'm afraid we
have to rely on the index.

Michal

Re: Differences between builtins and modules

[Harish Jenny Kandiga Nagaraj]

On Monday 23 February 2015 09:21 PM, Michal Marek wrote:
> On 2015-02-23 15:30, Lucas De Marchi wrote:
>> This could be particularly bad if in a kernel version an option was
>> tristate and in a new version it changed to boolean. I'm not sure if
>> this is common to happen in kernel. Any code that did "modprobe
>> <module>" would start to fail.
> I think it's quite uncommon (*) and also the use case for loading
> builtin modules is not that common. I can think of:
> 1) building the initramfs, to determine which *.ko files need to be
>    copied to it. Since such tools are often updated for other reasons,
>    it's not a big deal.
> 2) Hardcoded module names in things like softdep -- hopefully not that
>    common either, plus the kernel-provided soft dependencies can be
>    fixed together with the change.
>
> Until not so long ago, the kernel would return EINVAL if passed a
> non-existent (renamed, removed) module option to init_module, yet there
> were no attempts at preserving the module options for compatibility reasons.
>
> (*) I now did a quick search:
> $ git log -p origin/master --no-merges -- '*/Kconfig*' | grep -C3 '^-
> *tristate' | grep '^+ *bool'
> +       bool "Intel P state control"
> +       bool "Intel microcode patch loading support"
> +       bool "AMD microcode patch loading support"
> +        bool "STI text console"
> +       bool "Enable DDC2 Support"
> +       bool "Enable Console Acceleration"
>
> That's only 6 cases in the whole git history. Maybe there are a few more
> hidden outside the three-line context as part of larger edits, but I'm
> sure more modules have been *removed* entirely from the kernel over this
> period.
>
>
>> My questions are:
>> 1) should we put *all* the "modules" in the builtin index?
> You mean all *.o files that do not end up in some *.ko? That won't work,
> because unlike module names, the names of object files are not global.
> Plus, there was IIRC an idea to teach lsmod to print builtin modules --
> listing all *.o would make it rather useless.
>
>
>> 2) should we actually check /sys/module/<modulename> to report a
>> module as builtin or just stop doing that and rely solely in the
>> index? Initially I'd like to do the opposite, but given the race in
>> deciding this I'm favoring the index.
> If the race between the creation of /sys/module/<modulename> and
> /sys/module/<modulename>/initstate is inevitable, then I'm afraid we
> have to rely on the index.
>
> Michal

Can we add some flag like
KMOD_PROBE_FORCE_DIRECTORY_CHECK = 0x00040,
and pass it to kmod_module_get_initstate to make
"modprobe --show-depends vt" to report as "builtin" ?

Also if the use case for loading builtin modules is not that common
( Also don’t know if 'modprobe vt' command does the loading if not loaded)
can we have the same flags be used after checking if it is  .ko file or from .o file if required?

Re: Differences between builtins and modules

[Lucas De Marchi]

On Mon, Feb 23, 2015 at 12:51 PM, Michal Marek <mmarek@suse.cz> wrote:
> On 2015-02-23 15:30, Lucas De Marchi wrote:
>> This could be particularly bad if in a kernel version an option was
>> tristate and in a new version it changed to boolean. I'm not sure if
>> this is common to happen in kernel. Any code that did "modprobe
>> <module>" would start to fail.
>
> I think it's quite uncommon (*) and also the use case for loading
> builtin modules is not that common. I can think of:
> 1) building the initramfs, to determine which *.ko files need to be
>    copied to it. Since such tools are often updated for other reasons,
>    it's not a big deal.
> 2) Hardcoded module names in things like softdep -- hopefully not that
>    common either, plus the kernel-provided soft dependencies can be
>    fixed together with the change.
>
> Until not so long ago, the kernel would return EINVAL if passed a
> non-existent (renamed, removed) module option to init_module, yet there
> were no attempts at preserving the module options for compatibility reasons.
>
> (*) I now did a quick search:
> $ git log -p origin/master --no-merges -- '*/Kconfig*' | grep -C3 '^-
> *tristate' | grep '^+ *bool'
> +       bool "Intel P state control"
> +       bool "Intel microcode patch loading support"
> +       bool "AMD microcode patch loading support"
> +        bool "STI text console"
> +       bool "Enable DDC2 Support"
> +       bool "Enable Console Acceleration"
>
> That's only 6 cases in the whole git history. Maybe there are a few more
> hidden outside the three-line context as part of larger edits, but I'm
> sure more modules have been *removed* entirely from the kernel over this
> period.

thanks for looking in detail into this.

>
>
>> My questions are:
>> 1) should we put *all* the "modules" in the builtin index?
>
> You mean all *.o files that do not end up in some *.ko? That won't work,
> because unlike module names, the names of object files are not global.

I was actually meaning anything that can have a directory under
/sys/module/. I figure we can't easily know this.

> Plus, there was IIRC an idea to teach lsmod to print builtin modules --
> listing all *.o would make it rather useless.

This was one of my ideas... to traverse /sys/module and give more
information than we actually output right now, including builtin
modules. However, given the fact that builtin modules only have an
entry in /sys/module if they have params and now that I'm aware of the
race between the creation of the directory and the initstate file, I'm
giving up on this idea for now.

>> 2) should we actually check /sys/module/<modulename> to report a
>> module as builtin or just stop doing that and rely solely in the
>> index? Initially I'd like to do the opposite, but given the race in
>> deciding this I'm favoring the index.
>
> If the race between the creation of /sys/module/<modulename> and
> /sys/module/<modulename>/initstate is inevitable, then I'm afraid we
> have to rely on the index.

So my current plan is to rely solely on modules.builtin to output to
modprobe that a module is builtin. So things like "modprobe vt" will
start to fail saying there's no vt module. Any objections here?

-- 
Lucas De Marchi

Re: Differences between builtins and modules

[Lucas De Marchi]

On Tue, Feb 24, 2015 at 8:42 AM, Harish Jenny Kandiga Nagaraj
<harish_kandiga@mentor.com> wrote:
>
> On Monday 23 February 2015 09:21 PM, Michal Marek wrote:
>> On 2015-02-23 15:30, Lucas De Marchi wrote:
> Can we add some flag like
> KMOD_PROBE_FORCE_DIRECTORY_CHECK =3D 0x00040,
> and pass it to kmod_module_get_initstate to make
> "modprobe --show-depends vt" to report as "builtin" ?

I don't want to add more flags. It's already pretty complicated, more
than it should IMO. If the tools using kmod can live with the change
proposed here, I'm ok with that.

> Also if the use case for loading builtin modules is not that common
> ( Also don=E2=80=99t know if 'modprobe vt' command does the loading if no=
t loaded)
> can we have the same flags be used after checking if it is  .ko file or f=
rom .o file if required?

Well... there's no way for us to arrive at the final call to
finit_module() for a builtin module. Simply because there's no file to
load in memory (or fd to open) to pass to these functions. We simply
fail because we can't reach that far in the code.

--=20
Lucas De Marchi

Re: Differences between builtins and modules

[Michal Marek]

On Tue, Feb 24, 2015 at 10:02:55PM -0300, Lucas De Marchi wrote:
> On Mon, Feb 23, 2015 at 12:51 PM, Michal Marek <mmarek@suse.cz> wrote:
> > On 2015-02-23 15:30, Lucas De Marchi wrote:
> >> My questions are:
> >> 1) should we put *all* the "modules" in the builtin index?
> >
> > You mean all *.o files that do not end up in some *.ko? That won't work,
> > because unlike module names, the names of object files are not global.
> 
> I was actually meaning anything that can have a directory under
> /sys/module/. I figure we can't easily know this.

I see. Well, we could generate the .modinfo section also for builtin
object files and discard it from vmlinux later, like this:

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index ac78910..efe1798 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -683,6 +683,7 @@
 	EXIT_CALL							\
 	*(.discard)							\
 	*(.discard.*)							\
+	*(.modinfo)							\
 	}
 
 /**
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 1c9effa..f38abfa 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -16,23 +16,17 @@
 /* Chosen so that structs with an unsigned long line up. */
 #define MAX_PARAM_PREFIX_LEN (64 - sizeof(unsigned long))
 
-#ifdef MODULE
 #define __MODULE_INFO(tag, name, info)					  \
 static const char __UNIQUE_ID(name)[]					  \
   __used __attribute__((section(".modinfo"), unused, aligned(1)))	  \
   = __stringify(tag) "=" info
-#else  /* !MODULE */
-/* This struct is here for syntactic coherency, it is not used */
-#define __MODULE_INFO(tag, name, info)					  \
-  struct __UNIQUE_ID(name) {}
-#endif
 #define __MODULE_PARM_TYPE(name, _type)					  \
-  __MODULE_INFO(parmtype, name##type, #name ":" _type)
+  __MODULE_INFO(parmtype, name##type, MODULE_PARAM_PREFIX #name ":" _type)
 
 /* One for each parameter, describing how to use it.  Some files do
    multiple of these per line, so can't just use MODULE_INFO. */
 #define MODULE_PARM_DESC(_parm, desc) \
-	__MODULE_INFO(parm, _parm, #_parm ":" desc)
+	__MODULE_INFO(parm, _parm, MODULE_PARAM_PREFIX #_parm ":" desc)
 
 struct kernel_param;
 
Then some script would look at the parmtype= fields in the built-in.o
files and complement the list of builtin modules. However, the question
is whether this is worth it. As far as I understand, the problem is just
the discrepancy between modules.builtin and /sys/module. In practice, no
sane tool is going to need to modprobe 'vt' or 'printk', because there
is no configuration in which these modules would exist.


> > Plus, there was IIRC an idea to teach lsmod to print builtin modules --
> > listing all *.o would make it rather useless.
> 
> This was one of my ideas... to traverse /sys/module and give more
> information than we actually output right now, including builtin
> modules. However, given the fact that builtin modules only have an
> entry in /sys/module if they have params and now that I'm aware of the
> race between the creation of the directory and the initstate file, I'm
> giving up on this idea for now.

You could still print the module names from modules.builtin(.idx). But
this is an unrelated topic, I only brought it up because I did not
understand what you were proposing.

Michal

Re: Differences between builtins and modules

[Lucas De Marchi]

On Wed, Feb 25, 2015 at 8:53 AM, Michal Marek <mmarek@suse.cz> wrote:
> On Tue, Feb 24, 2015 at 10:02:55PM -0300, Lucas De Marchi wrote:
>> On Mon, Feb 23, 2015 at 12:51 PM, Michal Marek <mmarek@suse.cz> wrote:
>> > On 2015-02-23 15:30, Lucas De Marchi wrote:
>> >> My questions are:
>> >> 1) should we put *all* the "modules" in the builtin index?
>> >
>> > You mean all *.o files that do not end up in some *.ko? That won't work,
>> > because unlike module names, the names of object files are not global.
>>
>> I was actually meaning anything that can have a directory under
>> /sys/module/. I figure we can't easily know this.
>
> I see. Well, we could generate the .modinfo section also for builtin
> object files and discard it from vmlinux later, like this:
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index ac78910..efe1798 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -683,6 +683,7 @@
>         EXIT_CALL                                                       \
>         *(.discard)                                                     \
>         *(.discard.*)                                                   \
> +       *(.modinfo)                                                     \
>         }
>
>  /**
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index 1c9effa..f38abfa 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -16,23 +16,17 @@
>  /* Chosen so that structs with an unsigned long line up. */
>  #define MAX_PARAM_PREFIX_LEN (64 - sizeof(unsigned long))
>
> -#ifdef MODULE
>  #define __MODULE_INFO(tag, name, info)                                   \
>  static const char __UNIQUE_ID(name)[]                                    \
>    __used __attribute__((section(".modinfo"), unused, aligned(1)))        \
>    = __stringify(tag) "=" info
> -#else  /* !MODULE */
> -/* This struct is here for syntactic coherency, it is not used */
> -#define __MODULE_INFO(tag, name, info)                                   \
> -  struct __UNIQUE_ID(name) {}
> -#endif
>  #define __MODULE_PARM_TYPE(name, _type)                                          \
> -  __MODULE_INFO(parmtype, name##type, #name ":" _type)
> +  __MODULE_INFO(parmtype, name##type, MODULE_PARAM_PREFIX #name ":" _type)
>
>  /* One for each parameter, describing how to use it.  Some files do
>     multiple of these per line, so can't just use MODULE_INFO. */
>  #define MODULE_PARM_DESC(_parm, desc) \
> -       __MODULE_INFO(parm, _parm, #_parm ":" desc)
> +       __MODULE_INFO(parm, _parm, MODULE_PARAM_PREFIX #_parm ":" desc)
>
>  struct kernel_param;
>
> Then some script would look at the parmtype= fields in the built-in.o
> files and complement the list of builtin modules. However, the question
> is whether this is worth it. As far as I understand, the problem is just
> the discrepancy between modules.builtin and /sys/module. In practice, no
> sane tool is going to need to modprobe 'vt' or 'printk', because there
> is no configuration in which these modules would exist.

Indeed, not sure it's worth it. I did the change in kmod so we don't
check /sys/module anymore [1]. If this is problematic for distros then
maybe we add a patch like yours to the kernel so they are also present
in modules.builtin index.

thanks

-- 
Lucas De Marchi

[1] https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=fd44a98ae2eb5eb32161088954ab21e58e19dfc4

Re: Differences between builtins and modules

[Randy Dunlap]

On 02/24/2015 05:02 PM, Lucas De Marchi wrote:
> On Mon, Feb 23, 2015 at 12:51 PM, Michal Marek <mmarek@suse.cz> wrote:
>> On 2015-02-23 15:30, Lucas De Marchi wrote:
>>> This could be particularly bad if in a kernel version an option was
>>> tristate and in a new version it changed to boolean. I'm not sure if
>>> this is common to happen in kernel. Any code that did "modprobe
>>> <module>" would start to fail.
>>
>> I think it's quite uncommon (*) and also the use case for loading
>> builtin modules is not that common. I can think of:
>> 1) building the initramfs, to determine which *.ko files need to be
>>    copied to it. Since such tools are often updated for other reasons,
>>    it's not a big deal.
>> 2) Hardcoded module names in things like softdep -- hopefully not that
>>    common either, plus the kernel-provided soft dependencies can be
>>    fixed together with the change.
>>
>> Until not so long ago, the kernel would return EINVAL if passed a
>> non-existent (renamed, removed) module option to init_module, yet there
>> were no attempts at preserving the module options for compatibility reasons.
>>
>> (*) I now did a quick search:
>> $ git log -p origin/master --no-merges -- '*/Kconfig*' | grep -C3 '^-
>> *tristate' | grep '^+ *bool'
>> +       bool "Intel P state control"
>> +       bool "Intel microcode patch loading support"
>> +       bool "AMD microcode patch loading support"
>> +        bool "STI text console"
>> +       bool "Enable DDC2 Support"
>> +       bool "Enable Console Acceleration"
>>
>> That's only 6 cases in the whole git history. Maybe there are a few more
>> hidden outside the three-line context as part of larger edits, but I'm
>> sure more modules have been *removed* entirely from the kernel over this
>> period.
> 
> thanks for looking in detail into this.
> 
>>
>>
>>> My questions are:
>>> 1) should we put *all* the "modules" in the builtin index?
>>
>> You mean all *.o files that do not end up in some *.ko? That won't work,
>> because unlike module names, the names of object files are not global.
> 
> I was actually meaning anything that can have a directory under
> /sys/module/. I figure we can't easily know this.
> 
>> Plus, there was IIRC an idea to teach lsmod to print builtin modules --
>> listing all *.o would make it rather useless.
> 
> This was one of my ideas... to traverse /sys/module and give more
> information than we actually output right now, including builtin
> modules. However, given the fact that builtin modules only have an
> entry in /sys/module if they have params and now that I'm aware of the
> race between the creation of the directory and the initstate file, I'm
> giving up on this idea for now.
> 
>>> 2) should we actually check /sys/module/<modulename> to report a
>>> module as builtin or just stop doing that and rely solely in the
>>> index? Initially I'd like to do the opposite, but given the race in
>>> deciding this I'm favoring the index.
>>
>> If the race between the creation of /sys/module/<modulename> and
>> /sys/module/<modulename>/initstate is inevitable, then I'm afraid we
>> have to rely on the index.
> 
> So my current plan is to rely solely on modules.builtin to output to
> modprobe that a module is builtin. So things like "modprobe vt" will
> start to fail saying there's no vt module. Any objections here?
> 

Hi,
[sorry to resurrect such as old thread]


Would someone please answer/reply to this (related) kernel bugzilla entry:
  https://bugzilla.kernel.org/show_bug.cgi?id=118661

or I could just close it?

thanks,
-- 
~Randy

Re: Differences between builtins and modules

[Jason Vas Dias]

Sorry I didn't see this mail until now - RE:

Randy Dunlap <rdunlap@infradead.org> wrote:
> Would someone please answer/reply to this (related) kernel bugzilla entry:
> https://bugzilla.kernel.org/show_bug.cgi?id=118661

Yes, I raised this bug because I think modinfo should return 0 exit status
if a requested module is built-in, not just when it has been loaded, like
this modified version does:
$ modinfo snd
modinfo: ERROR: Module snd not found.
built-in: snd
$ echo $?
0

What was the query about the Bug 118661 that needs to be answered ?
I don't see any query on the bug report - just a comment from someone
who also agrees modinfo should return OK for a built-in module .

Glad to hear someone is finally considering fixing modinfo to report
status of built-in modules - with only a 2 year response time.

Thanks & Best Regards,
Jason