| Message ID | 87sj3tsawh.fsf@rustcorp.com.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell <rusty@rustcorp.com.au> wrote: > Andy Lutomirski <luto@amacapital.net> writes: >> On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell <rusty@rustcorp.com.au> wrote: >>> Andy Lutomirski <luto@amacapital.net> writes: >>>> Current parameter behavior is odd. Boot parameters that have values >>>> and don't match anything become environment variables, with no >>>> warning. Boot parameters without values that don't match anything >>>> go into argv_init. Everything goes into /proc/cmdline. >>>> >>>> The init_module and finit_module syscalls, however, are strict: >>>> parameters that don't match result in -ENOENT. >>>> >>>> kmod (and hence modprobe), when loading a module called foo, look in >>>> /proc/cmdline for foo.x or foo.x=y, strip off the foo., and pass the >>>> rest to init_module. >>>> >>>> The upshot is that booting with module.nonexistent_parameter=1 is >>>> okay if module is built in or missing entirely but prevents module >>>> from loading if it's an actual module. Similarly, option module >>>> nonexistent_parameter=1 in /etc/modprobe.d prevents the module from >>>> loading the parameter goes away. This means that removing module >>>> parameters unnecessarily breaks things. >>> >>> Err, yes. Don't remove module parameters, they're part of the API. Do >>> you have a particular example? >> >> So things like i915.i915_enable_ppgtt, which is there to enable >> something experimental, needs to stay forever once the relevant >> feature becomes non-experimental and non-optional? This seems silly. > > Well, it's either experimental, in which case you're happy to break > users who use it, or it's not, in which case, don't. Sure. The main issue for me annoyance. Install kernel with experimental option. Set that option in grub's config. Boot another kernel. Grr, the option got propagated there and now I have no graphics. > >> Having the module parameter go away while still allowing the module to >> load seems like a good solution (possibly with a warning in the logs >> so the user can eventually delete the parameter). > > Why not do that for *every* missing parameter then? Why have this weird > notation where the user must know that the parameter might one day go > away? Fair enough. What about the other approach, then? Always warn if an option doesn't match (built-in or otherwise) but load the module anyways. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/module.h b/include/linux/module.h index 46f1ea0..3433f6b 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -186,6 +186,9 @@ struct notifier_block; #ifdef CONFIG_MODULES +/* In generated file kernel/modnamelist.c */ +extern const char *builtin_modnames[]; + extern int modules_disabled; /* for sysctl */ /* Get/put a kernel symbol (calls must be symmetric) */ void *__symbol_get(const char *symbol); diff --git a/init/main.c b/init/main.c index 63534a1..e4dec79 100644 --- a/init/main.c +++ b/init/main.c @@ -245,12 +245,25 @@ static int __init repair_env_string(char *param, char *val, const char *unused) return 0; } +static bool builtin_modname(const char *name, size_t len) +{ + const char **n; + + for (n = builtin_modnames; *n; n++) { + if (strncmp(name, *n, len) == 0 && !(*n)[len]) + return true; + } + return false; +} + /* * Unknown boot options get handed to init, unless they look like * unused parameters (modprobe will find them in /proc/cmdline). */ static int __init unknown_bootoption(char *param, char *val, const char *unused) { + size_t modname_len = strcspn(param, "."); + repair_env_string(param, val, unused); /* Handle obsolete-style parameters */ @@ -258,8 +271,13 @@ static int __init unknown_bootoption(char *param, char *val, const char *unused) return 0; /* Unused module parameter. */ - if (strchr(param, '.') && (!val || strchr(param, '.') < val)) + if (param[modname_len] == '.') { + if (builtin_modname(param, modname_len)) { + printk(KERN_ERR "Unknown kernel parameter %s\n", param); + return -EINVAL; + } return 0; + } if (panic_later) return 0; diff --git a/kernel/.gitignore b/kernel/.gitignore index ab4f109..df5b7c7 100644 --- a/kernel/.gitignore +++ b/kernel/.gitignore @@ -4,3 +4,4 @@ config_data.h config_data.gz timeconst.h +modnamelist.c diff --git a/kernel/Makefile b/kernel/Makefile index bbde5f1..3f3dad7 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -10,7 +10,7 @@ obj-y = fork.o exec_domain.o panic.o printk.o \ kthread.o wait.o sys_ni.o posix-cpu-timers.o mutex.o \ hrtimer.o rwsem.o nsproxy.o srcu.o semaphore.o \ notifier.o ksysfs.o cred.o \ - async.o range.o groups.o lglock.o smpboot.o + async.o range.o groups.o lglock.o smpboot.o modnamelist.o ifdef CONFIG_FUNCTION_TRACER # Do not trace debug files and internal ftrace files @@ -139,6 +139,13 @@ targets += timeconst.h $(obj)/timeconst.h: $(obj)/hz.bc $(src)/timeconst.bc FORCE $(call if_changed,bc) +quiet_cmd_modnamelist = MODNAMELIST $@ + cmd_modnamelist = sh scripts/modnamelist.sh $@ $(src)/modules.builtin + +# Must match name-fix in scripts/Makefile.lib! +kernel/modnamelist.c: modules.builtin FORCE + $(call if_changed,modnamelist) + ifeq ($(CONFIG_MODULE_SIG),y) # # Pull the signing certificate and any extra certificates into the kernel diff --git a/scripts/modnamelist.sh b/scripts/modnamelist.sh new file mode 100755 index 0000000..5d06820 --- /dev/null +++ b/scripts/modnamelist.sh @@ -0,0 +1,22 @@ +#! /bin/sh +set -e + +OUT="$1" +LIST="$2" + +trap "rm -f $OUT.tmp" 0 + +cat > $OUT.tmp <<EOF +/* Autogenerated list of builtin modules. */ +#include <linux/module.h> + +const char *builtin_modnames[] = { +EOF + +sed -e 's@.*/\(.*\)\.ko@\t"\1"@' -e 's@[,-]@_@g' -e 's@$@,@' < $LIST >> $OUT.tmp +cat >> $OUT.tmp <<EOF + NULL +}; +EOF + +mv $OUT.tmp $OUT