Skip to contentSkip to conversation

module: sysfs - add 'uevent' file to allow coldplug

PATCH Kay Sievers 3 participants 16 messages Jun 18, 2011 – Jul 7, 2011 Module support

lore
Share & tools
lore

Kay Sievers

author
#0 lore raw

From: Kay Sievers <[email protected]>
Subject: module: sysfs - add 'uevent' file to allow built-in coldplug

Userspace wants to manage module parameters with udev rules. This currently only works for loaded modules, but not for built-in ones.

To allow access to the built-in modules we need to re-trigger all module load events that happened before any userspace was running. We already do the same thing for all devices, subsystems(buses) and drivers.

This adds the currently missing /sys/module/<name>/uevent files to all module entries.

Signed-off-by Kay Sievers <[email protected]>

---

 include/linux/module.h |   24 +++++++++++++-----------
 kernel/module.c        |   31 ++++++++++++++++++++++++-------
 kernel/params.c        |   12 +++++++-----
 3 files changed, 44 insertions(+), 23 deletions(-)
include/linux/module.h +13 -11
index d9ca2d5..ac6975c 100644
@@ -48,10 +48,18 @@ struct modversion_info
 
 struct module;
 
+struct module_kobject
+{
+	struct kobject kobj;
+	struct module *mod;
+	struct kobject *drivers_dir;
+	struct module_param_attrs *mp;
+};
+
 struct module_attribute {
-        struct attribute attr;
-        ssize_t (*show)(struct module_attribute *, struct module *, char *);
-        ssize_t (*store)(struct module_attribute *, struct module *,
+	struct attribute attr;
+	ssize_t (*show)(struct module_attribute *, struct module_kobject *, char *);
+	ssize_t (*store)(struct module_attribute *, struct module_kobject *,
 			 const char *, size_t count);
 	void (*setup)(struct module *, const char *);
 	int (*test)(struct module *);

@@ -65,15 +73,9 @@ struct module_version_attribute {
 } __attribute__ ((__aligned__(sizeof(void *))));
 
 extern ssize_t __modver_version_show(struct module_attribute *,
-				     struct module *, char *);
+				     struct module_kobject *, char *);
 
-struct module_kobject
-{
-	struct kobject kobj;
-	struct module *mod;
-	struct kobject *drivers_dir;
-	struct module_param_attrs *mp;
-};
+extern struct module_attribute module_uevent;
 
 /* These are either module local, or the kernel's dummy ones. */
 extern int init_module(void);
kernel/module.c +24 -7
index 795bdc7..2f0d562 100644
@@ -545,9 +545,9 @@ static void setup_modinfo_##field(struct module *mod, const char *s)  \
 	mod->field = kstrdup(s, GFP_KERNEL);                          \
 }                                                                     \
 static ssize_t show_modinfo_##field(struct module_attribute *mattr,   \
