[PATCH 2.6.10-rc2 0/4] module sysfs: module sysfs related clean ups

[PATCH 2.6.10-rc2 0/4] module sysfs: module sysfs related clean ups

[Tejun Heo]

 Hello,

 These are four patches to simplify/clean up implementation of module
sysfs stuff.

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

 Thanks.

-- 
tejun

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

[Tejun Heo]

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


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-export/include/linux/module.h
===================================================================
--- linux-export.orig/include/linux/module.h	2004-11-23 10:57:37.000000000 +0900
+++ linux-export/include/linux/module.h	2004-11-23 11:31:32.000000000 +0900
@@ -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 */
Index: linux-export/kernel/module.c
===================================================================
--- linux-export.orig/kernel/module.c	2004-11-23 10:57:37.000000000 +0900
+++ linux-export/kernel/module.c	2004-11-23 11:31:32.000000000 +0900
@@ -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);
 
Index: linux-export/kernel/params.c
===================================================================
--- linux-export.orig/kernel/params.c	2004-11-23 10:57:38.000000000 +0900
+++ linux-export/kernel/params.c	2004-11-23 11:31:32.000000000 +0900
@@ -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);
 
@@ -710,14 +710,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);

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

[Tejun Heo]

On Tue, Nov 23, 2004 at 11:45:37AM +0900, Tejun Heo wrote:
> 02_module_attribute_extension.patch
> 	Modify module_attribute show/store methods to accept self
> 	argument to enable further extensions.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-export/include/linux/module.h
===================================================================
--- linux-export.orig/include/linux/module.h	2004-11-23 11:31:32.000000000 +0900
+++ linux-export/include/linux/module.h	2004-11-23 11:32:13.000000000 +0900
@@ -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
Index: linux-export/kernel/module.c
===================================================================
--- linux-export.orig/kernel/module.c	2004-11-23 11:31:32.000000000 +0900
+++ linux-export/kernel/module.c	2004-11-23 11:32:13.000000000 +0900
@@ -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);
Index: linux-export/kernel/params.c
===================================================================
--- linux-export.orig/kernel/params.c	2004-11-23 11:31:32.000000000 +0900
+++ linux-export/kernel/params.c	2004-11-23 11:32:13.000000000 +0900
@@ -691,7 +691,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);
 

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

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


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-export/include/linux/module.h
===================================================================
--- linux-export.orig/include/linux/module.h	2004-11-23 11:32:13.000000000 +0900
+++ linux-export/include/linux/module.h	2004-11-23 11:32:28.000000000 +0900
@@ -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. */
Index: linux-export/kernel/module.c
===================================================================
--- linux-export.orig/kernel/module.c	2004-11-23 11:32:13.000000000 +0900
+++ linux-export/kernel/module.c	2004-11-23 11:32:28.000000000 +0900
@@ -937,76 +937,71 @@ static unsigned long resolve_symbol(Elf_
  * J. Corbet <corbet@lwn.net>
  */
 #ifdef CONFIG_KALLSYMS
-static void module_sect_attrs_release(struct kobject *kobj)
-{
-	kfree(container_of(kobj, struct module_sections, kobj));
-}
-
-static ssize_t module_sect_show(struct kobject *kobj, struct attribute *attr,
-		char *buf)
+static ssize_t module_sect_show(struct module_attribute *mattr,
+				struct module *mod, char *buf)
 {
 	struct module_sect_attr *sattr =
-		container_of(attr, struct module_sect_attr, attr);
+		container_of(mattr, struct module_sect_attr, mattr);
 	return sprintf(buf, "0x%lx\n", sattr->address);
 }
 
-static struct sysfs_ops module_sect_ops = {
-	.show = module_sect_show,
-};
-
-static struct kobj_type module_sect_ktype = {
-	.sysfs_ops = &module_sect_ops,
-	.release =   module_sect_attrs_release,
-};
-
 static void add_sect_attrs(struct module *mod, unsigned int nsect,
 		char *secstrings, Elf_Shdr *sechdrs)
 {
-	unsigned int nloaded = 0, i;
+	unsigned int nloaded = 0, i, size[2];
+	struct module_sect_attrs *sect_attrs;
 	struct module_sect_attr *sattr;
+	struct attribute **gattr;
 	
 	/* Count loaded sections and allocate structures */
 	for (i = 0; i < nsect; i++)
 		if (sechdrs[i].sh_flags & SHF_ALLOC)
 			nloaded++;
-	mod->sect_attrs = kmalloc(sizeof(struct module_sections) +
-			nloaded*sizeof(mod->sect_attrs->attrs[0]), GFP_KERNEL);
-	if (! mod->sect_attrs)
+	size[0] = ALIGN(sizeof(*sect_attrs)
+			+ nloaded * sizeof(sect_attrs->attrs[0]),
+			sizeof(sect_attrs->grp.attrs[0]));
+	size[1] = (nloaded + 1) * sizeof(sect_attrs->grp.attrs[0]);
+	if (! (sect_attrs = kmalloc(size[0] + size[1], GFP_KERNEL)))
 		return;
 
-	/* sections entry setup */
-	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.ktype = &module_sect_ktype;
-	if (kobject_register(&mod->sect_attrs->kobj))
-		goto out;
+	/* Setup section attributes. */
+	sect_attrs->grp.name = "sections";
+	sect_attrs->grp.attrs = (void *)sect_attrs + size[0];
 
-	/* And the section attributes. */
-	sattr = &mod->sect_attrs->attrs[0];
+	sattr = &sect_attrs->attrs[0];
+	gattr = &sect_attrs->grp.attrs[0];
 	for (i = 0; i < nsect; i++) {
 		if (! (sechdrs[i].sh_flags & SHF_ALLOC))
 			continue;
 		sattr->address = sechdrs[i].sh_addr;
 		strlcpy(sattr->name, secstrings + sechdrs[i].sh_name,
-				MODULE_SECT_NAME_LEN);
-		sattr->attr.name = sattr->name;
-		sattr->attr.owner = mod;
-		sattr->attr.mode = S_IRUGO;
-		(void) sysfs_create_file(&mod->sect_attrs->kobj, &sattr->attr);
-		sattr++;
+			MODULE_SECT_NAME_LEN);
+		sattr->mattr.show = module_sect_show;
+		sattr->mattr.store = NULL;
+		sattr->mattr.attr.name = sattr->name;
+		sattr->mattr.attr.owner = mod;
+		sattr->mattr.attr.mode = S_IRUGO;
+		*(gattr++) = &(sattr++)->mattr.attr;
 	}
+	*gattr = NULL;
+
+	if (sysfs_create_group(&mod->mkobj.kobj, &sect_attrs->grp))
+		goto out;
+
+	mod->sect_attrs = sect_attrs;
 	return;
   out:
-	kfree(mod->sect_attrs);
-	mod->sect_attrs = NULL;
+	kfree(sect_attrs);
 }
 
 static void remove_sect_attrs(struct module *mod)
 {
 	if (mod->sect_attrs) {
-		kobject_unregister(&mod->sect_attrs->kobj);
+		sysfs_remove_group(&mod->mkobj.kobj,
+				   &mod->sect_attrs->grp);
+		/* We are positive that no one is using any sect attrs
+		 * at this point.  Deallocate immediately. */
+		kfree(mod->sect_attrs);
 		mod->sect_attrs = NULL;
 	}
 }

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

