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