linux-next: boot problen with next-20110211

linux-next: boot problen with next-20110211

[Stephen Rothwell]

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

Hi all,

Today's linux-next boot test on a Power7 blade produced this message
several times:

sysfs: cannot create duplicate filename '/module/ehea/parameters/Ð'
------------[ cut here ]------------
WARNING: at fs/sysfs/dir.c:455
Modules linked in: ehea(+)
NIP: c0000000001ec990 LR: c0000000001ec98c CTR: c00000000056aa40
REGS: c00000002b7ab710 TRAP: 0700   Not tainted  (2.6.38-rc4-autokern1)
MSR: 8000000000029032 <EE,ME,CE,IR,DR>  CR: 28422484  XER: 00000005
TASK = c000000029379a20[2036] 'modprobe' THREAD: c00000002b7a8000 CPU: 13
GPR00: c0000000001ec98c c00000002b7ab990 c000000000b8a3d0 000000000000004a 
GPR04: 0000000000000000 ffffffffffffffff 0000000000010000 0000000000000000 
GPR08: 000000000a000000 0000000000000000 0000000000020000 0000000000000000 
GPR12: 0000000028422482 c00000000f33d080 00000fff9765d0a0 0000000000000000 
GPR16: 0000010012c800f0 0000010012c817d8 0000000000000000 0000000000000000 
GPR20: 0000010012c813e0 0000010012c801f0 0000000000000205 0000000000002190 
GPR24: fffffffffffffff4 0000000000000000 0000000000000001 c0000003fb85e000 
GPR28: c00000002b7abaa0 c0000003fb760280 c000000000b0abf8 ffffffffffffffef 
NIP [c0000000001ec990] .sysfs_add_one+0xa8/0xe0
LR [c0000000001ec98c] .sysfs_add_one+0xa4/0xe0
Call Trace:
[c00000002b7ab990] [c0000000001ec98c] .sysfs_add_one+0xa4/0xe0 (unreliable)
[c00000002b7aba30] [c0000000001ebf5c] .sysfs_add_file_mode+0x68/0xd4
[c00000002b7abae0] [c0000000001ef734] .internal_create_group+0x15c/0x23c
[c00000002b7abba0] [c0000000000c0528] .module_param_sysfs_setup+0x98/0xd8
[c00000002b7abc50] [c0000000000e1570] .load_module+0xa00/0x1104
[c00000002b7abd90] [c0000000000e1cd4] .SyS_init_module+0x60/0x244
[c00000002b7abe30] [c000000000008628] syscall_exit+0x0/0x40
Instruction dump:
7f64db78 4bffff11 e89e8010 4be527f1 60000000 e89d0018 4be527e5 60000000 
7c641b78 e87e8020 48513c8d 60000000 <0fe00000> 7f63db78 4bf8a079 60000000 
---[ end trace b4c38cc9ca87fe74 ]---

There are no changes to the ehea driver relative to Linus' tree.  ehea
was built as a module and it looks like this is the first module being
loaded.

Maybe the module parameter alignment patches?  That is a very strange
parameter name (it is the same in the other messages).
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: linux-next: boot problen with next-20110211

[Dmitry Torokhov]

Hi Stephen,

On Thu, Feb 10, 2011 at 10:22:54PM -0800, Stephen Rothwell wrote:
> 
> Maybe the module parameter alignment patches?  That is a very strange
> parameter name (it is the same in the other messages).

Yes, I think it is... I wonder why I did not see it locally.  Let me try
to fix that...

Thanks,

Dmitry

Re: linux-next: boot problen with next-20110211

[Dmitry Torokhov]

On Thu, Feb 10, 2011 at 10:57:31PM -0800, Dmitry Torokhov wrote:
> Hi Stephen,
> 
> On Thu, Feb 10, 2011 at 10:22:54PM -0800, Stephen Rothwell wrote:
> > 
> > Maybe the module parameter alignment patches?  That is a very strange
> > parameter name (it is the same in the other messages).
> 
> Yes, I think it is... I wonder why I did not see it locally.  Let me try
> to fix that...
> 

