Re: [RFC PATCH] Allow optional module parameters
On Wed, Jul 3, 2013 at 2:31 PM, Lucas De Marchi wrote: > On Wed, Jul 3, 2013 at 6:23 PM, Michal Marek wrote: >> Dne 3.7.2013 23:17, Andy Lutomirski napsal(a): >>> On Wed, Jul 3, 2013 at 2:03 PM, Michal Marek wrote: Dne 1.7.2013 18:33, Jonathan Masters napsal(a): > One caveat. Sometimes we have manufactured parameters intentionally > to cause a module to fail. We should standardize that piece. You have: blacklist foo to prevent udev from loading a module and install foo /bin/true to prevent modprobe from loading the module at all. What is the motivation for inventing a third way, through adding invalid parameters? >>> >>> FWIW, I've occasionally booted with modulename.garbage=1 to prevent >>> modulename from loading at boot. It may be worth adding a more >>> intentional way to do that. >> >> Hm, right, there seems to be no clean way to achieve this via a >> commandline argument. Maybe define a magic module option to tell the >> module loader not to load a module? > > modprobe.blacklist=modname1,modname2,... is already there, though all > the silliness of blacklist applies unless "-b" is passed (that's the > equivalent behavior of udev) That would probably be good enough for me. It would be neat if this worked for built-in "modules" as well, but that would probably be quite intrusive. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Michal, All,
On 2013-07-03 23:23 +0200, Michal Marek spake thusly:
> Dne 3.7.2013 23:17, Andy Lutomirski napsal(a):
> > On Wed, Jul 3, 2013 at 2:03 PM, Michal Marek wrote:
> >> Dne 1.7.2013 18:33, Jonathan Masters napsal(a):
> >>> One caveat. Sometimes we have manufactured parameters intentionally
> >>> to cause a module to fail. We should standardize that piece.
> >>
> >> You have:
> >>
> >> blacklist foo
> >>
> >> to prevent udev from loading a module and
> >>
> >> install foo /bin/true
> >>
> >> to prevent modprobe from loading the module at all. What is the
> >> motivation for inventing a third way, through adding invalid parameters?
> >>
> >
> > FWIW, I've occasionally booted with modulename.garbage=1 to prevent
> > modulename from loading at boot. It may be worth adding a more
> > intentional way to do that.
>
> Hm, right, there seems to be no clean way to achieve this via a
> commandline argument. Maybe define a magic module option to tell the
> module loader not to load a module?
I was going to suggest that, or a new kernel option:
noloadmodules=module1[,module2...]
The option may well be cumulative, too, so we could do:
noloadmodules=module1,module2 noloadmodules=module3
and none of module{1,2,3} would be loaded. This could allow bootloaders
to build up the command line more easily.
(Ab)using the module parameter to avoid loading is hackish at best, IMHO.
Regards,
Yann E. MORIN.
--
.-..--..
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `.---: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. |
'--^---^--^'
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Wed, Jul 3, 2013 at 6:23 PM, Michal Marek wrote: > Dne 3.7.2013 23:17, Andy Lutomirski napsal(a): >> On Wed, Jul 3, 2013 at 2:03 PM, Michal Marek wrote: >>> Dne 1.7.2013 18:33, Jonathan Masters napsal(a): One caveat. Sometimes we have manufactured parameters intentionally to cause a module to fail. We should standardize that piece. >>> >>> You have: >>> >>> blacklist foo >>> >>> to prevent udev from loading a module and >>> >>> install foo /bin/true >>> >>> to prevent modprobe from loading the module at all. What is the >>> motivation for inventing a third way, through adding invalid parameters? >>> >> >> FWIW, I've occasionally booted with modulename.garbage=1 to prevent >> modulename from loading at boot. It may be worth adding a more >> intentional way to do that. > > Hm, right, there seems to be no clean way to achieve this via a > commandline argument. Maybe define a magic module option to tell the > module loader not to load a module? modprobe.blacklist=modname1,modname2,... is already there, though all the silliness of blacklist applies unless "-b" is passed (that's the equivalent behavior of udev) Lucas De Marchi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Dne 3.7.2013 23:17, Andy Lutomirski napsal(a): > On Wed, Jul 3, 2013 at 2:03 PM, Michal Marek wrote: >> Dne 1.7.2013 18:33, Jonathan Masters napsal(a): >>> One caveat. Sometimes we have manufactured parameters intentionally >>> to cause a module to fail. We should standardize that piece. >> >> You have: >> >> blacklist foo >> >> to prevent udev from loading a module and >> >> install foo /bin/true >> >> to prevent modprobe from loading the module at all. What is the >> motivation for inventing a third way, through adding invalid parameters? >> > > FWIW, I've occasionally booted with modulename.garbage=1 to prevent > modulename from loading at boot. It may be worth adding a more > intentional way to do that. Hm, right, there seems to be no clean way to achieve this via a commandline argument. Maybe define a magic module option to tell the module loader not to load a module? Thanks, Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Wed, Jul 3, 2013 at 2:03 PM, Michal Marek wrote: > Dne 1.7.2013 18:33, Jonathan Masters napsal(a): >> One caveat. Sometimes we have manufactured parameters intentionally >> to cause a module to fail. We should standardize that piece. > > You have: > > blacklist foo > > to prevent udev from loading a module and > > install foo /bin/true > > to prevent modprobe from loading the module at all. What is the > motivation for inventing a third way, through adding invalid parameters? > FWIW, I've occasionally booted with modulename.garbage=1 to prevent modulename from loading at boot. It may be worth adding a more intentional way to do that. --Andy -- Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Dne 1.7.2013 18:33, Jonathan Masters napsal(a): > One caveat. Sometimes we have manufactured parameters intentionally > to cause a module to fail. We should standardize that piece. You have: blacklist foo to prevent udev from loading a module and install foo /bin/true to prevent modprobe from loading the module at all. What is the motivation for inventing a third way, through adding invalid parameters? Thanks, Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Dne 1.7.2013 18:33, Jonathan Masters napsal(a): One caveat. Sometimes we have manufactured parameters intentionally to cause a module to fail. We should standardize that piece. You have: blacklist foo to prevent udev from loading a module and install foo /bin/true to prevent modprobe from loading the module at all. What is the motivation for inventing a third way, through adding invalid parameters? Thanks, Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Wed, Jul 3, 2013 at 2:03 PM, Michal Marek mma...@suse.cz wrote: Dne 1.7.2013 18:33, Jonathan Masters napsal(a): One caveat. Sometimes we have manufactured parameters intentionally to cause a module to fail. We should standardize that piece. You have: blacklist foo to prevent udev from loading a module and install foo /bin/true to prevent modprobe from loading the module at all. What is the motivation for inventing a third way, through adding invalid parameters? FWIW, I've occasionally booted with modulename.garbage=1 to prevent modulename from loading at boot. It may be worth adding a more intentional way to do that. --Andy -- Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Dne 3.7.2013 23:17, Andy Lutomirski napsal(a): On Wed, Jul 3, 2013 at 2:03 PM, Michal Marek mma...@suse.cz wrote: Dne 1.7.2013 18:33, Jonathan Masters napsal(a): One caveat. Sometimes we have manufactured parameters intentionally to cause a module to fail. We should standardize that piece. You have: blacklist foo to prevent udev from loading a module and install foo /bin/true to prevent modprobe from loading the module at all. What is the motivation for inventing a third way, through adding invalid parameters? FWIW, I've occasionally booted with modulename.garbage=1 to prevent modulename from loading at boot. It may be worth adding a more intentional way to do that. Hm, right, there seems to be no clean way to achieve this via a commandline argument. Maybe define a magic module option to tell the module loader not to load a module? Thanks, Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Wed, Jul 3, 2013 at 6:23 PM, Michal Marek mma...@suse.cz wrote: Dne 3.7.2013 23:17, Andy Lutomirski napsal(a): On Wed, Jul 3, 2013 at 2:03 PM, Michal Marek mma...@suse.cz wrote: Dne 1.7.2013 18:33, Jonathan Masters napsal(a): One caveat. Sometimes we have manufactured parameters intentionally to cause a module to fail. We should standardize that piece. You have: blacklist foo to prevent udev from loading a module and install foo /bin/true to prevent modprobe from loading the module at all. What is the motivation for inventing a third way, through adding invalid parameters? FWIW, I've occasionally booted with modulename.garbage=1 to prevent modulename from loading at boot. It may be worth adding a more intentional way to do that. Hm, right, there seems to be no clean way to achieve this via a commandline argument. Maybe define a magic module option to tell the module loader not to load a module? modprobe.blacklist=modname1,modname2,... is already there, though all the silliness of blacklist applies unless -b is passed (that's the equivalent behavior of udev) Lucas De Marchi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Michal, All,
On 2013-07-03 23:23 +0200, Michal Marek spake thusly:
Dne 3.7.2013 23:17, Andy Lutomirski napsal(a):
On Wed, Jul 3, 2013 at 2:03 PM, Michal Marek mma...@suse.cz wrote:
Dne 1.7.2013 18:33, Jonathan Masters napsal(a):
One caveat. Sometimes we have manufactured parameters intentionally
to cause a module to fail. We should standardize that piece.
You have:
blacklist foo
to prevent udev from loading a module and
install foo /bin/true
to prevent modprobe from loading the module at all. What is the
motivation for inventing a third way, through adding invalid parameters?
FWIW, I've occasionally booted with modulename.garbage=1 to prevent
modulename from loading at boot. It may be worth adding a more
intentional way to do that.
Hm, right, there seems to be no clean way to achieve this via a
commandline argument. Maybe define a magic module option to tell the
module loader not to load a module?
I was going to suggest that, or a new kernel option:
noloadmodules=module1[,module2...]
The option may well be cumulative, too, so we could do:
noloadmodules=module1,module2 noloadmodules=module3
and none of module{1,2,3} would be loaded. This could allow bootloaders
to build up the command line more easily.
(Ab)using the module parameter to avoid loading is hackish at best, IMHO.
Regards,
Yann E. MORIN.
--
.-..--..
| Yann E. MORIN | Real-Time Embedded | /\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `.---: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. |
'--^---^--^'
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Wed, Jul 3, 2013 at 2:31 PM, Lucas De Marchi lucas.demar...@profusion.mobi wrote: On Wed, Jul 3, 2013 at 6:23 PM, Michal Marek mma...@suse.cz wrote: Dne 3.7.2013 23:17, Andy Lutomirski napsal(a): On Wed, Jul 3, 2013 at 2:03 PM, Michal Marek mma...@suse.cz wrote: Dne 1.7.2013 18:33, Jonathan Masters napsal(a): One caveat. Sometimes we have manufactured parameters intentionally to cause a module to fail. We should standardize that piece. You have: blacklist foo to prevent udev from loading a module and install foo /bin/true to prevent modprobe from loading the module at all. What is the motivation for inventing a third way, through adding invalid parameters? FWIW, I've occasionally booted with modulename.garbage=1 to prevent modulename from loading at boot. It may be worth adding a more intentional way to do that. Hm, right, there seems to be no clean way to achieve this via a commandline argument. Maybe define a magic module option to tell the module loader not to load a module? modprobe.blacklist=modname1,modname2,... is already there, though all the silliness of blacklist applies unless -b is passed (that's the equivalent behavior of udev) That would probably be good enough for me. It would be neat if this worked for built-in modules as well, but that would probably be quite intrusive. --Andy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Jonathan Masters writes: > One caveat. Sometimes we have manufactured parameters intentionally to cause > a module to fail. We should standardize that piece. Certainly. Can you give an example? Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Jonathan Masters j...@redhat.com writes: One caveat. Sometimes we have manufactured parameters intentionally to cause a module to fail. We should standardize that piece. Certainly. Can you give an example? Cheers, Rusty. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
One caveat. Sometimes we have manufactured parameters intentionally to cause a
module to fail. We should standardize that piece.
--
Sent from my iPad
On Jul 1, 2013, at 4:53, Rusty Russell wrote:
> Rusty Russell writes:
>> Lucas De Marchi writes:
>>> Hi Rusty,
>>>
>>> On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell
>>> wrote:
Andy Lutomirski writes:
> On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell
> wrote:
>> Andy Lutomirski writes:
>>> On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell
>>> wrote:
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.
...
>>> 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.
What does everyone think of this? Jon, Lucas, does this match your
experience?
Thanks,
Rusty.
Subject: modules: don't fail to load on unknown parameters.
Although parameters are supposed to be part of the kernel API, experimental
parameters are often removed. In addition, downgrading a kernel might
cause
previously-working modules to fail to load.
>>>
>>> I agree with this reasoning
>>>
On balance, it's probably better to warn, and load the module anyway.
>>>
>>> However loading the module anyway would bring at least one drawback:
>>> if the user made a typo when passing the option the module would load
>>> anyway and he will probably not even look in the log, since there's
>>> was no errors from modprobe.
>
> OK, so I've had this patch on the backburner, but noone has come up with
> anything better so I'll queue it into modules-next now.
>
> Thanks,
> Rusty.
>
> modules: don't fail to load on unknown parameters.
>
> Although parameters are supposed to be part of the kernel API, experimental
> parameters are often removed. In addition, downgrading a kernel might cause
> previously-working modules to fail to load.
>
> On balance, it's probably better to warn, and load the module anyway.
> This may let through a typo, but at least the logs will show it.
>
> Reported-by: Andy Lutomirski
> Signed-off-by: Rusty Russell
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 3c2c72d..46db10a 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3206,6 +3206,17 @@ out:
>return err;
> }
>
> +static int unknown_module_param_cb(char *param, char *val, const char
> *modname)
> +{
> +/* Check for magic 'dyndbg' arg */
> +int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
> +if (ret != 0) {
> +printk(KERN_WARNING "%s: unknown parameter '%s' ignored\n",
> + modname, param);
> +}
> +return 0;
> +}
> +
> /* Allocate and load the module: note that size of section 0 is always
>zero, and we rely on this for optional sections. */
> static int load_module(struct load_info *info, const char __user *uargs,
> @@ -3292,7 +3303,7 @@ static int load_module(struct load_info *info, const
> char __user *uargs,
>
>/* Module is ready to execute: parsing args may do that. */
>err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
> - -32768, 32767, _dyndbg_module_param_cb);
> + -32768, 32767, unknown_module_param_cb);
>if (err < 0)
>goto bug_cleanup;
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Rusty Russell writes:
> Lucas De Marchi writes:
>> Hi Rusty,
>>
>> On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell
>> wrote:
>>> Andy Lutomirski writes:
On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell
wrote:
> Andy Lutomirski writes:
>> On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell
>> wrote:
>>> 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.
>>> ...
>> 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.
>>>
>>> What does everyone think of this? Jon, Lucas, does this match your
>>> experience?
>>>
>>> Thanks,
>>> Rusty.
>>>
>>> Subject: modules: don't fail to load on unknown parameters.
>>>
>>> Although parameters are supposed to be part of the kernel API, experimental
>>> parameters are often removed. In addition, downgrading a kernel might cause
>>> previously-working modules to fail to load.
>>
>> I agree with this reasoning
>>
>>>
>>> On balance, it's probably better to warn, and load the module anyway.
>>
>> However loading the module anyway would bring at least one drawback:
>> if the user made a typo when passing the option the module would load
>> anyway and he will probably not even look in the log, since there's
>> was no errors from modprobe.
OK, so I've had this patch on the backburner, but noone has come up with
anything better so I'll queue it into modules-next now.
Thanks,
Rusty.
modules: don't fail to load on unknown parameters.
Although parameters are supposed to be part of the kernel API, experimental
parameters are often removed. In addition, downgrading a kernel might cause
previously-working modules to fail to load.
On balance, it's probably better to warn, and load the module anyway.
This may let through a typo, but at least the logs will show it.
Reported-by: Andy Lutomirski
Signed-off-by: Rusty Russell
diff --git a/kernel/module.c b/kernel/module.c
index 3c2c72d..46db10a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3206,6 +3206,17 @@ out:
return err;
}
+static int unknown_module_param_cb(char *param, char *val, const char *modname)
+{
+ /* Check for magic 'dyndbg' arg */
+ int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
+ if (ret != 0) {
+ printk(KERN_WARNING "%s: unknown parameter '%s' ignored\n",
+ modname, param);
+ }
+ return 0;
+}
+
/* Allocate and load the module: note that size of section 0 is always
zero, and we rely on this for optional sections. */
static int load_module(struct load_info *info, const char __user *uargs,
@@ -3292,7 +3303,7 @@ static int load_module(struct load_info *info, const char
__user *uargs,
/* Module is ready to execute: parsing args may do that. */
err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
--32768, 32767, _dyndbg_module_param_cb);
+-32768, 32767, unknown_module_param_cb);
if (err < 0)
goto bug_cleanup;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Rusty Russell ru...@rustcorp.com.au writes:
Lucas De Marchi lucas.demar...@profusion.mobi writes:
Hi Rusty,
On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell ru...@rustcorp.com.au
wrote:
Andy Lutomirski l...@amacapital.net writes:
On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell ru...@rustcorp.com.au
wrote:
Andy Lutomirski l...@amacapital.net writes:
On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell ru...@rustcorp.com.au
wrote:
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.
...
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.
What does everyone think of this? Jon, Lucas, does this match your
experience?
Thanks,
Rusty.
Subject: modules: don't fail to load on unknown parameters.
Although parameters are supposed to be part of the kernel API, experimental
parameters are often removed. In addition, downgrading a kernel might cause
previously-working modules to fail to load.
I agree with this reasoning
On balance, it's probably better to warn, and load the module anyway.
However loading the module anyway would bring at least one drawback:
if the user made a typo when passing the option the module would load
anyway and he will probably not even look in the log, since there's
was no errors from modprobe.
OK, so I've had this patch on the backburner, but noone has come up with
anything better so I'll queue it into modules-next now.
Thanks,
Rusty.
modules: don't fail to load on unknown parameters.
Although parameters are supposed to be part of the kernel API, experimental
parameters are often removed. In addition, downgrading a kernel might cause
previously-working modules to fail to load.
On balance, it's probably better to warn, and load the module anyway.
This may let through a typo, but at least the logs will show it.
Reported-by: Andy Lutomirski l...@amacapital.net
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
diff --git a/kernel/module.c b/kernel/module.c
index 3c2c72d..46db10a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3206,6 +3206,17 @@ out:
return err;
}
+static int unknown_module_param_cb(char *param, char *val, const char *modname)
+{
+ /* Check for magic 'dyndbg' arg */
+ int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
+ if (ret != 0) {
+ printk(KERN_WARNING %s: unknown parameter '%s' ignored\n,
+ modname, param);
+ }
+ return 0;
+}
+
/* Allocate and load the module: note that size of section 0 is always
zero, and we rely on this for optional sections. */
static int load_module(struct load_info *info, const char __user *uargs,
@@ -3292,7 +3303,7 @@ static int load_module(struct load_info *info, const char
__user *uargs,
/* Module is ready to execute: parsing args may do that. */
err = parse_args(mod-name, mod-args, mod-kp, mod-num_kp,
--32768, 32767, ddebug_dyndbg_module_param_cb);
+-32768, 32767, unknown_module_param_cb);
if (err 0)
goto bug_cleanup;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
One caveat. Sometimes we have manufactured parameters intentionally to cause a
module to fail. We should standardize that piece.
--
Sent from my iPad
On Jul 1, 2013, at 4:53, Rusty Russell ru...@rustcorp.com.au wrote:
Rusty Russell ru...@rustcorp.com.au writes:
Lucas De Marchi lucas.demar...@profusion.mobi writes:
Hi Rusty,
On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell ru...@rustcorp.com.au
wrote:
Andy Lutomirski l...@amacapital.net writes:
On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell ru...@rustcorp.com.au
wrote:
Andy Lutomirski l...@amacapital.net writes:
On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell ru...@rustcorp.com.au
wrote:
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.
...
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.
What does everyone think of this? Jon, Lucas, does this match your
experience?
Thanks,
Rusty.
Subject: modules: don't fail to load on unknown parameters.
Although parameters are supposed to be part of the kernel API, experimental
parameters are often removed. In addition, downgrading a kernel might
cause
previously-working modules to fail to load.
I agree with this reasoning
On balance, it's probably better to warn, and load the module anyway.
However loading the module anyway would bring at least one drawback:
if the user made a typo when passing the option the module would load
anyway and he will probably not even look in the log, since there's
was no errors from modprobe.
OK, so I've had this patch on the backburner, but noone has come up with
anything better so I'll queue it into modules-next now.
Thanks,
Rusty.
modules: don't fail to load on unknown parameters.
Although parameters are supposed to be part of the kernel API, experimental
parameters are often removed. In addition, downgrading a kernel might cause
previously-working modules to fail to load.
On balance, it's probably better to warn, and load the module anyway.
This may let through a typo, but at least the logs will show it.
Reported-by: Andy Lutomirski l...@amacapital.net
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
diff --git a/kernel/module.c b/kernel/module.c
index 3c2c72d..46db10a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3206,6 +3206,17 @@ out:
return err;
}
+static int unknown_module_param_cb(char *param, char *val, const char
*modname)
+{
+/* Check for magic 'dyndbg' arg */
+int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
+if (ret != 0) {
+printk(KERN_WARNING %s: unknown parameter '%s' ignored\n,
+ modname, param);
+}
+return 0;
+}
+
/* Allocate and load the module: note that size of section 0 is always
zero, and we rely on this for optional sections. */
static int load_module(struct load_info *info, const char __user *uargs,
@@ -3292,7 +3303,7 @@ static int load_module(struct load_info *info, const
char __user *uargs,
/* Module is ready to execute: parsing args may do that. */
err = parse_args(mod-name, mod-args, mod-kp, mod-num_kp,
- -32768, 32767, ddebug_dyndbg_module_param_cb);
+ -32768, 32767, unknown_module_param_cb);
if (err 0)
goto bug_cleanup;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Lucas De Marchi writes: > Hi Rusty, > > On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell wrote: >> Andy Lutomirski writes: >>> On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell >>> wrote: Andy Lutomirski writes: > On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell > wrote: >> 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. >> ... > 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. >> >> What does everyone think of this? Jon, Lucas, does this match your >> experience? >> >> Thanks, >> Rusty. >> >> Subject: modules: don't fail to load on unknown parameters. >> >> Although parameters are supposed to be part of the kernel API, experimental >> parameters are often removed. In addition, downgrading a kernel might cause >> previously-working modules to fail to load. > > I agree with this reasoning > >> >> On balance, it's probably better to warn, and load the module anyway. > > However loading the module anyway would bring at least one drawback: > if the user made a typo when passing the option the module would load > anyway and he will probably not even look in the log, since there's > was no errors from modprobe. > > For finit_module we could put a flag to trigger this behavior and > propagate it to modprobe, but this is not possible with init_module(). > I can't think in any other option right now... do you have any? No good ones :( MODULE_PARM_DESC isn't compulsory, so you can't rely on that to tell you about option names. Even if we had a flag, how would you know to set it? I guess you could try without then try with, and if it works the second time print a warning about typos. But it's still pretty ugly. We could implement such a flag with a fake "IGNORE_BAD_PARAMS" parameter, for example. That would fail nicely on older kernels, too. Hmmm Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Ben Hutchings writes: > This should also go to stable, so the downgrading issue doesn't continue > to bite people. Andy was complaining about experimental params going away: I haven't heard a single complaint about the downgrading issue. I think it's a nice to have, which is why I mentioned it. I also CC'd the two maintainers most likely to know if it *is* a current problem. And note that this code has been this way for over ten years! No bug report, no cc:stable. What if people were relying on detecting module parameters by the load failing? I'd rather find out when they upgrade to a new kernel instead of poisoning the old ones, too. Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Tue, Mar 19, 2013 at 8:32 PM, Andy Lutomirski wrote: > On Tue, Mar 19, 2013 at 8:26 PM, Lucas De Marchi > wrote: >> Hi Rusty, >> >> On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell >> wrote: >>> Andy Lutomirski writes: On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell wrote: > Andy Lutomirski writes: >> On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell >> wrote: >>> 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. >>> ... >> 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. >>> >>> What does everyone think of this? Jon, Lucas, does this match your >>> experience? >>> >>> Thanks, >>> Rusty. >>> >>> Subject: modules: don't fail to load on unknown parameters. >>> >>> Although parameters are supposed to be part of the kernel API, experimental >>> parameters are often removed. In addition, downgrading a kernel might cause >>> previously-working modules to fail to load. >> >> I agree with this reasoning >> >>> >>> On balance, it's probably better to warn, and load the module anyway. >> >> However loading the module anyway would bring at least one drawback: >> if the user made a typo when passing the option the module would load >> anyway and he will probably not even look in the log, since there's >> was no errors from modprobe. >> >> For finit_module we could put a flag to trigger this behavior and >> propagate it to modprobe, but this is not possible with init_module(). >> I can't think in any other option right now... do you have any? > > Have a different finit_module return value for "successfully loaded, > but there were warnings" perhaps? Never mind. I was thinking that finit_module was new in 3.9. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Tue, Mar 19, 2013 at 8:26 PM, Lucas De Marchi wrote: > Hi Rusty, > > On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell wrote: >> Andy Lutomirski writes: >>> On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell >>> wrote: Andy Lutomirski writes: > On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell > wrote: >> 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. >> ... > 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. >> >> What does everyone think of this? Jon, Lucas, does this match your >> experience? >> >> Thanks, >> Rusty. >> >> Subject: modules: don't fail to load on unknown parameters. >> >> Although parameters are supposed to be part of the kernel API, experimental >> parameters are often removed. In addition, downgrading a kernel might cause >> previously-working modules to fail to load. > > I agree with this reasoning > >> >> On balance, it's probably better to warn, and load the module anyway. > > However loading the module anyway would bring at least one drawback: > if the user made a typo when passing the option the module would load > anyway and he will probably not even look in the log, since there's > was no errors from modprobe. > > For finit_module we could put a flag to trigger this behavior and > propagate it to modprobe, but this is not possible with init_module(). > I can't think in any other option right now... do you have any? Have a different finit_module return value for "successfully loaded, but there were warnings" perhaps? --Andy > > > Lucas De Marchi -- Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Hi Rusty, On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell wrote: > Andy Lutomirski writes: >> On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell wrote: >>> Andy Lutomirski writes: On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell wrote: > 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. > ... 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. > > What does everyone think of this? Jon, Lucas, does this match your > experience? > > Thanks, > Rusty. > > Subject: modules: don't fail to load on unknown parameters. > > Although parameters are supposed to be part of the kernel API, experimental > parameters are often removed. In addition, downgrading a kernel might cause > previously-working modules to fail to load. I agree with this reasoning > > On balance, it's probably better to warn, and load the module anyway. However loading the module anyway would bring at least one drawback: if the user made a typo when passing the option the module would load anyway and he will probably not even look in the log, since there's was no errors from modprobe. For finit_module we could put a flag to trigger this behavior and propagate it to modprobe, but this is not possible with init_module(). I can't think in any other option right now... do you have any? Lucas De Marchi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Tue, 2013-03-19 at 13:02 +1030, Rusty Russell wrote: > Andy Lutomirski writes: > > On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell > > wrote: > >> Andy Lutomirski writes: > >>> On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell > >>> wrote: > 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. > ... > >>> 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. > > What does everyone think of this? Jon, Lucas, does this match your > experience? I'm not sure why I'm being cc'd on this, though I did recently remove a module parameter (sfc.rx_alloc_method). For what it's worth: > Subject: modules: don't fail to load on unknown parameters. > > Although parameters are supposed to be part of the kernel API, experimental > parameters are often removed. In addition, downgrading a kernel might cause > previously-working modules to fail to load. > > On balance, it's probably better to warn, and load the module anyway. I agree with this. > Reported-by: Andy Lutomirski > Signed-off-by: Rusty Russell [...] This should also go to stable, so the downgrading issue doesn't continue to bite people. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Tue, 2013-03-19 at 13:02 +1030, Rusty Russell wrote: Andy Lutomirski l...@amacapital.net writes: On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@amacapital.net writes: On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell ru...@rustcorp.com.au wrote: 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. ... 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. What does everyone think of this? Jon, Lucas, does this match your experience? I'm not sure why I'm being cc'd on this, though I did recently remove a module parameter (sfc.rx_alloc_method). For what it's worth: Subject: modules: don't fail to load on unknown parameters. Although parameters are supposed to be part of the kernel API, experimental parameters are often removed. In addition, downgrading a kernel might cause previously-working modules to fail to load. On balance, it's probably better to warn, and load the module anyway. I agree with this. Reported-by: Andy Lutomirski l...@amacapital.net Signed-off-by: Rusty Russell ru...@rustcorp.com.au [...] This should also go to stable, so the downgrading issue doesn't continue to bite people. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Hi Rusty, On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@amacapital.net writes: On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@amacapital.net writes: On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell ru...@rustcorp.com.au wrote: 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. ... 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. What does everyone think of this? Jon, Lucas, does this match your experience? Thanks, Rusty. Subject: modules: don't fail to load on unknown parameters. Although parameters are supposed to be part of the kernel API, experimental parameters are often removed. In addition, downgrading a kernel might cause previously-working modules to fail to load. I agree with this reasoning On balance, it's probably better to warn, and load the module anyway. However loading the module anyway would bring at least one drawback: if the user made a typo when passing the option the module would load anyway and he will probably not even look in the log, since there's was no errors from modprobe. For finit_module we could put a flag to trigger this behavior and propagate it to modprobe, but this is not possible with init_module(). I can't think in any other option right now... do you have any? Lucas De Marchi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Tue, Mar 19, 2013 at 8:26 PM, Lucas De Marchi lucas.demar...@profusion.mobi wrote: Hi Rusty, On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@amacapital.net writes: On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@amacapital.net writes: On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell ru...@rustcorp.com.au wrote: 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. ... 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. What does everyone think of this? Jon, Lucas, does this match your experience? Thanks, Rusty. Subject: modules: don't fail to load on unknown parameters. Although parameters are supposed to be part of the kernel API, experimental parameters are often removed. In addition, downgrading a kernel might cause previously-working modules to fail to load. I agree with this reasoning On balance, it's probably better to warn, and load the module anyway. However loading the module anyway would bring at least one drawback: if the user made a typo when passing the option the module would load anyway and he will probably not even look in the log, since there's was no errors from modprobe. For finit_module we could put a flag to trigger this behavior and propagate it to modprobe, but this is not possible with init_module(). I can't think in any other option right now... do you have any? Have a different finit_module return value for successfully loaded, but there were warnings perhaps? --Andy Lucas De Marchi -- Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Tue, Mar 19, 2013 at 8:32 PM, Andy Lutomirski l...@amacapital.net wrote: On Tue, Mar 19, 2013 at 8:26 PM, Lucas De Marchi lucas.demar...@profusion.mobi wrote: Hi Rusty, On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@amacapital.net writes: On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@amacapital.net writes: On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell ru...@rustcorp.com.au wrote: 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. ... 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. What does everyone think of this? Jon, Lucas, does this match your experience? Thanks, Rusty. Subject: modules: don't fail to load on unknown parameters. Although parameters are supposed to be part of the kernel API, experimental parameters are often removed. In addition, downgrading a kernel might cause previously-working modules to fail to load. I agree with this reasoning On balance, it's probably better to warn, and load the module anyway. However loading the module anyway would bring at least one drawback: if the user made a typo when passing the option the module would load anyway and he will probably not even look in the log, since there's was no errors from modprobe. For finit_module we could put a flag to trigger this behavior and propagate it to modprobe, but this is not possible with init_module(). I can't think in any other option right now... do you have any? Have a different finit_module return value for successfully loaded, but there were warnings perhaps? Never mind. I was thinking that finit_module was new in 3.9. --Andy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Ben Hutchings bhutchi...@solarflare.com writes: This should also go to stable, so the downgrading issue doesn't continue to bite people. Andy was complaining about experimental params going away: I haven't heard a single complaint about the downgrading issue. I think it's a nice to have, which is why I mentioned it. I also CC'd the two maintainers most likely to know if it *is* a current problem. And note that this code has been this way for over ten years! No bug report, no cc:stable. What if people were relying on detecting module parameters by the load failing? I'd rather find out when they upgrade to a new kernel instead of poisoning the old ones, too. Cheers, Rusty. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Lucas De Marchi lucas.demar...@profusion.mobi writes: Hi Rusty, On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@amacapital.net writes: On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@amacapital.net writes: On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell ru...@rustcorp.com.au wrote: 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. ... 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. What does everyone think of this? Jon, Lucas, does this match your experience? Thanks, Rusty. Subject: modules: don't fail to load on unknown parameters. Although parameters are supposed to be part of the kernel API, experimental parameters are often removed. In addition, downgrading a kernel might cause previously-working modules to fail to load. I agree with this reasoning On balance, it's probably better to warn, and load the module anyway. However loading the module anyway would bring at least one drawback: if the user made a typo when passing the option the module would load anyway and he will probably not even look in the log, since there's was no errors from modprobe. For finit_module we could put a flag to trigger this behavior and propagate it to modprobe, but this is not possible with init_module(). I can't think in any other option right now... do you have any? No good ones :( MODULE_PARM_DESC isn't compulsory, so you can't rely on that to tell you about option names. Even if we had a flag, how would you know to set it? I guess you could try without then try with, and if it works the second time print a warning about typos. But it's still pretty ugly. We could implement such a flag with a fake IGNORE_BAD_PARAMS parameter, for example. That would fail nicely on older kernels, too. Hmmm Rusty. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Andy Lutomirski writes:
> On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell wrote:
>> Andy Lutomirski writes:
>>> On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell
>>> wrote:
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.
...
>>> 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.
What does everyone think of this? Jon, Lucas, does this match your
experience?
Thanks,
Rusty.
Subject: modules: don't fail to load on unknown parameters.
Although parameters are supposed to be part of the kernel API, experimental
parameters are often removed. In addition, downgrading a kernel might cause
previously-working modules to fail to load.
On balance, it's probably better to warn, and load the module anyway.
Reported-by: Andy Lutomirski
Signed-off-by: Rusty Russell
diff --git a/kernel/module.c b/kernel/module.c
index 3c2c72d..46db10a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3206,6 +3206,17 @@ out:
return err;
}
+static int unknown_module_param_cb(char *param, char *val, const char *modname)
+{
+ /* Check for magic 'dyndbg' arg */
+ int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
+ if (ret != 0) {
+ printk(KERN_WARNING "%s: unknown parameter '%s' ignored\n",
+ modname, param);
+ }
+ return 0;
+}
+
/* Allocate and load the module: note that size of section 0 is always
zero, and we rely on this for optional sections. */
static int load_module(struct load_info *info, const char __user *uargs,
@@ -3292,7 +3303,7 @@ static int load_module(struct load_info *info, const char
__user *uargs,
/* Module is ready to execute: parsing args may do that. */
err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
--32768, 32767, _dyndbg_module_param_cb);
+-32768, 32767, unknown_module_param_cb);
if (err < 0)
goto bug_cleanup;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell wrote: > Andy Lutomirski writes: >> On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell >> wrote: >>> Andy Lutomirski 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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@amacapital.net writes: On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Andy Lutomirski l...@amacapital.net writes:
On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell ru...@rustcorp.com.au wrote:
Andy Lutomirski l...@amacapital.net writes:
On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell ru...@rustcorp.com.au
wrote:
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.
...
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.
What does everyone think of this? Jon, Lucas, does this match your
experience?
Thanks,
Rusty.
Subject: modules: don't fail to load on unknown parameters.
Although parameters are supposed to be part of the kernel API, experimental
parameters are often removed. In addition, downgrading a kernel might cause
previously-working modules to fail to load.
On balance, it's probably better to warn, and load the module anyway.
Reported-by: Andy Lutomirski l...@amacapital.net
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
diff --git a/kernel/module.c b/kernel/module.c
index 3c2c72d..46db10a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3206,6 +3206,17 @@ out:
return err;
}
+static int unknown_module_param_cb(char *param, char *val, const char *modname)
+{
+ /* Check for magic 'dyndbg' arg */
+ int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
+ if (ret != 0) {
+ printk(KERN_WARNING %s: unknown parameter '%s' ignored\n,
+ modname, param);
+ }
+ return 0;
+}
+
/* Allocate and load the module: note that size of section 0 is always
zero, and we rely on this for optional sections. */
static int load_module(struct load_info *info, const char __user *uargs,
@@ -3292,7 +3303,7 @@ static int load_module(struct load_info *info, const char
__user *uargs,
/* Module is ready to execute: parsing args may do that. */
err = parse_args(mod-name, mod-args, mod-kp, mod-num_kp,
--32768, 32767, ddebug_dyndbg_module_param_cb);
+-32768, 32767, unknown_module_param_cb);
if (err 0)
goto bug_cleanup;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Andy Lutomirski writes:
> On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell wrote:
>> Andy Lutomirski 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.
> 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?
> In any case, I find it odd that the only parameters you can set that
> cause errors when the parameter is deleted are parameters for modules
> that are built as modules.
We could definitely expand the check: we should check for unknown
parameters to builtin modules.
This refers back to my question on lkml 'Re: No sysfs directory for
openvswitch module when built-in' as the kernel doesn't currently know
what modules are builtin. We intuit it from parameter names, which is
sloppy.
Here's a rough patch. Maybe someone on linux-kbuild can tell me why it
fails?
DOESNT_WORK: kernel: generate list of builtin modnames.
For some reason, the only contents of modules.builtin when this runs is
one line "kernel/kernel/configs.ko"?
This lets us emit an error when invalid boot parameters are used, since
we know what names can never be external modules.
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
Re: [RFC PATCH] Allow optional module parameters
Andy Lutomirski l...@amacapital.net writes:
On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell ru...@rustcorp.com.au wrote:
Andy Lutomirski l...@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.
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?
In any case, I find it odd that the only parameters you can set that
cause errors when the parameter is deleted are parameters for modules
that are built as modules.
We could definitely expand the check: we should check for unknown
parameters to builtin modules.
This refers back to my question on lkml 'Re: No sysfs directory for
openvswitch module when built-in' as the kernel doesn't currently know
what modules are builtin. We intuit it from parameter names, which is
sloppy.
Here's a rough patch. Maybe someone on linux-kbuild can tell me why it
fails?
DOESNT_WORK: kernel: generate list of builtin modnames.
For some reason, the only contents of modules.builtin when this runs is
one line kernel/kernel/configs.ko?
This lets us emit an error when invalid boot parameters are used, since
we know what names can never be external modules.
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 \
Re: [RFC PATCH] Allow optional module parameters
On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell wrote: > Andy Lutomirski 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. 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). In any case, I find it odd that the only parameters you can set that cause errors when the parameter is deleted are parameters for modules that are built as modules. > >> With this patch, module parameters can be made explicitly optional. >> This approach is IMO silly, but it's unlikely to break anything, >> since I doubt that anyone needs init parameters or init environment >> variables that end in a tilde. > > It's silly for the removal problem: that should be handled in the > kernel. How would the poor user know that the option is going away? > So how about we add a module_param_obsolete(name) macro? > > If a parameter were introduced, and the user wanted to specify it *if* > it was supported, that might justify this approach rather than using > complex install commands. But I don't believe that's common, is it? > It's happened multiple times to me with things like pcie_aspm.force_aspm and i915.whatever. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell ru...@rustcorp.com.au wrote: Andy Lutomirski l...@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. 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). In any case, I find it odd that the only parameters you can set that cause errors when the parameter is deleted are parameters for modules that are built as modules. With this patch, module parameters can be made explicitly optional. This approach is IMO silly, but it's unlikely to break anything, since I doubt that anyone needs init parameters or init environment variables that end in a tilde. It's silly for the removal problem: that should be handled in the kernel. How would the poor user know that the option is going away? So how about we add a module_param_obsolete(name) macro? If a parameter were introduced, and the user wanted to specify it *if* it was supported, that might justify this approach rather than using complex install commands. But I don't believe that's common, is it? It's happened multiple times to me with things like pcie_aspm.force_aspm and i915.whatever. --Andy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Andy Lutomirski 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? > With this patch, module parameters can be made explicitly optional. > This approach is IMO silly, but it's unlikely to break anything, > since I doubt that anyone needs init parameters or init environment > variables that end in a tilde. It's silly for the removal problem: that should be handled in the kernel. How would the poor user know that the option is going away? So how about we add a module_param_obsolete(name) macro? If a parameter were introduced, and the user wanted to specify it *if* it was supported, that might justify this approach rather than using complex install commands. But I don't believe that's common, is it? Thanks, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Allow optional module parameters
Andy Lutomirski l...@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? With this patch, module parameters can be made explicitly optional. This approach is IMO silly, but it's unlikely to break anything, since I doubt that anyone needs init parameters or init environment variables that end in a tilde. It's silly for the removal problem: that should be handled in the kernel. How would the poor user know that the option is going away? So how about we add a module_param_obsolete(name) macro? If a parameter were introduced, and the user wanted to specify it *if* it was supported, that might justify this approach rather than using complex install commands. But I don't believe that's common, is it? Thanks, Rusty. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