[Tejun Heo]

> 04_module_paramters_attr_grp.patch 
> 	Reimplement parameter attributes using attribute group.
> 	Ditto.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-export/include/linux/module.h
===================================================================
--- linux-export.orig/include/linux/module.h	2004-11-23 11:32:28.000000000 +0900
+++ linux-export/include/linux/module.h	2004-11-23 11:32:41.000000000 +0900
@@ -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;
Index: linux-export/kernel/params.c
===================================================================
--- linux-export.orig/kernel/params.c	2004-11-23 11:32:13.000000000 +0900
+++ linux-export/kernel/params.c	2004-11-23 11:32:41.000000000 +0900
@@ -357,26 +357,23 @@ extern struct kernel_param __start___par
 
 struct param_attribute
 {
-	struct attribute attr;
+	struct module_attribute mattr;
 	struct kernel_param *param;
 };
 
-struct param_kobject
+struct module_param_attrs
 {
-	struct kobject kobj;
-
-	unsigned int num_attributes;
-	struct param_attribute attr[0];
+	struct attribute_group grp;
+	struct param_attribute attrs[0];
 };
 
-#define to_param_attr(n) container_of(n, struct param_attribute, attr);
+#define to_param_attr(n) container_of(n, struct param_attribute, mattr);
 
-static ssize_t param_attr_show(struct kobject *kobj,
-			       struct attribute *attr,
-			       char *buf)
+static ssize_t param_attr_show(struct module_attribute *mattr,
+			       struct module *mod, char *buf)
 {
 	int count;
-	struct param_attribute *attribute = to_param_attr(attr);
+	struct param_attribute *attribute = to_param_attr(mattr);
 
 	if (!attribute->param->get)
 		return -EPERM;
@@ -390,12 +387,12 @@ static ssize_t param_attr_show(struct ko
 }
 
 /* sysfs always hands a nul-terminated string in buf.  We rely on that. */
-static ssize_t param_attr_store(struct kobject *kobj,
-				struct attribute *attr,
+static ssize_t param_attr_store(struct module_attribute *mattr,
+				struct module *owner,
 				const char *buf, size_t len)
 {
  	int err;
-	struct param_attribute *attribute = to_param_attr(attr);
+	struct param_attribute *attribute = to_param_attr(mattr);
 
 	if (!attribute->param->set)
 		return -EPERM;
@@ -406,27 +403,6 @@ static ssize_t param_attr_store(struct k
 	return err;
 }
 
-
-static struct sysfs_ops param_sysfs_ops = {
-	.show = param_attr_show,
-	.store = param_attr_store,
-};
-
-static void param_kobj_release(struct kobject *kobj)
-{
-	kfree(container_of(kobj, struct param_kobject, kobj));
-}
-
-static struct kobj_type param_ktype = {
-	.sysfs_ops =	&param_sysfs_ops,
-	.release =	&param_kobj_release,
-};
-
-static struct kset param_kset = {
-	.subsys =	&module_subsys,
-	.ktype =	&param_ktype,
-};
-
 #ifdef CONFIG_MODULES
 #define __modinit
 #else
@@ -434,54 +410,6 @@ static struct kset param_kset = {
 #endif
 
 /*
- * param_add_attribute - actually adds an parameter to sysfs
- * @mod: owner of parameter
- * @pk: param_kobject the attribute shall be assigned to.
- *      One per module, one per KBUILD_MODNAME.
- * @kp: kernel_param to be added
- * @skip: offset where the parameter name start in kp->name.
- * Needed for built-in modules
- *
- * Fill in data into appropriate &pk->attr[], and create sysfs file.
- */
-static __modinit int param_add_attribute(struct module *mod,
-					 struct param_kobject *pk,
-					 struct kernel_param *kp,
-					 unsigned int skip)
-{
-	struct param_attribute *a;
-	int err;
-
-	a = &pk->attr[pk->num_attributes];
-	a->attr.name = (char *) &kp->name[skip];
-	a->attr.owner = mod;
-	a->attr.mode = kp->perm;
-	a->param = kp;
-	err = sysfs_create_file(&pk->kobj, &a->attr);
-	if (!err)
-		pk->num_attributes++;
-	return err;
-}
-
-/*
- * param_sysfs_remove - remove sysfs support for one module or KBUILD_MODNAME
- * @pk: struct param_kobject which is to be removed
- *
- * Called when an error in registration occurs or a module is removed
- * from the system.
- */
-static __modinit void param_sysfs_remove(struct param_kobject *pk)
-{
-	unsigned int i;
-	for (i = 0; i < pk->num_attributes; i++)
-		sysfs_remove_file(&pk->kobj,&pk->attr[i].attr);
-
-	/* Calls param_kobj_release */
-	kobject_unregister(&pk->kobj);
-}
-
-
-/*
  * param_sysfs_setup - setup sysfs support for one module or KBUILD_MODNAME
  * @mk: struct module_kobject (contains parent kobject)
  * @kparam: array of struct kernel_param, the actual parameter definitions
@@ -492,15 +420,17 @@ static __modinit void param_sysfs_remove
  * in sysfs. A pointer to the param_kobject is returned on success,
  * NULL if there's no parameter to export, or other ERR_PTR(err).
  */
-static __modinit struct param_kobject *
+static __modinit struct module_param_attrs *
 param_sysfs_setup(struct module_kobject *mk,
 		  struct kernel_param *kparam,
 		  unsigned int num_params,
 		  unsigned int name_skip)
 {
-	struct param_kobject *pk;
+	struct module_param_attrs *mp;
 	unsigned int valid_attrs = 0;
-	unsigned int i;
+	unsigned int i, size[2];
+	struct param_attribute *pattr;
+	struct attribute **gattr;
 	int err;
 
 	for (i=0; i<num_params; i++) {
@@ -511,42 +441,39 @@ param_sysfs_setup(struct module_kobject 
 	if (!valid_attrs)
 		return NULL;
 
-	pk = kmalloc(sizeof(struct param_kobject)
-		     + sizeof(struct param_attribute) * valid_attrs,
-		     GFP_KERNEL);
-	if (!pk)
+	size[0] = ALIGN(sizeof(*mp) +
+			valid_attrs * sizeof(mp->attrs[0]),
+			sizeof(mp->grp.attrs[0]));
+	size[1] = (valid_attrs + 1) * sizeof(mp->grp.attrs[0]);
+
+	mp = kmalloc(size[0] + size[1], GFP_KERNEL);
+	if (!mp)
 		return ERR_PTR(-ENOMEM);
-	memset(pk, 0, sizeof(struct param_kobject)
-	       + sizeof(struct param_attribute) * valid_attrs);
 
-	err = kobject_set_name(&pk->kobj, "parameters");
-	if (err)
-		goto out;
-
-	pk->kobj.kset = &param_kset;
-	pk->kobj.parent = &mk->kobj;
-	err = kobject_register(&pk->kobj);
-	if (err)
-		goto out;
+	mp->grp.name = "parameters";
+	mp->grp.attrs = (void *)mp + size[0];
 
+	pattr = &mp->attrs[0];
+	gattr = &mp->grp.attrs[0];
 	for (i = 0; i < num_params; i++) {
-		if (kparam[i].perm) {
-			err = param_add_attribute(mk->mod, pk,
-						  &kparam[i], name_skip);
-			if (err)
-				goto out_unreg;
+		struct kernel_param *kp = &kparam[i];
+		if (kp->perm) {
+			pattr->param = kp;
+			pattr->mattr.show = param_attr_show;
+			pattr->mattr.store = param_attr_store;
+			pattr->mattr.attr.name = (char *)&kp->name[name_skip];
+			pattr->mattr.attr.owner = mk->mod;
+			pattr->mattr.attr.mode = kp->perm;
+			*(gattr++) = &(pattr++)->mattr.attr;
 		}
 	}
+	*gattr = NULL;
 
-	return pk;
-
-out_unreg:
-	param_sysfs_remove(pk);
-	return ERR_PTR(err);
-
-out:
-	kfree(pk);
-	return ERR_PTR(err);
+	if ((err = sysfs_create_group(&mk->kobj, &mp->grp))) {
+		kfree(mp);
+		return ERR_PTR(err);
+	}
+	return mp;
 }
 
 
@@ -565,13 +492,13 @@ int module_param_sysfs_setup(struct modu
 			     struct kernel_param *kparam,
 			     unsigned int num_params)
 {
-	struct param_kobject *pk;
+	struct module_param_attrs *mp;
 
-	pk = param_sysfs_setup(&mod->mkobj, kparam, num_params, 0);
-	if (IS_ERR(pk))
-		return PTR_ERR(pk);
+	mp = param_sysfs_setup(&mod->mkobj, kparam, num_params, 0);
+	if (IS_ERR(mp))
+		return PTR_ERR(mp);
 
-	mod->params_kobject = pk;
+	mod->param_attrs = mp;
 	return 0;
 }
 
@@ -584,9 +511,13 @@ int module_param_sysfs_setup(struct modu
  */
 void module_param_sysfs_remove(struct module *mod)
 {
-	if (mod->params_kobject) {
-		param_sysfs_remove(mod->params_kobject);
-		mod->params_kobject = NULL;
+	if (mod->param_attrs) {
+		sysfs_remove_group(&mod->mkobj.kobj,
+				   &mod->param_attrs->grp);
+		/* We are positive that no one is using any param
+		 * attrs at this point.  Deallocate immediately. */
+		kfree(mod->param_attrs);
+		mod->param_attrs = NULL;
 	}
 }
 #endif
@@ -722,8 +653,6 @@ decl_subsys(module, &module_ktype, NULL)
 static int __init param_sysfs_init(void)
 {
 	subsystem_register(&module_subsys);
-	kobject_set_name(&param_kset.kobj, "parameters");
-	kset_init(&param_kset);
 
 	param_sysfs_builtin();
 

Re: [PATCH 2.6.10-rc2 0/4] module sysfs: module sysfs related clean ups

[Rusty Russell]

On Tue, 2004-11-23 at 11:45 +0900, Tejun Heo wrote:
>  Hello,
> 
>  These are four patches to simplify/clean up implementation of module
> sysfs stuff.

All look good to me.  Thanks!

Greg?
Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

Re: [PATCH 2.6.10-rc2 0/4] module sysfs: module sysfs related clean ups

[Greg KH]

On Tue, Nov 23, 2004 at 02:58:28PM +1100, Rusty Russell wrote:
> On Tue, 2004-11-23 at 11:45 +0900, Tejun Heo wrote:
> >  Hello,
> > 
> >  These are four patches to simplify/clean up implementation of module
> > sysfs stuff.
> 
> All look good to me.  Thanks!
> 
> Greg?

No objections from me either, look nice.  Want me to stage them in a bk
tree for inclusion in the -mm trees, so they get some coverage for a
while?

Or do you want to do that?

thanks,

greg k-h

Re: [PATCH 2.6.10-rc2 0/4] module sysfs: module sysfs related clean ups

[Rusty Russell]

On Mon, 2004-11-22 at 20:31 -0800, Greg KH wrote:
> On Tue, Nov 23, 2004 at 02:58:28PM +1100, Rusty Russell wrote:
> > On Tue, 2004-11-23 at 11:45 +0900, Tejun Heo wrote:
> > >  Hello,
> > > 
> > >  These are four patches to simplify/clean up implementation of module
> > > sysfs stuff.
> > 
> > All look good to me.  Thanks!
> > 
> > Greg?
> 
> No objections from me either, look nice.  Want me to stage them in a bk
> tree for inclusion in the -mm trees, so they get some coverage for a
> while?
> 
> Or do you want to do that?

Please take it as a sign of my infinite respect for you, that I insist
on you doing all the work.

Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman