All of lore.kernel.org
 help / color / mirror / Atom feed
* init: multi param parsing regression since 3.4
       [not found] <1002174455.1843950.1338570468781.JavaMail.root@advansee.com>
@ 2012-06-01 17:26 ` Benoît Thébaudeau
  2012-06-02 15:50     ` Benoît Thébaudeau
  2012-06-04  4:32   ` Rusty Russell
  0 siblings, 2 replies; 10+ messages in thread
From: Benoît Thébaudeau @ 2012-06-01 17:26 UTC (permalink / raw)
  To: Pawel Moll, Rusty Russell; +Cc: benoit thebaudeau, linux-kernel

Hi Pawel, Rusty, all,

Since linux 3.4 (OK with 3.3.6), I get the following regression for UBI (built
in kernel):
[    1.327228] UBI: attaching mtd4 to ubi0
[    1.331082] UBI: physical eraseblock size:   131072 bytes (128 KiB)
[    1.337388] UBI: logical eraseblock size:    129024 bytes
[    1.342794] UBI: smallest flash I/O unit:    2048
[    1.347518] UBI: sub-page size:              512
[    1.352144] UBI: VID header offset:          512 (aligned 512)
[    1.357996] UBI: data offset:                2048
[    1.538686] UBI: max. sequence number:       456
[    1.551271] UBI: attached mtd4 to ubi0
[    1.555085] UBI: MTD device name:            "rootfs"
[    1.560147] UBI: MTD device size:            123 MiB
[    1.565133] UBI: number of good PEBs:        980
[    1.569757] UBI: number of bad PEBs:         4
[    1.574206] UBI: number of corrupted PEBs:   0
[    1.578669] UBI: max. allowed volumes:       128
[    1.583293] UBI: wear-leveling threshold:    4096
[    1.588015] UBI: number of internal volumes: 1
[    1.592465] UBI: number of user volumes:     1
[    1.596927] UBI: available PEBs:             0
[    1.601377] UBI: total number of reserved PEBs: 980
[    1.606273] UBI: number of PEBs reserved for bad PEB handling: 9
[    1.612288] UBI: max/mean erase counter: 3/1
[    1.616576] UBI: image sequence number:  1970526931
[    1.621483] UBI: background thread "ubi_bgt0d" started, PID 32
[    1.627362] UBI error: ubi_init: cannot attach mtd4

After some debugging, I found that this is caused by "ubi.mtd=rootfs" in my
kernel command line being parsed twice while appearing once in this line.

The root cause is "parse_args(initcall_level_names[level], ..." that you added
to init/main.c in commit #026cee0086fe1df4cf74691cf273062cc769617d, because
level 0 is shared by "Booting kernel" and "early parameters".

This may also cause issues for other modules.

Should "parse_args("Booting kernel", ..." have been removed with your commit?

OR

Should do_initcall_level() call parse_args() starting at level 1?

OR

Should the UBI module register to another level?

OR

It doesn't matter and it's up to the modules to be robust to multi param
parsing?

Regards,
Benoît (Please keep me Cc'ed)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: init: multi param parsing regression since 3.4
  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-04  4:32   ` Rusty Russell
  1 sibling, 0 replies; 10+ messages in thread
From: Benoît Thébaudeau @ 2012-06-02 15:50 UTC (permalink / raw)
  To: Pawel Moll, Rusty Russell, Artem Bityutskiy, Linus Torvalds
  Cc: linux-kernel, linux-mtd, benoit thebaudeau

Hi all,

On Fri, Jun 1, 2012 at 7:26:51PM +0200, Benoît Thébaudeau wrote:
> Hi Pawel, Rusty, all,
> 
> Since linux 3.4 (OK with 3.3.6), I get the following regression for
> UBI (built
> in kernel):
> [    1.327228] UBI: attaching mtd4 to ubi0
> [    1.331082] UBI: physical eraseblock size:   131072 bytes (128
> KiB)
> [    1.337388] UBI: logical eraseblock size:    129024 bytes
> [    1.342794] UBI: smallest flash I/O unit:    2048
> [    1.347518] UBI: sub-page size:              512
> [    1.352144] UBI: VID header offset:          512 (aligned 512)
> [    1.357996] UBI: data offset:                2048
> [    1.538686] UBI: max. sequence number:       456
> [    1.551271] UBI: attached mtd4 to ubi0
> [    1.555085] UBI: MTD device name:            "rootfs"
> [    1.560147] UBI: MTD device size:            123 MiB
> [    1.565133] UBI: number of good PEBs:        980
> [    1.569757] UBI: number of bad PEBs:         4
> [    1.574206] UBI: number of corrupted PEBs:   0
> [    1.578669] UBI: max. allowed volumes:       128
> [    1.583293] UBI: wear-leveling threshold:    4096
> [    1.588015] UBI: number of internal volumes: 1
> [    1.592465] UBI: number of user volumes:     1
> [    1.596927] UBI: available PEBs:             0
> [    1.601377] UBI: total number of reserved PEBs: 980
> [    1.606273] UBI: number of PEBs reserved for bad PEB handling: 9
> [    1.612288] UBI: max/mean erase counter: 3/1
> [    1.616576] UBI: image sequence number:  1970526931
> [    1.621483] UBI: background thread "ubi_bgt0d" started, PID 32
> [    1.627362] UBI error: ubi_init: cannot attach mtd4
> 
> After some debugging, I found that this is caused by "ubi.mtd=rootfs"
> in my
> kernel command line being parsed twice while appearing once in this
> line.
> 
> The root cause is "parse_args(initcall_level_names[level], ..." that
> you added
> to init/main.c in commit #026cee0086fe1df4cf74691cf273062cc769617d,
> because
> level 0 is shared by "Booting kernel" and "early parameters".
> 
> This may also cause issues for other modules.
> 
> Should "parse_args("Booting kernel", ..." have been removed with your
> commit?
> 
> OR
> 
> Should do_initcall_level() call parse_args() starting at level 1?
> 
> OR
> 
> Should the UBI module register to another level?
> 
> OR
> 
> It doesn't matter and it's up to the modules to be robust to multi
> param
> parsing?

