Skip to contentSkip to conversation

module sysfs: module sysfs related cleanups

PATCH In review Tejun Heo 2 participants 6 messages Dec 20, 2004 – Dec 21, 2004 Module support

lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

cover module sysfs: module sysfs related cleanups

Hello, Greg.

Here are the regenerated patches with the fix.

01_mkobj_inline.patch
Make module.mkobj inline. As this is simpler and what's usually done with kobjs when it's representing an entity.

02_module_attribute_extension.patch
Modify module_attribute show/store methods to accept self argument to enable further extensions.

03_module_sections_attr_grp.patch
Reimplement section attributes using attribute group. This makes more sense, for, while they reside in a separate subdirectory, they belong to the ownig module and their lifetime exactly equals the lifetime of the owning module, and it's simpler.

04_module_paramters_attr_grp.patch
Reimplement parameter attributes using attribute group. This makes more sense, for, while they reside in a separate subdirectory, they belong to the ownig module and their lifetime exactly equals the lifetime of the owning module, and it's simpler.

Thanks.

Signature · 1 line
tejun
author
#1 lore raw

Re: [PATCH REPOST 2.6.10-rc3 1/4] module sysfs: make module.mkobj inline

01_mkobj_inline.patch
Make module.mkobj inline. As this is simpler and what's usually done with kobjs when it's representing an entity.

include/linux/module.h +1 -1
Index: linux-export/include/linux/module.h
===================================================================
@@ -250,7 +250,7 @@ struct module
 	char name[MODULE_NAME_LEN];
 
 	/* Sysfs stuff. */
-	struct module_kobject *mkobj;
+	struct module_kobject mkobj;
 	struct param_kobject *params_kobject;
 
 	/* Exported symbols */
