2011-02-11 06:23:09

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: boot problen with next-20110211

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 [email protected]
http://www.canb.auug.org.au/~sfr/


Attachments:
(No filename) (2.26 kB)
(No filename) (490.00 B)
Download all attachments

2011-02-11 06:58:28

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: linux-next: boot problen with next-20110211

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

2011-02-11 07:07:39

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: linux-next: boot problen with next-20110211

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

2011-02-11 08:37:59

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: linux-next: boot problen with next-20110211

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 <[email protected]>
Date: Fri, 11 Feb 2011 00:34:25 -0800
Subject: [PATCH] module: fix fallout from alignment patch

Signed-off-by: Dmitry Torokhov <[email protected]>
---
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