On Thu, Feb 19, 2015 at 12:25 AM, Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote: > Lucas De Marchi <lucas.de.marchi@xxxxxxxxx> writes: >> On Wed, Feb 18, 2015 at 8:40 PM, Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote: >>> Lucas De Marchi <lucas.de.marchi@xxxxxxxxx> writes: >>>> On Wed, Feb 18, 2015 at 2:07 AM, Rusty Russell <rusty@xxxxxxxxxxxxxxx> 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. >> >>> 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. yeah... ideally all modules would have their entries in /sys/module > If we returned EEXIST for builtin modules, would you have to do the > initstate check at all? In this case there would be some behavior changes wrt blacklist, softdeps and builtin modules. Currently if the module is live/builtin we just return success (oh, unless --first-time is passed :-/). Otherwise we apply blacklists, softdeps and dependencies. With this we could reach a scenario in which we fail to load a builtin module which is very weird. And... the race for "lsmod" would still exist. Instead of thinking about a race between 2 modprobe calls, think about a race between 1 modprobe and 1 lsmod. What do I answer for the module that is loading? IMO the "coming" alternative is the one that makes sense (and would also fix the 2 modprobes) -- Lucas De Marchi -- To unsubscribe from this list: send the line "unsubscribe linux-modules" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