I forgot to mention that the only consequence for UBI is a spurious error
message: The MTD device is correctly attached the 1st time, and the extra
attachment attempt is simply discarded with this message. This might be worse
for other modules.

Regards,
Benoît

(Please keep me Cc'ed: benoit dot thebaudeau at advansee dot com)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: init: multi param parsing regression since 3.4
@ 2012-06-02 15:50     ` Benoît Thébaudeau
  0 siblings, 0 replies; 10+ messages in thread
From: Benoît Thébaudeau @ 2012-06-02 15:50 UTC (permalink / raw)
  To: Pawel Moll, Rusty Russell, Artem Bityutskiy, Linus Torvalds
  Cc: benoit thebaudeau, linux-mtd, linux-kernel

Hi all,

On Fri, Jun 1, 2012 at 7:26:51PM +0200, Benoît Thébaudeau wrote:
> Hi Pawel, Rusty, all,
> 
> Since linux 3.4 (OK with 3.3.6), I get the following regression for
> UBI (built
> in kernel):
> [    1.327228] UBI: attaching mtd4 to ubi0
> [    1.331082] UBI: physical eraseblock size:   131072 bytes (128
> KiB)
> [    1.337388] UBI: logical eraseblock size:    129024 bytes
> [    1.342794] UBI: smallest flash I/O unit:    2048
> [    1.347518] UBI: sub-page size:              512
> [    1.352144] UBI: VID header offset:          512 (aligned 512)
> [    1.357996] UBI: data offset:                2048
> [    1.538686] UBI: max. sequence number:       456
> [    1.551271] UBI: attached mtd4 to ubi0
> [    1.555085] UBI: MTD device name:            "rootfs"
> [    1.560147] UBI: MTD device size:            123 MiB
> [    1.565133] UBI: number of good PEBs:        980
> [    1.569757] UBI: number of bad PEBs:         4
> [    1.574206] UBI: number of corrupted PEBs:   0
> [    1.578669] UBI: max. allowed volumes:       128
> [    1.583293] UBI: wear-leveling threshold:    4096
> [    1.588015] UBI: number of internal volumes: 1
> [    1.592465] UBI: number of user volumes:     1
> [    1.596927] UBI: available PEBs:             0
> [    1.601377] UBI: total number of reserved PEBs: 980
> [    1.606273] UBI: number of PEBs reserved for bad PEB handling: 9
> [    1.612288] UBI: max/mean erase counter: 3/1
> [    1.616576] UBI: image sequence number:  1970526931
> [    1.621483] UBI: background thread "ubi_bgt0d" started, PID 32
> [    1.627362] UBI error: ubi_init: cannot attach mtd4
> 
> After some debugging, I found that this is caused by "ubi.mtd=rootfs"
> in my
> kernel command line being parsed twice while appearing once in this
> line.
> 
> The root cause is "parse_args(initcall_level_names[level], ..." that
> you added
> to init/main.c in commit #026cee0086fe1df4cf74691cf273062cc769617d,
> because
> level 0 is shared by "Booting kernel" and "early parameters".
> 
> This may also cause issues for other modules.
> 
> Should "parse_args("Booting kernel", ..." have been removed with your
> commit?
> 
> OR
> 
> Should do_initcall_level() call parse_args() starting at level 1?
> 
> OR
> 
> Should the UBI module register to another level?
> 
> OR
> 
> It doesn't matter and it's up to the modules to be robust to multi
> param
> parsing?

I forgot to mention that the only consequence for UBI is a spurious error
message: The MTD device is correctly attached the 1st time, and the extra
attachment attempt is simply discarded with this message. This might be worse
for other modules.

Regards,
Benoît

(Please keep me Cc'ed: benoit dot thebaudeau at advansee dot com)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: init: multi param parsing regression since 3.4
  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-04  4:32   ` Rusty Russell
  2012-06-04 12:38       ` Benoît Thébaudeau
                       ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Rusty Russell @ 2012-06-04  4:32 UTC (permalink / raw)
  To: Benoît Thébaudeau, Pawel Moll; +Cc: benoit thebaudeau, linux-kernel

On Fri, 1 Jun 2012 19:26:51 +0200 (CEST), Benoît Thébaudeau <benoit.thebaudeau@advansee.com> wrote:
> Hi Pawel, Rusty, all,
> 
...
> After some debugging, I found that this is caused by "ubi.mtd=rootfs" in my
> kernel command line being parsed twice while appearing once in this line.

Weird, that shouldn't happen.

> The root cause is "parse_args(initcall_level_names[level], ..." that you added
> to init/main.c in commit #026cee0086fe1df4cf74691cf273062cc769617d, because
> level 0 is shared by "Booting kernel" and "early parameters".

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, &param_ops_##type, &var, perm, 0)
+	__module_param_call("", name, &param_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,			\
 			    &param_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,			\
 			    &param_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();
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: init: multi param parsing regression since 3.4
  2012-06-04  4:32   ` Rusty Russell
@ 2012-06-04 12:38       ` Benoît Thébaudeau
  2012-06-04 20:39     ` Linus Torvalds
  2012-06-06  8:28     ` Artem Bityutskiy
  2 siblings, 0 replies; 10+ messages in thread
From: Benoît Thébaudeau @ 2012-06-04 12:38 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Pawel Moll, Artem Bityutskiy, Linus Torvalds, linux-kernel,
	linux-mtd, benoit thebaudeau

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, &param_ops_##type, &var, perm, 0)
> +	__module_param_call("", name, &param_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,			\
>  			    &param_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,			\
>  			    &param_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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: init: multi param parsing regression since 3.4
@ 2012-06-04 12:38       ` Benoît Thébaudeau
  0 siblings, 0 replies; 10+ messages in thread
From: Benoît Thébaudeau @ 2012-06-04 12:38 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Pawel Moll, Artem Bityutskiy, linux-kernel, benoit thebaudeau,
	linux-mtd, Linus Torvalds

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, &param_ops_##type, &var, perm, 0)
> +	__module_param_call("", name, &param_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,			\
>  			    &param_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,			\
>  			    &param_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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: init: multi param parsing regression since 3.4
  2012-06-04  4:32   ` Rusty Russell
  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-06  8:28     ` Artem Bityutskiy
  2 siblings, 1 reply; 10+ messages in thread
From: Linus Torvalds @ 2012-06-04 20:39 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Benoît Thébaudeau, Pawel Moll, linux-kernel

On Sun, Jun 3, 2012 at 9:32 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> Subject: module_param: stop double-calling parameters.
> From: Rusty Russell <rusty@rustcorp.com.au>

Rusy, should I take this (with the ubi/build.c part removed) from the
email - along with the initcall debug printk removals - or can I just
expect to get a pull request with these things?

                Linus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: init: multi param parsing regression since 3.4
  2012-06-04 20:39     ` Linus Torvalds
@ 2012-06-05  0:17       ` Rusty Russell
  2012-06-05  1:25         ` Linus Torvalds
  0 siblings, 1 reply; 10+ messages in thread
From: Rusty Russell @ 2012-06-05  0:17 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Benoît Thébaudeau, Pawel Moll, linux-kernel

On Mon, 4 Jun 2012 13:39:27 -0700, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sun, Jun 3, 2012 at 9:32 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> >
> > Subject: module_param: stop double-calling parameters.
> > From: Rusty Russell <rusty@rustcorp.com.au>
> 
> Rusy, should I take this (with the ubi/build.c part removed) from the
> email - along with the initcall debug printk removals - or can I just
> expect to get a pull request with these things?
> 
>                 Linus

I like to spin this stuff for a day through linux-next, which should be
happening right now.

But if you want to save me the trouble of a pull request, sure.

Thanks,
Rusty.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: init: multi param parsing regression since 3.4
  2012-06-05  0:17       ` Rusty Russell
@ 2012-06-05  1:25         ` Linus Torvalds
  0 siblings, 0 replies; 10+ messages in thread
From: Linus Torvalds @ 2012-06-05  1:25 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Benoît Thébaudeau, Pawel Moll, linux-kernel

On Mon, Jun 4, 2012 at 5:17 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> I like to spin this stuff for a day through linux-next, which should be
> happening right now.

Good.

> But if you want to save me the trouble of a pull request, sure.

No, I just wanted to make sure I can forget about these threads..

       Linus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: init: multi param parsing regression since 3.4
  2012-06-04  4:32   ` Rusty Russell
  2012-06-04 12:38       ` Benoît Thébaudeau
  2012-06-04 20:39     ` Linus Torvalds
@ 2012-06-06  8:28     ` Artem Bityutskiy
  2 siblings, 0 replies; 10+ messages in thread
From: Artem Bityutskiy @ 2012-06-06  8:28 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Benoît Thébaudeau, Pawel Moll, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 252 bytes --]

On Mon, 2012-06-04 at 14:02 +0930, Rusty Russell wrote:
> +	printk("ubi_mtd_param_parse: val = %s\n", val);
> +	WARN_ON(1);
>  	if (!val)
>  		return -EINVAL;

Hi, please, zap this printk() and WARN_ON().

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-06-06  8:24 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [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
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

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.