export module parameters in sysfs for modules _and_ built-in code

RFC PATCH In review Dominik Brodowski 5 participants 9 messages Aug 1, 2004 – Aug 29, 2004 Module support

Share & tools

Dominik Brodowski

author

2004-08-01 16:58 UTC #0 lore raw

RFC PATCH 1/2 export module parameters in sysfs for modules _and_ built-in code

Create a new /sys top-level directory named "parameters", and make all to-be-sysfs-exported module parameters available as attributes to kobjects. Currently, only module parameters in _modules_ are exported in /sys/modules/, while those of "modules" built into the kernel can be set by the kernel command line, but not read or set via sysfs.

For modules, a symlink

brodo@mondschein param $ ls -l /sys/module/ehci_hcd/ | grep param
lrwxrwxrwx 1 brodo brodo 0 1. Aug 17:50 parameters -> ../../parameters/ehci_hcd

is added. Removal of double module parameters export for modules is sent in a second patch, so the diffstat

 include/linux/module.h |    2
 kernel/module.c        |   14 +
 kernel/params.c        |  377 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 393 insertions(+)

looks worse than it is.

Much of this work is based on the current code for modules only to be found in linux/kernel/module.c

Signed-off-by Dominik Brodowski <[email protected]>

include/linux/module.h +2 -0

diff -ruN linux-original/include/linux/module.h linux/include/linux/module.h
@@ -240,6 +240,7 @@
 	struct module_sect_attr attrs[0];
 };
 
+struct param_kobject;
 
 struct module
 {

@@ -253,6 +254,7 @@
 
 	/* Sysfs stuff. */
 	struct module_kobject *mkobj;
+	struct param_kobject *params_kobject;
 
 	/* Exported symbols */
 	const struct kernel_symbol *syms;

kernel/module.c +14 -0

diff -ruN linux-original/kernel/module.c linux/kernel/module.c
@@ -1131,6 +1131,12 @@
 };
 static decl_subsys(module, &module_ktype, NULL);
 
+extern int module_param_sysfs_setup(struct module *mod, 
+				    struct kernel_param *kparam,
+				    unsigned int num_params);
+
+extern void module_param_sysfs_remove(struct module *mod);
+
 static int mod_sysfs_setup(struct module *mod,
 			   struct kernel_param *kparam,
 			   unsigned int num_params)

@@ -1166,6 +1172,11 @@
 	err = sysfs_unload_setup(mod);
 	if (err)
 		goto out_unreg;
+
+	err = module_param_sysfs_setup(mod, kparam, num_params);
+	if (err)
+		goto out_unreg;
+
 	return 0;
 
 out_unreg:

@@ -1182,6 +1193,9 @@
 static void mod_kobject_remove(struct module *mod)
 {
 	unsigned int i;
+
+	module_param_sysfs_remove(mod);
+
 	for (i = 0; i < mod->mkobj->num_attributes; i++)
 		sysfs_remove_file(&mod->mkobj->kobj,&mod->mkobj->attr[i].attr);
 	/* Calls module_kobj_release */

kernel/params.c +377 -0

389 lines not on this page · raw message

Rusty Russell

2004-08-02 06:00 UTC #1 lore raw

Quoted 5 lines · Dominik Brodowski

Create a new /sys top-level directory named "parameters", and make all to-be-sysfs-exported module parameters available as attributes to kobjects. Currently, only module parameters in _modules_ are exported in /sys/modules/, while those of "modules" built into the kernel can be set by the kernel command line, but not read or set via sysfs.

Thanks for this Dominik!

One question from reading the code:

kernel/module.c line 1138

Quoted 12 lines

diff -ruN linux-original/kernel/module.c linux/kernel/module.c

kernel/module.c +5 -0

@@ -1131,6 +1131,12 @@
 };
 static decl_subsys(module, &module_ktype, NULL);
 
+extern int module_param_sysfs_setup(struct module *mod, 
+				    struct kernel_param *kparam,
+				    unsigned int num_params);
+
+extern void module_param_sysfs_remove(struct module *mod);

Put these in moduleparam.h please, otherwise AKPM will kill us both.

Quoted 4 lines