kernel/module.c +11 -24
Index: linux-export/kernel/module.c
===================================================================
@@ -964,9 +964,6 @@ static void add_sect_attrs(struct module
 	unsigned int nloaded = 0, i;
 	struct module_sect_attr *sattr;
 	
-	if (!mod->mkobj)
-		return;
-	
 	/* Count loaded sections and allocate structures */
 	for (i = 0; i < nsect; i++)
 		if (sechdrs[i].sh_flags & SHF_ALLOC)
@@ -980,7 +977,7 @@ static void add_sect_attrs(struct module
 	memset(mod->sect_attrs, 0, sizeof(struct module_sections));
 	if (kobject_set_name(&mod->sect_attrs->kobj, "sections"))
 		goto out;
-	mod->sect_attrs->kobj.parent = &mod->mkobj->kobj;
+	mod->sect_attrs->kobj.parent = &mod->mkobj.kobj;
 	mod->sect_attrs->kobj.ktype = &module_sect_ktype;
 	if (kobject_register(&mod->sect_attrs->kobj))
 		goto out;
@@ -1029,11 +1026,11 @@ static inline void remove_sect_attrs(str
 #ifdef CONFIG_MODULE_UNLOAD
 static inline int module_add_refcnt_attr(struct module *mod)
 {
-	return sysfs_create_file(&mod->mkobj->kobj, &refcnt.attr);
+	return sysfs_create_file(&mod->mkobj.kobj, &refcnt.attr);
 }
 static void module_remove_refcnt_attr(struct module *mod)
 {
-	return sysfs_remove_file(&mod->mkobj->kobj, &refcnt.attr);
+	return sysfs_remove_file(&mod->mkobj.kobj, &refcnt.attr);
 }
 #else
 static inline int module_add_refcnt_attr(struct module *mod)
@@ -1052,17 +1049,13 @@ static int mod_sysfs_setup(struct module
 {
 	int err;
 
-	mod->mkobj = kmalloc(sizeof(struct module_kobject), GFP_KERNEL);
-	if (!mod->mkobj)
-		return -ENOMEM;
-
-	memset(&mod->mkobj->kobj, 0, sizeof(mod->mkobj->kobj));
-	err = kobject_set_name(&mod->mkobj->kobj, "%s", mod->name);
+	memset(&mod->mkobj.kobj, 0, sizeof(mod->mkobj.kobj));
+	err = kobject_set_name(&mod->mkobj.kobj, "%s", mod->name);
 	if (err)
 		goto out;
-	kobj_set_kset_s(mod->mkobj, module_subsys);
-	mod->mkobj->mod = mod;
-	err = kobject_register(&mod->mkobj->kobj);
+	kobj_set_kset_s(&mod->mkobj, module_subsys);
+	mod->mkobj.mod = mod;
+	err = kobject_register(&mod->mkobj.kobj);
 	if (err)
 		goto out;
 
@@ -1077,11 +1070,8 @@ static int mod_sysfs_setup(struct module
 	return 0;
 
 out_unreg:
-	/* Calls module_kobj_release */
-	kobject_unregister(&mod->mkobj->kobj);
-	return err;
+	kobject_unregister(&mod->mkobj.kobj);
 out:
-	kfree(mod->mkobj);
 	return err;
 }
 
@@ -1090,8 +1080,7 @@ static void mod_kobject_remove(struct mo
 	module_remove_refcnt_attr(mod);
 	module_param_sysfs_remove(mod);
 
-	/* Calls module_kobj_release */
-	kobject_unregister(&mod->mkobj->kobj);
+	kobject_unregister(&mod->mkobj.kobj);
 }
 
 /* Free a module, remove from lists, etc (must hold module mutex). */
@@ -2089,11 +2078,9 @@ void module_add_driver(struct module *mo
 {
 	if (!mod || !drv)
 		return;
-	if (!mod->mkobj)
-		return;
 
 	/* Don't check return code; this call is idempotent */
-	sysfs_create_link(&drv->kobj, &mod->mkobj->kobj, "module");
+	sysfs_create_link(&drv->kobj, &mod->mkobj.kobj, "module");
 }
 EXPORT_SYMBOL(module_add_driver);
 
kernel/params.c +4 -8
Index: linux-export/kernel/params.c
===================================================================
@@ -567,7 +567,7 @@ int module_param_sysfs_setup(struct modu
 {
 	struct param_kobject *pk;
 
-	pk = param_sysfs_setup(mod->mkobj, kparam, num_params, 0);
+	pk = param_sysfs_setup(&mod->mkobj, kparam, num_params, 0);
 	if (IS_ERR(pk))
 		return PTR_ERR(pk);
 
@@ -610,8 +610,10 @@ static void __init kernel_param_sysfs_se
 	kobject_register(&mk->kobj);
 
 	/* no need to keep the kobject if no parameter is exported */
-	if (!param_sysfs_setup(mk, kparam, num_params, name_skip))
+	if (!param_sysfs_setup(mk, kparam, num_params, name_skip)) {
 		kobject_unregister(&mk->kobj);
+		kfree(mk);
+	}
 }
 
 /*
@@ -710,14 +712,8 @@ static struct sysfs_ops module_sysfs_ops
 };
 #endif
 
-static void module_kobj_release(struct kobject *kobj)
-{
-	kfree(container_of(kobj, struct module_kobject, kobj));
-}
-
 static struct kobj_type module_ktype = {
 	.sysfs_ops =	&module_sysfs_ops,
-	.release =	&module_kobj_release,
 };
 
 decl_subsys(module, &module_ktype, NULL);
author
#2 lore raw

Re: [PATCH REPOST 2.6.10-rc3 2/4] module sysfs: expand module_attribute methods

In reply to Tejun Heo

02_module_attribute_extension.patch
Modify module_attribute show/store methods to accept self argument to enable further extensions.

include/linux/module.h +3 -2
Index: linux-export/include/linux/module.h
===================================================================
@@ -48,8 +48,9 @@ struct module;
 
 struct module_attribute {
         struct attribute attr;
-        ssize_t (*show)(struct module *, char *);
-        ssize_t (*store)(struct module *, const char *, size_t count);
+        ssize_t (*show)(struct module_attribute *, struct module *, char *);
+        ssize_t (*store)(struct module_attribute *, struct module *,
+			 const char *, size_t count);
 };
 
 struct module_kobject
kernel/module.c +2 -1
Index: linux-export/kernel/module.c
===================================================================
@@ -651,7 +651,8 @@ void symbol_put_addr(void *addr)
 }
 EXPORT_SYMBOL_GPL(symbol_put_addr);
 
-static ssize_t show_refcnt(struct module *mod, char *buffer)
+static ssize_t show_refcnt(struct module_attribute *mattr,
+			   struct module *mod, char *buffer)
 {
 	/* sysfs holds a reference */
 	return sprintf(buffer, "%u\n", module_refcount(mod)-1);
kernel/params.c +1 -1
Index: linux-export/kernel/params.c
===================================================================
@@ -693,7 +693,7 @@ static ssize_t module_attr_show(struct k
 	if (!try_module_get(mk->mod))
 		return -ENODEV;
 
-	ret = attribute->show(mk->mod, buf);
+	ret = attribute->show(attribute, mk->mod, buf);
 
 	module_put(mk->mod);
 
author
#3 lore raw

Re: [PATCH REPOST 2.6.10-rc3 3/4] module sysfs: sections attr reimplemented using attr group

In reply to Tejun Heo

03_module_sections_attr_grp.patch
Reimplement section attributes using attribute group. This makes more sense, for, while they reside in a separate subdirectory, they belong to the ownig module and their lifetime exactly equals the lifetime of the owning module, and it's simpler.

include/linux/module.h +4 -4
Index: linux-export/include/linux/module.h
===================================================================
@@ -227,14 +227,14 @@ enum module_state
 #define MODULE_SECT_NAME_LEN 32
 struct module_sect_attr
 {
-	struct attribute attr;
+	struct module_attribute mattr;
 	char name[MODULE_SECT_NAME_LEN];
 	unsigned long address;
 };
 
-struct module_sections
+struct module_sect_attrs
 {
-	struct kobject kobj;
+	struct attribute_group grp;
 	struct module_sect_attr attrs[0];
 };
 
@@ -313,7 +313,7 @@ struct module
 	char *strtab;
 
 	/* Section attributes */
-	struct module_sections *sect_attrs;
+	struct module_sect_attrs *sect_attrs;
 #endif
 
 	/* Per-cpu data. */
kernel/module.c +35 -40

111 lines not on this page · raw message

author
#4 lore raw

Re: [PATCH REPOST 2.6.10-rc3 4/4] module sysfs: module parameters reimplemented using attr group

In reply to Tejun Heo

04_module_paramters_attr_grp.patch
Reimplement parameter attributes using attribute group. This makes more sense, for, while they reside in a separate subdirectory, they belong to the ownig module and their lifetime exactly equals the lifetime of the owning module, and it's simpler.

include/linux/module.h +2 -2
Index: linux-export/include/linux/module.h
===================================================================
@@ -238,7 +238,7 @@ struct module_sect_attrs
 	struct module_sect_attr attrs[0];
 };
 
-struct param_kobject;
+struct module_param_attrs;
 
 struct module
 {
@@ -252,7 +252,7 @@ struct module
 
 	/* Sysfs stuff. */
 	struct module_kobject mkobj;
-	struct param_kobject *params_kobject;
+	struct module_param_attrs *param_attrs;
 
 	/* Exported symbols */
 	const struct kernel_symbol *syms;
kernel/params.c +54 -125

260 lines not on this page · raw message

#5 lore raw
In reply to Tejun Heo
Quoted 3 lines · Tejun Heo

Hello, Greg.

Here are the regenerated patches with the fix.

Thanks, I've applied all 4 patches to my trees, and they will show up in the next -mm release.

greg k-h