All of lore.kernel.org
 help / color / mirror / Atom feed
* module: sysfs - add 'uevent' file to allow coldplug
@ 2011-06-18 22:00 Kay Sievers
  2011-06-19 23:23 ` Rusty Russell
  2011-07-01 21:14 ` Greg KH
  0 siblings, 2 replies; 16+ messages in thread
From: Kay Sievers @ 2011-06-18 22:00 UTC (permalink / raw)
  To: Rusty Russell; +Cc: linux-kernel, Greg KH

From: Kay Sievers <kay.sievers@vrfy.org>
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 <kay.sievers@vrfy.org>
---

 include/linux/module.h |   24 +++++++++++++-----------
 kernel/module.c        |   31 ++++++++++++++++++++++++-------
 kernel/params.c        |   12 +++++++-----
 3 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index d9ca2d5..ac6975c 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -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);
diff --git a/kernel/module.c b/kernel/module.c
index 795bdc7..2f0d562 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -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);
diff --git a/kernel/params.c b/kernel/params.c
index ed72e13..d2f02ca 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -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;
 }



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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-06-18 22:00 module: sysfs - add 'uevent' file to allow coldplug Kay Sievers
@ 2011-06-19 23:23 ` Rusty Russell
  2011-06-20 11:20   ` Kay Sievers
  2011-07-01 21:14 ` Greg KH
  1 sibling, 1 reply; 16+ messages in thread
From: Rusty Russell @ 2011-06-19 23:23 UTC (permalink / raw)
  To: Kay Sievers; +Cc: linux-kernel, Greg KH

On Sun, 19 Jun 2011 00:00:29 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
> From: Kay Sievers <kay.sievers@vrfy.org>
> 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.

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-06-19 23:23 ` Rusty Russell
@ 2011-06-20 11:20   ` Kay Sievers
  2011-06-21  1:53     ` Rusty Russell
  0 siblings, 1 reply; 16+ messages in thread
From: Kay Sievers @ 2011-06-20 11:20 UTC (permalink / raw)
  To: Rusty Russell; +Cc: linux-kernel, Greg KH

On Mon, Jun 20, 2011 at 01:23, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Sun, 19 Jun 2011 00:00:29 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
>> From: Kay Sievers <kay.sievers@vrfy.org>
>> 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

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-06-20 11:20   ` Kay Sievers
@ 2011-06-21  1:53     ` Rusty Russell
  2011-06-21 22:47       ` Kay Sievers
  0 siblings, 1 reply; 16+ messages in thread
From: Rusty Russell @ 2011-06-21  1:53 UTC (permalink / raw)
  To: Kay Sievers; +Cc: linux-kernel, Greg KH

On Mon, 20 Jun 2011 13:20:00 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Mon, Jun 20, 2011 at 01:23, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > On Sun, 19 Jun 2011 00:00:29 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
> >> From: Kay Sievers <kay.sievers@vrfy.org>
> >> 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.

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.

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-06-21  1:53     ` Rusty Russell
@ 2011-06-21 22:47       ` Kay Sievers
  2011-06-22  2:00         ` Rusty Russell
  0 siblings, 1 reply; 16+ messages in thread
From: Kay Sievers @ 2011-06-21 22:47 UTC (permalink / raw)
  To: Rusty Russell; +Cc: linux-kernel, Greg KH

On Tue, Jun 21, 2011 at 03:53, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Mon, 20 Jun 2011 13:20:00 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
>> On Mon, Jun 20, 2011 at 01:23, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> > On Sun, 19 Jun 2011 00:00:29 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
>> >> From: Kay Sievers <kay.sievers@vrfy.org>
>> >> 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.
>
> Sorry, that's another vague answer :(
>
> udev already knows about module load

Not for built-ins.

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

The can be changed, and sometimes they need.

> 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


> 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

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-06-21 22:47       ` Kay Sievers
@ 2011-06-22  2:00         ` Rusty Russell
  2011-06-22 10:17           ` Kay Sievers
  0 siblings, 1 reply; 16+ messages in thread
From: Rusty Russell @ 2011-06-22  2:00 UTC (permalink / raw)
  To: Kay Sievers; +Cc: linux-kernel, Greg KH

On Wed, 22 Jun 2011 00:47:55 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Tue, Jun 21, 2011 at 03:53, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > 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*:

>        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:

> 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.

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-06-22  2:00         ` Rusty Russell
@ 2011-06-22 10:17           ` Kay Sievers
  2011-06-23  0:27             ` Rusty Russell
  0 siblings, 1 reply; 16+ messages in thread