kernel/module.c +4 -0

+	kbuild_modname = kmalloc(sizeof(char) * (MAX_KBUILD_MODNAME + 1), GFP_KERNEL);
+	if (!kbuild_modname)
+		return -ENOMEM;
+	memset(kbuild_modname, 0, sizeof(char) * (MAX_KBUILD_MODNAME + 1));

...

kernel/module.c

Quoted 1 line

kernel/module.c +1 -0
+	kfree (kbuild_modname);

I would have thought this a good candidate for a stack variable?

kernel/module.c

Quoted 2 lines

kernel/module.c +2 -0

+/* Needs to be before __initcall(module_init) */
+fs_initcall(param_sysfs_init);

That's horrible. And I think the initcall in module.c should be removed in your second patch, no?

Rusty,

Signature · 1 line

Anyone who quotes me in their signature is an idiot -- Rusty Russell

Dominik Brodowski

author

2004-08-02 21:49 UTC #2 lore raw

PATCH 1/2 export module parameters in sysfs for modules _and_ built-in code

Thanks for your feedback, Rusty.

Quoted 7 lines · Rusty Russell

Review
Quoted 5 lines
(quoted diff) +5 -0
+extern int module_param_sysfs_setup(struct module *mod, 
+				    struct kernel_param *kparam,
+				    unsigned int num_params);
+
+extern void module_param_sysfs_remove(struct module *mod);
Put these in moduleparam.h please, otherwise AKPM will kill us both.

Done.

Quoted 10 lines

Quoted 4 lines
(quoted diff) +4 -0
+	kbuild_modname = kmalloc(sizeof(char) * (MAX_KBUILD_MODNAME + 1), GFP_KERNEL);
+	if (!kbuild_modname)
+		return -ENOMEM;
+	memset(kbuild_modname, 0, sizeof(char) * (MAX_KBUILD_MODNAME + 1));
...
Review
Quoted 1 line
(quoted diff) +1 -0
+	kfree (kbuild_modname);
I would have thought this a good candidate for a stack variable?

Done. Was overzealous with stack space...

Quoted 5 lines

Review
Quoted 2 lines
(quoted diff) +2 -0
+/* Needs to be before __initcall(module_init) */
+fs_initcall(param_sysfs_init);
That's horrible. And I think the initcall in module.c should be removed in your second patch, no?

Actually, it could have remained __initcall... and the one in module.c is needed as we still register a module subsystem. This updated patch doesn't add an own initcall in params.c, though, but rather param_sysfs_init() gets called by module_init.

For modules, a symlink

brodo@mondschein param $ ls -l /sys/module/ehci_hcd/ | grep param
lrwxrwxrwx 1 brodo brodo 0 1. Aug 17:50 parameters -> ../../parameters/ehci_hcd

is added. Removal of double module parameters export for modules is sent in a second patch, so the diffstat

 include/linux/module.h      |    2
 include/linux/moduleparam.h |   14 +
 kernel/module.c             |   16 +
 kernel/params.c             |  368 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 399 insertions(+), 1 deletion(-)

looks worse than it is.

Much of this work is based on the current code for modules only to be found in linux/kernel/module.c; many thanks to Rusty Russell for his code and his feedback!

Signed-off-by Dominik Brodowski <[email protected]>

include/linux/module.h +2 -0

diff -ruN linux-original/include/linux/module.h linux/include/linux/module.h
@@ -240,6 +240,7 @@
 	struct module_sect_attr attrs[0];
 };
 
+struct param_kobject;
 
 struct module
 {

@@ -253,6 +254,7 @@
 
 	/* Sysfs stuff. */
 	struct module_kobject *mkobj;
+	struct param_kobject *params_kobject;
 
 	/* Exported symbols */
 	const struct kernel_symbol *syms;

include/linux/moduleparam.h +14 -0

diff -ruN linux-original/include/linux/moduleparam.h linux/include/linux/moduleparam.h
@@ -147,4 +147,18 @@
 		void *elem, int elemsize,
 		int (*set)(const char *, struct kernel_param *kp),
 		int *num);
