From: "Benoît Thébaudeau" <benoit.thebaudeau@advansee.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Pawel Moll <pawel.moll@arm.com>,
Artem Bityutskiy <dedekind1@gmail.com>,
linux-kernel@vger.kernel.org,
benoit thebaudeau <benoit.thebaudeau@advansee.com>,
linux-mtd@lists.infradead.org,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: init: multi param parsing regression since 3.4
Date: Mon, 4 Jun 2012 14:38:51 +0200 (CEST) [thread overview]
Message-ID: <28190110.1966162.1338813531173.JavaMail.root@advansee.com> (raw)
In-Reply-To: <87ipf71ytx.fsf@rustcorp.com.au>
On Mon, Jun 4, 2012 at 6:32:42AM, Rusty Russell wrote:
> Erk, I tested it and you're right. Level 0 is 'early' initcalls, but
> traditional module parameters are called even earlier. Simplest fix
> is to move the default parameters to -1.
>
> This works for me:
>
> Subject: module_param: stop double-calling parameters.
> From: Rusty Russell <rusty@rustcorp.com.au>
>
> Commit 026cee0086fe1df4cf74691cf273062cc769617d "params:
> <level>_initcall-like kernel parameters" set old-style module
> parameters to level 0. And we call those level 0 calls where we used
> to, early in start_kernel().
>
> We also loop through the initcall levels and call the levelled
> module_params before the corresponding initcall. Unfortunately level
> 0 is early_init(), so we call the standard module_param calls twice.
>
> (Turns out most things don't care, but at least ubi.mtd does).
>
> Change the level to -1 for standard module_param calls.
>
> Reported-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> Cc: stable@kernel.org
>
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -1293,6 +1293,8 @@ static int __init ubi_mtd_param_parse(co
> char *pbuf = &buf[0];
> char *tokens[2] = {NULL, NULL};
>
> + printk("ubi_mtd_param_parse: val = %s\n", val);
> + WARN_ON(1);
> if (!val)
> return -EINVAL;
>
> diff --git a/include/linux/moduleparam.h
> b/include/linux/moduleparam.h
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -128,7 +128,7 @@ struct kparam_array
> * The ops can have NULL set or get functions.
> */
> #define module_param_cb(name, ops, arg, perm) \
> - __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, 0)
> + __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1)
>
> /**
> * <level>_param_cb - general callback for a module/cmdline
> parameter
> @@ -192,7 +192,7 @@ struct kparam_array
> { (void *)set, (void *)get }; \
> __module_param_call(MODULE_PARAM_PREFIX, \
> name, &__param_ops_##name, arg, \
> - (perm) + sizeof(__check_old_set_param(set))*0, 0)
> + (perm) + sizeof(__check_old_set_param(set))*0, -1)
>
> /* We don't get oldget: it's often a new-style param_get_uint, etc.
> */
> static inline int
> @@ -272,7 +272,7 @@ static inline void __kernel_param_unlock
> */
> #define core_param(name, var, type, perm) \
> param_check_##type(name, &(var)); \
> - __module_param_call("", name, ¶m_ops_##type, &var, perm, 0)
> + __module_param_call("", name, ¶m_ops_##type, &var, perm, -1)
> #endif /* !MODULE */
>
> /**
> @@ -290,7 +290,7 @@ static inline void __kernel_param_unlock
> = { len, string }; \
> __module_param_call(MODULE_PARAM_PREFIX, name, \
> ¶m_ops_string, \
> - .str = &__param_string_##name, perm, 0); \
> + .str = &__param_string_##name, perm, -1); \
> __MODULE_PARM_TYPE(name, "string")
>
> /**
> @@ -432,7 +432,7 @@ extern int param_set_bint(const char *va
> __module_param_call(MODULE_PARAM_PREFIX, name, \
> ¶m_array_ops, \
> .arr = &__param_arr_##name, \
> - perm, 0); \
> + perm, -1); \
> __MODULE_PARM_TYPE(name, "array of " #type)
>
> extern struct kernel_param_ops param_array_ops;
> diff --git a/init/main.c b/init/main.c
> --- a/init/main.c
> +++ b/init/main.c
> @@ -508,7 +508,7 @@ asmlinkage void __init start_kernel(void
> parse_early_param();
> parse_args("Booting kernel", static_command_line, __start___param,
> __stop___param - __start___param,
> - 0, 0, &unknown_bootoption);
> + -1, -1, &unknown_bootoption);
>
> jump_label_init();
>
>
It works for me too. But I think you can remove the 1st hunk, which seems to be
a leftover from your tests.
Regards,
Benoît
WARNING: multiple messages have this Message-ID (diff)
From: "Benoît Thébaudeau" <benoit.thebaudeau@advansee.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Pawel Moll <pawel.moll@arm.com>,
Artem Bityutskiy <dedekind1@gmail.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
benoit thebaudeau <benoit.thebaudeau@advansee.com>
Subject: Re: init: multi param parsing regression since 3.4
Date: Mon, 4 Jun 2012 14:38:51 +0200 (CEST) [thread overview]
Message-ID: <28190110.1966162.1338813531173.JavaMail.root@advansee.com> (raw)
In-Reply-To: <87ipf71ytx.fsf@rustcorp.com.au>
On Mon, Jun 4, 2012 at 6:32:42AM, Rusty Russell wrote:
> Erk, I tested it and you're right. Level 0 is 'early' initcalls, but
> traditional module parameters are called even earlier. Simplest fix
> is to move the default parameters to -1.
>
> This works for me:
>
> Subject: module_param: stop double-calling parameters.
> From: Rusty Russell <rusty@rustcorp.com.au>
>
> Commit 026cee0086fe1df4cf74691cf273062cc769617d "params:
> <level>_initcall-like kernel parameters" set old-style module
> parameters to level 0. And we call those level 0 calls where we used
> to, early in start_kernel().
>
> We also loop through the initcall levels and call the levelled
> module_params before the corresponding initcall. Unfortunately level
> 0 is early_init(), so we call the standard module_param calls twice.
>
> (Turns out most things don't care, but at least ubi.mtd does).
>
> Change the level to -1 for standard module_param calls.
>
> Reported-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> Cc: stable@kernel.org
>
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -1293,6 +1293,8 @@ static int __init ubi_mtd_param_parse(co
> char *pbuf = &buf[0];
> char *tokens[2] = {NULL, NULL};
>
> + printk("ubi_mtd_param_parse: val = %s\n", val);
> + WARN_ON(1);
> if (!val)
> return -EINVAL;
>
> diff --git a/include/linux/moduleparam.h
> b/include/linux/moduleparam.h
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -128,7 +128,7 @@ struct kparam_array
> * The ops can have NULL set or get functions.
> */
> #define module_param_cb(name, ops, arg, perm) \
> - __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, 0)
> + __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1)
>
> /**
> * <level>_param_cb - general callback for a module/cmdline
> parameter
> @@ -192,7 +192,7 @@ struct kparam_array
> { (void *)set, (void *)get }; \
> __module_param_call(MODULE_PARAM_PREFIX, \
> name, &__param_ops_##name, arg, \
> - (perm) + sizeof(__check_old_set_param(set))*0, 0)
> + (perm) + sizeof(__check_old_set_param(set))*0, -1)
>
> /* We don't get oldget: it's often a new-style param_get_uint, etc.
> */
> static inline int
> @@ -272,7 +272,7 @@ static inline void __kernel_param_unlock
> */
> #define core_param(name, var, type, perm) \
> param_check_##type(name, &(var)); \
> - __module_param_call("", name, ¶m_ops_##type, &var, perm, 0)
> + __module_param_call("", name, ¶m_ops_##type, &var, perm, -1)
> #endif /* !MODULE */
>
> /**
> @@ -290,7 +290,7 @@ static inline void __kernel_param_unlock
> = { len, string }; \
> __module_param_call(MODULE_PARAM_PREFIX, name, \
> ¶m_ops_string, \
> - .str = &__param_string_##name, perm, 0); \
> + .str = &__param_string_##name, perm, -1); \
> __MODULE_PARM_TYPE(name, "string")
>
> /**
> @@ -432,7 +432,7 @@ extern int param_set_bint(const char *va
> __module_param_call(MODULE_PARAM_PREFIX, name, \
> ¶m_array_ops, \
> .arr = &__param_arr_##name, \
> - perm, 0); \
> + perm, -1); \
> __MODULE_PARM_TYPE(name, "array of " #type)
>
> extern struct kernel_param_ops param_array_ops;
> diff --git a/init/main.c b/init/main.c
> --- a/init/main.c
> +++ b/init/main.c
> @@ -508,7 +508,7 @@ asmlinkage void __init start_kernel(void
> parse_early_param();
> parse_args("Booting kernel", static_command_line, __start___param,
> __stop___param - __start___param,
> - 0, 0, &unknown_bootoption);
> + -1, -1, &unknown_bootoption);
>
> jump_label_init();
>
>
It works for me too. But I think you can remove the 1st hunk, which seems to be
a leftover from your tests.
Regards,
Benoît
next prev parent reply other threads:[~2012-06-04 12:34 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1002174455.1843950.1338570468781.JavaMail.root@advansee.com>
2012-06-01 17:26 ` init: multi param parsing regression since 3.4 Benoît Thébaudeau
2012-06-02 15:50 ` Benoît Thébaudeau
2012-06-02 15:50 ` Benoît Thébaudeau
2012-06-04 4:32 ` Rusty Russell
2012-06-04 12:38 ` Benoît Thébaudeau [this message]
2012-06-04 12:38 ` Benoît Thébaudeau
2012-06-04 20:39 ` Linus Torvalds
2012-06-05 0:17 ` Rusty Russell
2012-06-05 1:25 ` Linus Torvalds
2012-06-06 8:28 ` Artem Bityutskiy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=28190110.1966162.1338813531173.JavaMail.root@advansee.com \
--to=benoit.thebaudeau@advansee.com \
--cc=dedekind1@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=pawel.moll@arm.com \
--cc=rusty@rustcorp.com.au \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.