-	                struct module *mod, char *buffer)             \
+	                struct module_kobject *mk, char *buffer)      \
 {                                                                     \
-	return sprintf(buffer, "%s\n", mod->field);                   \
+	return sprintf(buffer, "%s\n", mk->mod->field);               \
 }                                                                     \
 static int modinfo_##field##_exists(struct module *mod)               \
 {                                                                     \

@@ -902,9 +902,9 @@ void symbol_put_addr(void *addr)
 EXPORT_SYMBOL_GPL(symbol_put_addr);
 
 static ssize_t show_refcnt(struct module_attribute *mattr,
-			   struct module *mod, char *buffer)
+			   struct module_kobject *mk, char *buffer)
 {
-	return sprintf(buffer, "%u\n", module_refcount(mod));
+	return sprintf(buffer, "%u\n", module_refcount(mk->mod));
 }
 
 static struct module_attribute refcnt = {

@@ -952,11 +952,11 @@ static inline int module_unload_init(struct module *mod)
 #endif /* CONFIG_MODULE_UNLOAD */
 
 static ssize_t show_initstate(struct module_attribute *mattr,
-			   struct module *mod, char *buffer)
+			      struct module_kobject *mk, char *buffer)
 {
 	const char *state = "unknown";
 
-	switch (mod->state) {
+	switch (mk->mod->state) {
 	case MODULE_STATE_LIVE:
 		state = "live";
 		break;

@@ -975,10 +975,27 @@ static struct module_attribute initstate = {
 	.show = show_initstate,
 };
 
+static ssize_t store_uevent(struct module_attribute *mattr,
+			    struct module_kobject *mk,
+			    const char *buffer, size_t count)
+{
+	enum kobject_action action;
+
+	if (kobject_action_type(buffer, count, &action) == 0)
+		kobject_uevent(&mk->kobj, action);
+	return count;
+}
+
+struct module_attribute module_uevent = {
+	.attr = { .name = "uevent", .mode = 0200 },
+	.store = store_uevent,
+};
+
 static struct module_attribute *modinfo_attrs[] = {
 	&modinfo_version,
 	&modinfo_srcversion,
 	&initstate,
+	&module_uevent,
 #ifdef CONFIG_MODULE_UNLOAD
 	&refcnt,
 #endif

@@ -1187,7 +1204,7 @@ struct module_sect_attrs
 };
 
 static ssize_t module_sect_show(struct module_attribute *mattr,
-				struct module *mod, char *buf)
+				struct module_kobject *mk, char *buf)
 {
 	struct module_sect_attr *sattr =
 		container_of(mattr, struct module_sect_attr, mattr);
kernel/params.c +7 -5
index ed72e13..d2f02ca 100644
@@ -511,7 +511,7 @@ struct module_param_attrs
 #define to_param_attr(n) container_of(n, struct param_attribute, mattr)
 
 static ssize_t param_attr_show(struct module_attribute *mattr,
-			       struct module *mod, char *buf)
+			       struct module_kobject *mk, char *buf)
 {
 	int count;
 	struct param_attribute *attribute = to_param_attr(mattr);

@@ -531,7 +531,7 @@ static ssize_t param_attr_show(struct module_attribute *mattr,
 
 /* sysfs always hands a nul-terminated string in buf.  We rely on that. */
 static ssize_t param_attr_store(struct module_attribute *mattr,
-				struct module *owner,
+				struct module_kobject *km,
 				const char *buf, size_t len)
 {
  	int err;

@@ -730,6 +730,8 @@ static struct module_kobject * __init locate_module_kobject(const char *name)
 		mk->kobj.kset = module_kset;
 		err = kobject_init_and_add(&mk->kobj, &module_ktype, NULL,
 					   "%s", name);
+		if (!err)
+			err = sysfs_create_file(&mk->kobj, &module_uevent.attr);
 		if (err) {
 			kobject_put(&mk->kobj);
 			printk(KERN_ERR

@@ -807,7 +809,7 @@ static void __init param_sysfs_builtin(void)
 }
 
 ssize_t __modver_version_show(struct module_attribute *mattr,
-			      struct module *mod, char *buf)
+			      struct module_kobject *mk, char *buf)
 {
 	struct module_version_attribute *vattr =
 		container_of(mattr, struct module_version_attribute, mattr);

@@ -852,7 +854,7 @@ static ssize_t module_attr_show(struct kobject *kobj,
 	if (!attribute->show)
 		return -EIO;
 
-	ret = attribute->show(attribute, mk->mod, buf);
+	ret = attribute->show(attribute, mk, buf);
 
 	return ret;
 }

@@ -871,7 +873,7 @@ static ssize_t module_attr_store(struct kobject *kobj,
 	if (!attribute->store)
 		return -EIO;
 
-	ret = attribute->store(attribute, mk->mod, buf, len);
+	ret = attribute->store(attribute, mk, buf, len);
 
 	return ret;
 }
#1 lore raw
Quoted 6 lines · Kay Sievers

From: Kay Sievers <[email protected]>
Subject: module: sysfs - add 'uevent' file to allow built-in coldplug

Userspace wants to manage module parameters with udev rules. This currently only works for loaded modules, but not for built-in ones.

I'm confused. What does "manage" mean here?

Thanks,
Rusty.

author
#2 lore raw
Quoted 9 lines · Rusty Russell
Quoted 6 lines · Kay Sievers

From: Kay Sievers <[email protected]>
Subject: module: sysfs - add 'uevent' file to allow built-in coldplug

Userspace wants to manage module parameters with udev rules. This currently only works for loaded modules, but not for built-in ones.

I'm confused.  What does "manage" mean here?

Hook system management into module-load events, which might include changing module parameters in /sys/module/*/parameters/*, or at bootup/coldplug run it for built-in modules.

We do the same to set properties for buses, drivers or devices when they appear.

Kay

#3 lore raw
Quoted 9 lines · Kay Sievers
Quoted 8 lines · Rusty Russell
Quoted 6 lines · Kay Sievers

From: Kay Sievers <[email protected]>
Subject: module: sysfs - add 'uevent' file to allow built-in coldplug

Userspace wants to manage module parameters with udev rules. This currently only works for loaded modules, but not for built-in ones.

Quoted 7 lines
Quoted 1 line

I'm confused.  What does "manage" mean here?

Hook system management into module-load events, which might include changing module parameters in /sys/module/*/parameters/*, or at bootup/coldplug run it for built-in modules.

We do the same to set properties for buses, drivers or devices when they appear.

Sorry, that's another vague answer :(

udev already knows about module load, and the parameters are created at that point; they are not *changed*.

What, *exactly* will this enable? Don't use the word "hook" or "management": both vague terms which assume I know what you're talking about. I don't.

Show me exactly what udev will now be able to do that it can't do now, and how it will be used. Preferably with examples. Assume (correctly) that I have only a vague idea what udev does.

Thanks,
Rusty.

author
#4 lore raw
Quoted 16 lines · Rusty Russell
Quoted 15 lines · Kay Sievers
Quoted 9 lines · Rusty Russell
Quoted 6 lines · Kay Sievers

From: Kay Sievers <[email protected]>
Subject: module: sysfs - add 'uevent' file to allow built-in coldplug

Userspace wants to manage module parameters with udev rules. This currently only works for loaded modules, but not for built-in ones.

I'm confused.  What does "manage" mean here?

Hook system management into module-load events, which might include changing module parameters in /sys/module/*/parameters/*, or at bootup/coldplug run it for built-in modules.

Quoted 5 lines
Quoted 1 line

We do the same to set properties for buses, drivers or devices when they appear.

Sorry, that's another vague answer :(

udev already knows about module load

Not for built-ins.

Quoted 2 lines

and the parameters are created at
that point; they are not *changed*.

The can be changed, and sometimes they need.

Quoted 3 lines

What, *exactly* will this enable?  Don't use the word "hook" or "management": both vague terms which assume I know what you're talking about.  I don't.

Just used these words because it's absolutely generic infrastructure we use already everywhere in /sys. :) Just run a: find /sys -name uevent

Quoted 3 lines

Show me exactly what udev will now be able to do that it can't do now, and how it will be used.  Preferably with examples.  Assume (correctly) that I have only a vague idea what udev does.

Set the polling interval of a always-built-in module parameter:

SUBSYSTEM=="module", KERNEL=="block",
ATTR{parameters/events_dfl_poll_msecs}="2000"

http://git.kernel.org/?p=linux/hotplug/udev.git;a=commitdiff;h=c5a41da94916815ac14d950a0547bffc4411f7af

Kay

#5 lore raw
Quoted 6 lines · Kay Sievers
Quoted 3 lines · Rusty Russell

Sorry, that's another vague answer :(

udev already knows about module load

Not for built-ins.

OK, I re-read your commit message, and down the bottom it does say what it *does*:

Quoted 2 lines

This adds the currently missing /sys/module/<name>/uevent files to all module entries.

I apologize for skimming, but this should be the *title* of the patch!

Then I saw your patch hit params.c and thought you were adding a uevent file to /sys/module/<name>/parameters/. I was even more confused when you replied:

Quoted 2 lines

Hook system management into module-load events, which might include changing module parameters in /sys/module/*/parameters/*...

Because loading a module might *create* module parameters, but it won't *change* them. If we want to have events for change, we need much more...

Now we've got that sorted, is there a reason why you changed all the signatures rather than just using mod->mkobj in store_uevent()?

Thanks,
Rusty.

author
#6 lore raw
Quoted 8 lines · Rusty Russell
Quoted 6 lines · Kay Sievers
Quoted 3 lines · Rusty Russell

Sorry, that's another vague answer :(

udev already knows about module load

Not for built-ins.

Quoted 7 lines

OK, I re-read your commit message, and down the bottom it does say what it *does*:

Quoted 2 lines

       This adds the currently missing /sys/module/<name>/uevent files to all module entries.

I apologize for skimming,

Absolutely no problem. Asking such questions can not be wrong.

Quoted 1 line

but this should be the *title* of the patch!

Right, it's hard sometimes from 'inside' to make good titles that make titles that are properly understood 'outside'. If you have any better idea, please just change it.

Quoted 7 lines

Then I saw your patch hit params.c and thought you were adding a uevent file to /sys/module/<name>/parameters/.  I was even more confused when you replied:

Quoted 2 lines

Hook system management into module-load events, which might include changing module parameters in /sys/module/*/parameters/*...

Quoted 6 lines

Because loading a module might *create* module parameters, but it won't *change* them.  If we want to have events for change, we need much more...

Now we've got that sorted, is there a reason why you changed all the signatures rather than just using mod->mkobj in store_uevent()?

Because we should be able to use the same 'struct module_attribute' for built-in modules and for loaded modules at the same time. The current 'struct module_attribute' has 'struct module' references, but 'struct module' will never exist for built-in modules.

'Struct module_kobject' has nice back-pointer to 'struct module', so this was the simplest to do, and looks still fine, I thought.

Kay

#7 lore raw
Quoted 4 lines · Kay Sievers
Quoted 1 line · Rusty Russell

I apologize for skimming,

Absolutely no problem. Asking such questions can not be wrong.

Sure, but as we know nothing is as abrupt as a hacker who has just encountered their own ignorance :)

Quoted 5 lines
Quoted 1 line

but this should be the *title* of the patch!

Right, it's hard sometimes from 'inside' to make good titles that make titles that are properly understood 'outside'. If you have any better idea, please just change it.

Good observation; for me writing documentation serves this purpose. I renamed half the functions in the iptables code after I'd written the user docs...

Quoted 10 lines
Quoted 2 lines

Now we've got that sorted, is there a reason why you changed all the signatures rather than just using mod->mkobj in store_uevent()?

Because we should be able to use the same 'struct module_attribute' for built-in modules and for loaded modules at the same time. The current 'struct module_attribute' has 'struct module' references, but 'struct module' will never exist for built-in modules.

'Struct module_kobject' has nice back-pointer to 'struct module', so this was the simplest to do, and looks still fine, I thought.

Yes, it's weird. The only reason it currently works is because we don't use the mod parameter in param_attr_show and param_attr_store; it's NULL for built-in modules.

I'd prefer that patch first, I think: it's a sensible cleanup.

Thanks,
Rusty.

author
#8 lore raw
Quoted 11 lines · Rusty Russell
Quoted 10 lines · Kay Sievers
Quoted 2 lines · Rusty Russell

Now we've got that sorted, is there a reason why you changed all the signatures rather than just using mod->mkobj in store_uevent()?

Because we should be able to use the same 'struct module_attribute' for built-in modules and for loaded modules at the same time. The current 'struct module_attribute' has 'struct module' references, but 'struct module' will never exist for built-in modules.

Quoted 8 lines
Quoted 2 lines

'Struct module_kobject' has nice back-pointer to 'struct module', so this was the simplest to do, and looks still fine, I thought.

Yes, it's weird.  The only reason it currently works is because we don't use the mod parameter in param_attr_show and param_attr_store; it's NULL for built-in modules.

I'd prefer that patch first, I think: it's a sensible cleanup.

You want the patch split up in two? You want to remove the mod parameter somehow?

Thanks,
Kay

author
#9 lore raw
Quoted 15 lines · Kay Sievers
Quoted 14 lines · Rusty Russell
Quoted 12 lines · Kay Sievers
Quoted 2 lines · Rusty Russell

Now we've got that sorted, is there a reason why you changed all the signatures rather than just using mod->mkobj in store_uevent()?

Because we should be able to use the same 'struct module_attribute' for built-in modules and for loaded modules at the same time. The current 'struct module_attribute' has 'struct module' references, but 'struct module' will never exist for built-in modules.

'Struct module_kobject' has nice back-pointer to 'struct module', so this was the simplest to do, and looks still fine, I thought.

Quoted 8 lines
Quoted 5 lines

Yes, it's weird.  The only reason it currently works is because we don't use the mod parameter in param_attr_show and param_attr_store; it's NULL for built-in modules.

I'd prefer that patch first, I think: it's a sensible cleanup.

You want the patch split up in two? You want to remove the mod parameter somehow?

Can we get these 20 lines of code sorted out please? :)

Kay

#10 lore raw
Quoted 10 lines · Kay Sievers
Quoted 7 lines · Kay Sievers
Quoted 3 lines · Rusty Russell
Quoted 1 line · Kay Sievers

On Wed, Jun 22, 2011 at 04:00, Rusty Russell <[email protected]> wrote:

I'd prefer that patch first, I think: it's a sensible cleanup.

You want the patch split up in two? You want to remove the mod parameter somehow?

Can we get these 20 lines of code sorted out please? :)

An odd question, since I was waiting for you to do exactly that!

Rather than go around again, I have:
1) Split the patch into one which changes the attr functions, and one which adds the uevent file.
2) Fixed the title of the (second) patch to "module: add /sys/module/<name>/uevent files"
3) Fixed up the three checkpatch.pl errors (sure, 2 were just code moves, but we're slowly neatening things).

Here they are below, back-to-back.

Thanks,
Rusty.

Subject: module: change attr callbacks to take struct module_kobject

This simplifies the next patch, where we have an attribute on a builtin module (ie. module == NULL).

Signed-off-by Kay Sievers <[email protected]>
Signed-off-by Rusty Russell <[email protected]> (split into 2)
---
 include/linux/module.h |   23 ++++++++++++-----------
 kernel/module.c        |   14 +++++++-------
 kernel/params.c        |   10 +++++-----
 3 files changed, 24 insertions(+), 23 deletions(-)
include/linux/module.h +12 -11
@@ -48,10 +48,18 @@ struct modversion_info
 
 struct module;
 
+struct module_kobject {
+	struct kobject kobj;
+	struct module *mod;
+	struct kobject *drivers_dir;
+	struct module_param_attrs *mp;
+};
+
 struct module_attribute {
-        struct attribute attr;
-        ssize_t (*show)(struct module_attribute *, struct module *, char *);
-        ssize_t (*store)(struct module_attribute *, struct module *,
+	struct attribute attr;
+	ssize_t (*show)(struct module_attribute *, struct module_kobject *,
+			char *);
+	ssize_t (*store)(struct module_attribute *, struct module_kobject *,
 			 const char *, size_t count);
 	void (*setup)(struct module *, const char *);
 	int (*test)(struct module *);

@@ -65,15 +72,8 @@ struct module_version_attribute {
 } __attribute__ ((__aligned__(sizeof(void *))));
 
 extern ssize_t __modver_version_show(struct module_attribute *,
-				     struct module *, char *);
+				     struct module_kobject *, char *);
 
-struct module_kobject
-{
-	struct kobject kobj;
-	struct module *mod;
-	struct kobject *drivers_dir;
-	struct module_param_attrs *mp;
-};
 
 /* These are either module local, or the kernel's dummy ones. */
 extern int init_module(void);
kernel/module.c +7 -7
@@ -545,9 +545,9 @@ static void setup_modinfo_##field(struct
 	mod->field = kstrdup(s, GFP_KERNEL);                          \
 }                                                                     \
 static ssize_t show_modinfo_##field(struct module_attribute *mattr,   \
-	                struct module *mod, char *buffer)             \
+			struct module_kobject *mk, char *buffer)      \
 {                                                                     \
-	return sprintf(buffer, "%s\n", mod->field);                   \
+	return sprintf(buffer, "%s\n", mk->mod->field);               \
 }                                                                     \
 static int modinfo_##field##_exists(struct module *mod)               \
 {                                                                     \

@@ -902,9 +902,9 @@ void symbol_put_addr(void *addr)
 EXPORT_SYMBOL_GPL(symbol_put_addr);
 
 static ssize_t show_refcnt(struct module_attribute *mattr,
-			   struct module *mod, char *buffer)
+			   struct module_kobject *mk, char *buffer)
 {
-	return sprintf(buffer, "%u\n", module_refcount(mod));
+	return sprintf(buffer, "%u\n", module_refcount(mk->mod));
 }
 
 static struct module_attribute refcnt = {

@@ -952,11 +952,11 @@ static inline int module_unload_init(str
 #endif /* CONFIG_MODULE_UNLOAD */
 
 static ssize_t show_initstate(struct module_attribute *mattr,
-			   struct module *mod, char *buffer)
+			      struct module_kobject *mk, char *buffer)
 {
 	const char *state = "unknown";
 
-	switch (mod->state) {
+	switch (mk->mod->state) {
 	case MODULE_STATE_LIVE:
 		state = "live";
 		break;

@@ -1187,7 +1187,7 @@ struct module_sect_attrs
 };
 
 static ssize_t module_sect_show(struct module_attribute *mattr,
-				struct module *mod, char *buf)
+				struct module_kobject *mk, char *buf)
 {
 	struct module_sect_attr *sattr =
 		container_of(mattr, struct module_sect_attr, mattr);
kernel/params.c +5 -5
@@ -511,7 +511,7 @@ struct module_param_attrs
 #define to_param_attr(n) container_of(n, struct param_attribute, mattr)
 
 static ssize_t param_attr_show(struct module_attribute *mattr,
-			       struct module *mod, char *buf)
+			       struct module_kobject *mk, char *buf)
 {
 	int count;
 	struct param_attribute *attribute = to_param_attr(mattr);

@@ -531,7 +531,7 @@ static ssize_t param_attr_show(struct mo
 
 /* sysfs always hands a nul-terminated string in buf.  We rely on that. */
 static ssize_t param_attr_store(struct module_attribute *mattr,
-				struct module *owner,
+				struct module_kobject *km,
 				const char *buf, size_t len)
 {
  	int err;

@@ -807,7 +807,7 @@ static void __init param_sysfs_builtin(v
 }
 
 ssize_t __modver_version_show(struct module_attribute *mattr,
-			      struct module *mod, char *buf)
+			      struct module_kobject *mk, char *buf)
 {
 	struct module_version_attribute *vattr =
 		container_of(mattr, struct module_version_attribute, mattr);

@@ -852,7 +852,7 @@ static ssize_t module_attr_show(struct k
 	if (!attribute->show)
 		return -EIO;
 
-	ret = attribute->show(attribute, mk->mod, buf);
+	ret = attribute->show(attribute, mk, buf);
 
 	return ret;
 }

@@ -871,7 +871,7 @@ static ssize_t module_attr_store(struct 
 	if (!attribute->store)
 		return -EIO;
 
-	ret = attribute->store(attribute, mk->mod, buf, len);
+	ret = attribute->store(attribute, mk, buf, len);
 
 	return ret;
 }

From: Kay Sievers <[email protected]>
Subject: module: add /sys/module/<name>/uevent files

Userspace wants to manage module parameters with udev rules. This currently only works for loaded modules, but not for built-in ones.

To allow access to the built-in modules we need to re-trigger all module load events that happened before any userspace was running. We already do the same thing for all devices, subsystems(buses) and drivers.

This adds the currently missing /sys/module/<name>/uevent files to all module entries.

Signed-off-by Kay Sievers <[email protected]>
Signed-off-by Rusty Russell <[email protected]> (split)
---
 include/linux/module.h |    1 +
 kernel/module.c        |   17 +++++++++++++++++
 kernel/params.c        |    2 ++
 3 files changed, 20 insertions(+)
include/linux/module.h +1 -0
@@ -74,6 +74,7 @@ struct module_version_attribute {
 extern ssize_t __modver_version_show(struct module_attribute *,
 				     struct module_kobject *, char *);
 
+extern struct module_attribute module_uevent;
 
 /* These are either module local, or the kernel's dummy ones. */
 extern int init_module(void);
kernel/module.c +17 -0
@@ -975,10 +975,27 @@ static struct module_attribute initstate
 	.show = show_initstate,
 };
 
+static ssize_t store_uevent(struct module_attribute *mattr,
+			    struct module_kobject *mk,
+			    const char *buffer, size_t count)
+{
+	enum kobject_action action;
+
+	if (kobject_action_type(buffer, count, &action) == 0)
+		kobject_uevent(&mk->kobj, action);
+	return count;
+}
+
+struct module_attribute module_uevent = {
+	.attr = { .name = "uevent", .mode = 0200 },
+	.store = store_uevent,
+};
+
 static struct module_attribute *modinfo_attrs[] = {
 	&modinfo_version,
 	&modinfo_srcversion,
 	&initstate,
+	&module_uevent,
 #ifdef CONFIG_MODULE_UNLOAD
 	&refcnt,
 #endif
kernel/params.c +2 -0
@@ -730,6 +730,8 @@ static struct module_kobject * __init lo
 		mk->kobj.kset = module_kset;
 		err = kobject_init_and_add(&mk->kobj, &module_ktype, NULL,
 					   "%s", name);
+		if (!err)
+			err = sysfs_create_file(&mk->kobj, &module_uevent.attr);
 		if (err) {
 			kobject_put(&mk->kobj);
 			printk(KERN_ERR
author
#11 lore raw
Quoted 5 lines · Rusty Russell
Quoted 4 lines · Kay Sievers
Quoted 3 lines · Kay Sievers
Quoted 2 lines · Rusty Russell
Quoted 1 line · Kay Sievers

On Wed, Jun 22, 2011 at 04:00, Rusty Russell <[email protected]> wrote:

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

I'd prefer that patch first, I think: it's a sensible cleanup.

You want the patch split up in two? You want to remove the mod parameter somehow?

Can we get these 20 lines of code sorted out please? :)

An odd question, since I was waiting for you to do exactly that!

Oh, I was just waiting for your answer to the earlier question 6 lines above here. :)

Quoted 9 lines

Rather than go around again, I have:
1) Split the patch into one which changes the attr functions, and one which adds the uevent file.
2) Fixed the title of the (second) patch to "module: add /sys/module/<name>/uevent files"
3) Fixed up the three checkpatch.pl errors (sure, 2 were just code moves, but we're slowly neatening things).

Here they are below, back-to-back.

Great. Thanks a lot for doing this.

Kay

#12 lore raw
Quoted 11 lines · Kay Sievers
Quoted 10 lines · Rusty Russell
Quoted 9 lines · Kay Sievers
Quoted 7 lines · Kay Sievers
Quoted 3 lines · Rusty Russell
Quoted 1 line · Kay Sievers

On Wed, Jun 22, 2011 at 04:00, Rusty Russell <[email protected]> wrote:

I'd prefer that patch first, I think: it's a sensible cleanup.

You want the patch split up in two? You want to remove the mod parameter somehow?

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Can we get these 20 lines of code sorted out please? :)

An odd question, since I was waiting for you to do exactly that!

Oh, I was just waiting for your answer to the earlier question 6 lines above here. :)

Yes, but rather than go around again, code seemed simpler.

linux-next says we broke CONFIG_MODULES=n though.

I'll fix that now...

Thanks,
Rusty.

Acked-by Greg Kroah-Hartman
#13 lore raw
In reply to Kay Sievers
Quoted 12 lines · Kay Sievers

From: Kay Sievers <[email protected]>
Subject: module: sysfs - add 'uevent' file to allow built-in coldplug

Userspace wants to manage module parameters with udev rules. This currently only works for loaded modules, but not for built-in ones.

To allow access to the built-in modules we need to re-trigger all module load events that happened before any userspace was running. We already do the same thing for all devices, subsystems(buses) and drivers.

Quoted 4 lines

This adds the currently missing /sys/module/<name>/uevent files to all module entries.

Signed-off-by Kay Sievers <[email protected]>
Acked-by Greg Kroah-Hartman <[email protected]>

Rusty, are you going to take this in your tree?

#14 lore raw
Quoted 13 lines · Greg KH
Quoted 12 lines · Kay Sievers

From: Kay Sievers <[email protected]>
Subject: module: sysfs - add 'uevent' file to allow built-in coldplug

Userspace wants to manage module parameters with udev rules. This currently only works for loaded modules, but not for built-in ones.

To allow access to the built-in modules we need to re-trigger all module load events that happened before any userspace was running. We already do the same thing for all devices, subsystems(buses) and drivers.

Quoted 8 lines
Quoted 4 lines

This adds the currently missing /sys/module/<name>/uevent files to all module entries.

Signed-off-by Kay Sievers <[email protected]>
Acked-by Greg Kroah-Hartman <[email protected]>

Rusty, are you going to take this in your tree?

Yep, done.

Thanks,
Rusty.

#15 lore raw
Quoted 17 lines · Rusty Russell
Quoted 16 lines · Greg KH
Quoted 15 lines · Kay Sievers

From: Kay Sievers <[email protected]>
Subject: module: sysfs - add 'uevent' file to allow built-in coldplug

Userspace wants to manage module parameters with udev rules. This currently only works for loaded modules, but not for built-in ones.

To allow access to the built-in modules we need to re-trigger all module load events that happened before any userspace was running. We already do the same thing for all devices, subsystems(buses) and drivers.

This adds the currently missing /sys/module/<name>/uevent files to all module entries.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Signed-off-by Kay Sievers <[email protected]>
Acked-by Greg Kroah-Hartman <[email protected]>

Rusty, are you going to take this in your tree?

Yep, done.

Wonderful, thanks.