+
+
+/* for exporting parameters in /sys/parameters */
+
+struct module;
+
+extern int module_param_sysfs_setup(struct module *mod, 
+				    struct kernel_param *kparam,
+				    unsigned int num_params);
+
+extern void module_param_sysfs_remove(struct module *mod);
+
+extern int __init param_sysfs_init(void);
+
 #endif /* _LINUX_MODULE_PARAMS_H */

kernel/module.c +15 -1

diff -ruN linux-original/kernel/module.c linux/kernel/module.c
@@ -1166,6 +1166,11 @@
 	err = sysfs_unload_setup(mod);
 	if (err)
 		goto out_unreg;
+
+	err = module_param_sysfs_setup(mod, kparam, num_params);
+	if (err)
+		goto out_unreg;
+
 	return 0;
 
 out_unreg:

@@ -1182,6 +1187,9 @@
 static void mod_kobject_remove(struct module *mod)
 {
 	unsigned int i;
+
+	module_param_sysfs_remove(mod);
+
 	for (i = 0; i < mod->mkobj->num_attributes; i++)
 		sysfs_remove_file(&mod->mkobj->kobj,&mod->mkobj->attr[i].attr);
 	/* Calls module_kobj_release */

@@ -2170,6 +2178,12 @@
 
 static int __init modules_init(void)
 {
-	return subsystem_register(&module_subsys);
+	int ret;
+
+	ret = subsystem_register(&module_subsys);
+	if (ret)
+		return (ret);
+
+	return param_sysfs_init();
 }
 __initcall(modules_init);

kernel/params.c +368 -0

380 lines not on this page · raw message

Rusty Russell

2004-08-03 00:32 UTC #3 lore raw

Quoted 1 line · Dominik Brodowski

Thanks for your feedback, Rusty.

Hey, thanks for the code!

Quoted 10 lines

Quoted 5 lines
Review
Quoted 2 lines
(quoted diff) +2 -0
+/* Needs to be before __initcall(module_init) */
+fs_initcall(param_sysfs_init);
That's horrible. And I think the initcall in module.c should be removed in your second patch, no?
Actually, it could have remained __initcall... and the one in module.c is needed as we still register a module subsystem. This updated patch doesn't add an own initcall in params.c, though, but rather param_sysfs_init() gets called by module_init.

It's logically separate, IMHO. I'm thinking about CONFIG_MODULES=n here...

Cheers,
Rusty.

Signature · 1 line

Anyone who quotes me in their signature is an idiot -- Rusty Russell

Patrick Mansfield

2004-08-13 17:11 UTC #4 lore raw

In reply to Dominik Brodowski

I finally tried this out, nice.

Is someone pushing it? I don't see it in 2.6.8-rc4-mm1.

-- Patrick Mansfield

Quoted 27 lines · Dominik Brodowski

Create a new /sys top-level directory named "parameters", and make all to-be-sysfs-exported module parameters available as attributes to kobjects. Currently, only module parameters in _modules_ are exported in /sys/modules/, while those of "modules" built into the kernel can be set by the kernel command line, but not read or set via sysfs.
For modules, a symlink
brodo@mondschein param $ ls -l /sys/module/ehci_hcd/ | grep param
lrwxrwxrwx 1 brodo brodo 0 1. Aug 17:50 parameters -> ../../parameters/ehci_hcd
is added. Removal of double module parameters export for modules is sent in a second patch, so the diffstat
 include/linux/module.h      |    2
 include/linux/moduleparam.h |   14 +
 kernel/module.c             |   16 +
 kernel/params.c             |  368 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 399 insertions(+), 1 deletion(-)
looks worse than it is.
Much of this work is based on the current code for modules only to be found in linux/kernel/module.c; many thanks to Rusty Russell for his code and his feedback!
Signed-off-by Dominik Brodowski <[email protected]>

Martin Schlemmer

2004-08-18 19:52 UTC #5 lore raw

In reply to Dominik Brodowski

On Mon, 2004-08-02 at 23:47, Dominik Brodowski wrote:

I know its tainted (nvidia), but this is difficult to test, as it usually only happens if the box have been up for a while and I modprobe something (ext2 in most of the cases).

---

Unable to handle kernel paging request at virtual address 39cc6b10
 printing eip:
c03d16c4
*pde = 00000000
Oops: 0000 [#1]
PREEMPT SMP
Modules linked in: e1000 snd_intel8x0 snd_ac97_codec gameport snd_mpu401_uart snd_rawmidi snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_pcm snd_timer snd_page_alloc snd_mixer_oss snd joydev nvidia usbhid uhci_hcd ehci_hcd usbcore w83781d i2c_sensor i2c_isa i2c_i801 i2c_dev i2c_core
CPU:    1
EIP:    0060:[<c03d16c4>]    Tainted: P
EFLAGS: 00010286   (2.6.8.1)

EIP is at param_sysfs_setup+0x0/0x129
eax: f9cb338c ebx: 00000000 ecx: f7db4ec0 edx: c0333820
esi: f9cb3380 edi: f9cb33cc ebp: f9b48000 esp: c9709ebc
ds: 007b es: 007b ss: 0068
Process modprobe (pid: 17632, threadinfo=c9708000 task=e15a4670)

Stack: c01312f4 f9cb338c f9cb3380 00000000 00000000 00000000 f9cb33cc 00000000
       f9cb3380 00000000 c013644a f9cb3380 00000000 00000000 f9c90000 f9c9d0f4
       f9cb3380 c0137644 f9cb3380 00000000 00000000 00000000 00000000 f9cb3380
Call Trace:
 [<c01312f4>] module_param_sysfs_setup+0x41/0x95
 [<c013644a>] mod_sysfs_setup+0x89/0xb4
 [<c0137644>] load_module+0x916/0xbd9
 [<c013798a>] sys_init_module+0x83/0x25e
 [<c0104195>] sysenter_past_esp+0x52/0x71
Code: 65 6e 40 6e 6f 72 74 65 6c 6e 65 74 77 6f 72 6b 73 2e 63 6f

---

PS: If needed, I can try to get some time to leave it up not

    in X ...

Thanks,

Signature · 1 line

Martin Schlemmer

Tonnerre

2004-08-20 13:47 UTC #6 lore raw

Salut,

Quoted 2 lines · Martin Schlemmer

PS: If needed, I can try to get some time to leave it up not
    in X ...

Try this in xorg.conf:

Section "Device"

	Identifier	"Generic VESA interface"
	Driver		"vesa"

EndSection

So you can have both X and a debuggable kernel.

Tonnerre

Dominik Brodowski

author

2004-08-29 13:32 UTC #7 lore raw

In reply to Martin Schlemmer

Quoted 5 lines · Martin Schlemmer

On Mon, 2004-08-02 at 23:47, Dominik Brodowski wrote:
I know its tainted (nvidia), but this is difficult to test, as it usually only happens if the box have been up for a while and I modprobe something (ext2 in most of the cases).

Sorry for the delay, was on vacations... Which variant of the patch were you using? Did it already shuffle the section of kernel_param around? If not, that's the cause -- and that patch has reached Linus' tree by now.

Thanks for testing,
Dominik

Martin Schlemmer [c]

2004-08-29 13:46 UTC #8 lore raw

Re: [UPDATED PATCH 1/2] export module parameters in sysfs for modules _and_ built-in code [u]

Quoted 11 lines · Dominik Brodowski

Quoted 5 lines · Martin Schlemmer
On Mon, 2004-08-02 at 23:47, Dominik Brodowski wrote:
I know its tainted (nvidia), but this is difficult to test, as it usually only happens if the box have been up for a while and I modprobe something (ext2 in most of the cases).
Sorry for the delay, was on vacations... Which variant of the patch were you using? Did it already shuffle the section of kernel_param around? If not, that's the cause -- and that patch has reached Linus' tree by now.

Hmm, I cant remember if it was you second last, or last version. I dropped it for the time being, and have not yet checked through my patches for 2.6.9-rc1 - hopefully will get time next week, and if the problem persist, I will let you know (should be 2.6.9-rc1-bk_something_, so I assume your last stuff would have gone in already if it went to Linus ...).

Thanks,

Signature · 1 line

Martin Schlemmer