From: Kay Sievers @ 2011-06-22 10:17 UTC (permalink / raw)
  To: Rusty Russell; +Cc: linux-kernel, Greg KH

On Wed, Jun 22, 2011 at 04:00, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Wed, 22 Jun 2011 00:47:55 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
>> On Tue, Jun 21, 2011 at 03:53, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> > 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*:
>
>>        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.

> 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.

> 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:
>
>> 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()?

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

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-06-22 10:17           ` Kay Sievers
@ 2011-06-23  0:27             ` Rusty Russell
  2011-06-23 11:24               ` Kay Sievers
  0 siblings, 1 reply; 16+ messages in thread
From: Rusty Russell @ 2011-06-23  0:27 UTC (permalink / raw)
  To: Kay Sievers; +Cc: linux-kernel, Greg KH

On Wed, 22 Jun 2011 12:17:49 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Wed, Jun 22, 2011 at 04:00, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > 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 :)

> > 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...

> > 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.

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-06-23  0:27             ` Rusty Russell
@ 2011-06-23 11:24               ` Kay Sievers
  2011-07-01  3:07                 ` Kay Sievers
  0 siblings, 1 reply; 16+ messages in thread
From: Kay Sievers @ 2011-06-23 11:24 UTC (permalink / raw)
  To: Rusty Russell; +Cc: linux-kernel, Greg KH

On Thu, Jun 23, 2011 at 02:27, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Wed, 22 Jun 2011 12:17:49 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
>> On Wed, Jun 22, 2011 at 04:00, Rusty Russell <rusty@rustcorp.com.au> wrote:

>> > 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.

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

Thanks,
Kay

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-06-23 11:24               ` Kay Sievers
@ 2011-07-01  3:07                 ` Kay Sievers
  2011-07-04  5:05                   ` Rusty Russell
  0 siblings, 1 reply; 16+ messages in thread
From: Kay Sievers @ 2011-07-01  3:07 UTC (permalink / raw)
  To: Rusty Russell; +Cc: linux-kernel, Greg KH

On Thu, Jun 23, 2011 at 13:24, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Thu, Jun 23, 2011 at 02:27, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> On Wed, 22 Jun 2011 12:17:49 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
>>> On Wed, Jun 22, 2011 at 04:00, Rusty Russell <rusty@rustcorp.com.au> wrote:
>
>>> > 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.
>
> 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

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-06-18 22:00 module: sysfs - add 'uevent' file to allow coldplug Kay Sievers
  2011-06-19 23:23 ` Rusty Russell
@ 2011-07-01 21:14 ` Greg KH
  2011-07-04  4:56   ` Rusty Russell
  1 sibling, 1 reply; 16+ messages in thread
From: Greg KH @ 2011-07-01 21:14 UTC (permalink / raw)
  To: Kay Sievers; +Cc: Rusty Russell, linux-kernel

On Sun, Jun 19, 2011 at 12:00:29AM +0200, Kay Sievers wrote:
> From: Kay Sievers <kay.sievers@vrfy.org>
> 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 <kay.sievers@vrfy.org>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

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


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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-07-01 21:14 ` Greg KH
@ 2011-07-04  4:56   ` Rusty Russell
  2011-07-04 15:28     ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Rusty Russell @ 2011-07-04  4:56 UTC (permalink / raw)
  To: Greg KH, Kay Sievers; +Cc: linux-kernel

On Fri, 1 Jul 2011 14:14:49 -0700, Greg KH <greg@kroah.com> wrote:
> On Sun, Jun 19, 2011 at 12:00:29AM +0200, Kay Sievers wrote:
> > From: Kay Sievers <kay.sievers@vrfy.org>
> > 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 <kay.sievers@vrfy.org>
> 
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> Rusty, are you going to take this in your tree?

Yep, done.

