* 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-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 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
* 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-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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox