Thread (20 messages) 20 messages, 4 authors, 2015-03-02

Re: [PATCH] libkmod-module: Remove directory existence check for KMOD_MODULE_BUILTIN

From: Rusty Russell <hidden>
Date: 2015-02-19 02:32:07
Also in: lkml

Lucas De Marchi [off-list ref] writes:
On Wed, Feb 18, 2015 at 8:40 PM, Rusty Russell [off-list ref] wrote:
quoted
Lucas De Marchi [off-list ref] writes:
quoted
On Wed, Feb 18, 2015 at 2:07 AM, Rusty Russell [off-list ref] wrote:
Yeah, I just thought (an wanted that) the attributes were being
created first and then hooked up in the sysfs tree under
/sys/module/<modulename>. I.e. if the directory exists and there's no
initstate this is because it's a builtin module. I don't want to
wait/sleep on the file to appear because users of
kmod_module_get_initstate() may not tolerate this behavior.

Looking up at the old module-init-tools, it used an ugly loop with
usleep() before trying to read the file again :-/

Can we change kernel side guaranteeing the initstate file appears
together with the directory?
Greg?  The core problem is that kmod looks for
/sys/module/<name>/initstate; if it's not there, it assumes a builtin
module.
Just to make it clear:

We try to open /sys/module/<name>/initstate. If it fails we stat
/sys/module/<name> checking if it exists and is a directory. If it
does then we assume the module is builtin.
quoted
However, this is racy when a module is being inserted.  Is there a way
to create this sysfs file and dir atomically?
Greg, the question is still valid since it'd be nice to have this
guarantee and be able to correctly reply the state with whatever is in
initstate file, but...

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.

If we returned EEXIST for builtin modules, would you have to do the
initstate check at all?

Thanks,
Rusty.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help