Thanks,
Rusty.


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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-07-01  3:07                 ` Kay Sievers
@ 2011-07-04  5:05                   ` Rusty Russell
  2011-07-04 15:56                     ` Kay Sievers
  0 siblings, 1 reply; 16+ messages in thread
From: Rusty Russell @ 2011-07-04  5:05 UTC (permalink / raw)
  To: Kay Sievers; +Cc: linux-kernel, Greg KH

On Fri, 1 Jul 2011 05:07:41 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Thu, Jun 23, 2011 at 13:24, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > On Thu, Jun 23, 2011 at 02:27, Rusty Russell <rusty@rustcorp.com.au> wrote:
> >> On Wed, 22 Jun 2011 12:17:49 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
> >>> On Wed, Jun 22, 2011 at 04:00, Rusty Russell <rusty@rustcorp.com.au> 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 <kay.sievers@vrfy.org>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (split into 2)
---
 include/linux/module.h |   23 ++++++++++++-----------
 kernel/module.c        |   14 +++++++-------
 kernel/params.c        |   10 +++++-----
 3 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -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);
diff --git a/kernel/module.c b/kernel/module.c
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -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);
diff --git a/kernel/params.c b/kernel/params.c
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -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 <kay.sievers@vrfy.org>
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 <kay.sievers@vrfy.org>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (split)
---
 include/linux/module.h |    1 +
 kernel/module.c        |   17 +++++++++++++++++
 kernel/params.c        |    2 ++
 3 files changed, 20 insertions(+)

diff --git a/include/linux/module.h b/include/linux/module.h
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -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);
diff --git a/kernel/module.c b/kernel/module.c
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -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
diff --git a/kernel/params.c b/kernel/params.c
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -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

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-07-04  4:56   ` Rusty Russell
@ 2011-07-04 15:28     ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2011-07-04 15:28 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Kay Sievers, linux-kernel

On Mon, Jul 04, 2011 at 02:26:21PM +0930, Rusty Russell wrote:
> On Fri, 1 Jul 2011 14:14:49 -0700, Greg KH <greg@kroah.com> wrote:
> > On Sun, Jun 19, 2011 at 12:00:29AM +0200, Kay Sievers wrote:
> > > From: Kay Sievers <kay.sievers@vrfy.org>
> > > 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 <kay.sievers@vrfy.org>
> > 
> > Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> > 
> > Rusty, are you going to take this in your tree?
> 
> Yep, done.

Wonderful, thanks.

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-07-04  5:05                   ` Rusty Russell
@ 2011-07-04 15:56                     ` Kay Sievers
  2011-07-06  5:27                       ` Rusty Russell
  0 siblings, 1 reply; 16+ messages in thread
From: Kay Sievers @ 2011-07-04 15:56 UTC (permalink / raw)
  To: Rusty Russell; +Cc: linux-kernel, Greg KH

On Mon, Jul 4, 2011 at 07:05, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Fri, 1 Jul 2011 05:07:41 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
>> On Thu, Jun 23, 2011 at 13:24, Kay Sievers <kay.sievers@vrfy.org> wrote:
>> > On Thu, Jun 23, 2011 at 02:27, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> >> On Wed, 22 Jun 2011 12:17:49 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
>> >>> On Wed, Jun 22, 2011 at 04:00, Rusty Russell <rusty@rustcorp.com.au> 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!

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

> 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

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

* Re: module: sysfs - add 'uevent' file to allow coldplug
  2011-07-04 15:56                     ` Kay Sievers
@ 2011-07-06  5:27                       ` Rusty Russell
  0 siblings, 0 replies; 16+ messages in thread
From: Rusty Russell @ 2011-07-06  5:27 UTC (permalink / raw)
  To: Kay Sievers; +Cc: linux-kernel, Greg KH, Stephen Rothwell

On Mon, 4 Jul 2011 17:56:37 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Mon, Jul 4, 2011 at 07:05, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > On Fri, 1 Jul 2011 05:07:41 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
> >> On Thu, Jun 23, 2011 at 13:24, Kay Sievers <kay.sievers@vrfy.org> wrote:
> >> > On Thu, Jun 23, 2011 at 02:27, Rusty Russell <rusty@rustcorp.com.au> wrote:
> >> >> On Wed, 22 Jun 2011 12:17:49 +0200, Kay Sievers <kay.sievers@vrfy.org> wrote:
> >> >>> On Wed, Jun 22, 2011 at 04:00, Rusty Russell <rusty@rustcorp.com.au> 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!
> 
> 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.

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

end of thread, other threads:[~2011-07-07  0:38 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-18 22:00 module: sysfs - add 'uevent' file to allow coldplug Kay Sievers
2011-06-19 23:23 ` Rusty Russell
2011-06-20 11:20   ` Kay Sievers
2011-06-21  1:53     ` Rusty Russell
2011-06-21 22:47       ` Kay Sievers
2011-06-22  2:00         ` Rusty Russell
2011-06-22 10:17           ` Kay Sievers
2011-06-23  0:27             ` Rusty Russell
2011-06-23 11:24               ` Kay Sievers
2011-07-01  3:07                 ` Kay Sievers
2011-07-04  5:05                   ` Rusty Russell
2011-07-04 15:56                     ` Kay Sievers
2011-07-06  5:27                       ` Rusty Russell
2011-07-01 21:14 ` Greg KH
2011-07-04  4:56   ` Rusty Russell
2011-07-04 15:28     ` Greg KH

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.