Gah, it is better Rusty drops the module param alignment patch so I can
redo it properly. The module case is completely broken as it still
operates with assumption that __param section contains full structures
and not pointers.

-- 
Dmitry

Re: linux-next: boot problen with next-20110211

[Dmitry Torokhov]

On Thu, Feb 10, 2011 at 11:06:42PM -0800, Dmitry Torokhov wrote:
> On Thu, Feb 10, 2011 at 10:57:31PM -0800, Dmitry Torokhov wrote:
> > Hi Stephen,
> > 
> > On Thu, Feb 10, 2011 at 10:22:54PM -0800, Stephen Rothwell wrote:
> > > 
> > > Maybe the module parameter alignment patches?  That is a very strange
> > > parameter name (it is the same in the other messages).
> > 
> > Yes, I think it is... I wonder why I did not see it locally.  Let me try
> > to fix that...
> > 
> 
> Gah, it is better Rusty drops the module param alignment patch so I can
> redo it properly. The module case is completely broken as it still
> operates with assumption that __param section contains full structures
> and not pointers.
> 

OK, so the following seems to boot fine on x86_64 and i386. Anyway,
tomorrow I will fold everything together and resend properly.

Thanks,
Dmitry


>From 24871cd7d3afc5e71c8ae22dfc6196bb1c605d60 Mon Sep 17 00:00:00 2001
From: Dmitry Torokhov <dtor@vmware.com>
Date: Fri, 11 Feb 2011 00:34:25 -0800
Subject: [PATCH] module: fix fallout from alignment patch

Signed-off-by: Dmitry Torokhov <dtor@vmware.com>
---
 include/linux/module.h      |    2 +-
 include/linux/moduleparam.h |   10 +++++-----
 init/main.c                 |    2 +-
 kernel/module.c             |    2 +-
 kernel/params.c             |   35 +++++++++++++++++++----------------
 5 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index cb41837..c256380 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -292,7 +292,7 @@ struct module
 	unsigned int num_syms;
 
 	/* Kernel parameters. */
-	struct kernel_param *kp;
+	const struct kernel_param **kp;
 	unsigned int num_kp;
 
 	/* GPL-only exported symbols. */
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 05b92c0..75c5dad 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -267,15 +267,15 @@ static inline void __kernel_param_unlock(void)
 /* Called on module insert or kernel boot */
 extern int parse_args(const char *name,
 		      char *args,
-		      const struct kernel_param *params,
+		      const struct kernel_param **params,
 		      unsigned num,
 		      int (*unknown)(char *param, char *val));
 
 /* Called by module remove. */
 #ifdef CONFIG_SYSFS
-extern void destroy_params(const struct kernel_param *params, unsigned num);
+extern void destroy_params(const struct kernel_param **params, unsigned num);
 #else
-static inline void destroy_params(const struct kernel_param *params,
+static inline void destroy_params(const struct kernel_param **params,
 				  unsigned num)
 {
 }
@@ -393,13 +393,13 @@ struct module;
 
 #if defined(CONFIG_SYSFS) && defined(CONFIG_MODULES)
 extern int module_param_sysfs_setup(struct module *mod,
-				    const struct kernel_param *kparam,
+				    const struct kernel_param **kparam,
 				    unsigned int num_params);
 
 extern void module_param_sysfs_remove(struct module *mod);
 #else
 static inline int module_param_sysfs_setup(struct module *mod,
-			     const struct kernel_param *kparam,
+			     const struct kernel_param **kparam,
 			     unsigned int num_params)
 {
 	return 0;
diff --git a/init/main.c b/init/main.c
index 33c37c3..3601586 100644
--- a/init/main.c
+++ b/init/main.c
@@ -544,7 +544,7 @@ static void __init mm_init(void)
 asmlinkage void __init start_kernel(void)
 {
 	char * command_line;
-	extern const struct kernel_param __start___param[], __stop___param[];
+	extern const struct kernel_param *__start___param[], *__stop___param[];
 
 	smp_setup_processor_id();
 
diff --git a/kernel/module.c b/kernel/module.c
index efa290e..7c6c3bf 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1470,7 +1470,7 @@ out:
 
 static int mod_sysfs_setup(struct module *mod,
 			   const struct load_info *info,
-			   struct kernel_param *kparam,
+			   const struct kernel_param **kparam,
 			   unsigned int num_params)
 {
 	int err;
diff --git a/kernel/params.c b/kernel/params.c
index 66f7e66..4f2eb43 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -83,9 +83,9 @@ static inline int parameq(const char *input, const char *paramname)
 	return 0;
 }
 
-static int parse_one(char *param,
+static int parse_one(char *name,
 		     char *val,
-		     const struct kernel_param *params,
+		     const struct kernel_param **params,
 		     unsigned num_params,
 		     int (*handle_unknown)(char *param, char *val))
 {
@@ -94,14 +94,16 @@ static int parse_one(char *param,
 
 	/* Find parameter */
 	for (i = 0; i < num_params; i++) {
-		if (parameq(param, params[i].name)) {
+		const struct kernel_param *param = params[i];
+
+		if (parameq(name, param->name)) {
 			/* Noone handled NULL, so do it here. */
-			if (!val && params[i].ops->set != param_set_bool)
+			if (!val && param->ops->set != param_set_bool)
 				return -EINVAL;
 			DEBUGP("They are equal!  Calling %p\n",
-			       params[i].ops->set);
+			       param->ops->set);
 			mutex_lock(&param_lock);
-			err = params[i].ops->set(val, &params[i]);
+			err = param->ops->set(val, param);
 			mutex_unlock(&param_lock);
 			return err;
 		}
@@ -109,7 +111,7 @@ static int parse_one(char *param,
 
 	if (handle_unknown) {
 		DEBUGP("Unknown argument: calling %p\n", handle_unknown);
-		return handle_unknown(param, val);
+		return handle_unknown(name, val);
 	}
 
 	DEBUGP("Unknown argument `%s'\n", param);
@@ -171,7 +173,7 @@ static char *next_arg(char *args, char **param, char **val)
 /* Args looks like "foo=bar,bar2 baz=fuz wiz". */
 int parse_args(const char *name,
 	       char *args,
-	       const struct kernel_param *params,
+	       const struct kernel_param **params,
 	       unsigned num,
 	       int (*unknown)(char *param, char *val))
 {
@@ -190,8 +192,9 @@ int parse_args(const char *name,
 		irq_was_disabled = irqs_disabled();
 		ret = parse_one(param, val, params, num, unknown);
 		if (irq_was_disabled && !irqs_disabled()) {
-			printk(KERN_WARNING "parse_args(): option '%s' enabled "
-					"irq's!\n", param);
+			printk(KERN_WARNING
+				"parse_args(): option '%s' enabled irq's!\n",
+				param);
 		}
 		switch (ret) {
 		case -ENOENT:
@@ -668,16 +671,16 @@ static void free_module_param_attrs(struct module_kobject *mk)
  * /sys/module/[mod->name]/parameters/
  */
 int module_param_sysfs_setup(struct module *mod,
-			     const struct kernel_param *kparam,
+			     const struct kernel_param **kparam,
 			     unsigned int num_params)
 {
 	int i, err;
 	bool params = false;
 
 	for (i = 0; i < num_params; i++) {
-		if (kparam[i].perm == 0)
+		if (kparam[i]->perm == 0)
 			continue;
-		err = add_sysfs_param(&mod->mkobj, &kparam[i], kparam[i].name);
+		err = add_sysfs_param(&mod->mkobj, kparam[i], kparam[i]->name);
 		if (err)
 			return err;
 		params = true;
@@ -711,13 +714,13 @@ void module_param_sysfs_remove(struct module *mod)
 }
 #endif
 
-void destroy_params(const struct kernel_param *params, unsigned num)
+void destroy_params(const struct kernel_param **params, unsigned num)
 {
 	unsigned int i;
 
 	for (i = 0; i < num; i++)
-		if (params[i].ops->free)
-			params[i].ops->free(params[i].arg);
+		if (params[i]->ops->free)
+			params[i]->ops->free(params[i]->arg);
 }
 
 static struct module_kobject * __init locate_module_kobject(const char *name)
-- 
1.7.3.2