Re: kref refcounting breakage in mainline

[Mike Galbraith]

On Wed, 2007-03-07 at 06:39 +0100, Mike Galbraith wrote:
> On Tue, 2007-03-06 at 13:04 -0800, Greg KH wrote:
> > On Tue, Mar 06, 2007 at 06:43:22AM +0100, Mike Galbraith wrote:
> > > On Mon, 2007-03-05 at 16:25 -0800, Greg KH wrote:
> > > 
> > > > Mike, I've reverted this patch, and I don't see any references leaking.
> > > > And, as your patch released the reference on the driver, and the
> > > > module_add_driver() call would not grab a reference to the driver, only
> > > > the module kobject, I don't see what you were trying to fix with this
> > > > patch.
> > > > 
> > > > Do you have a test case that this fixes?
> > > 
> > > What it fixed for me was the hard hang reported below.
> > > 
> > > http://lkml.org/lkml/2007/2/16/96
> > 
> > What specific module are you trying to unload that causes the hang?  I
> > think it might just be a problem with that module, and not with all
> > others.
> 
> It's ipmi_si that's hanging, waits for completion that never comes.
> 
> > So, I'm going to revert your patch and work to try to find the real
> > cause of this problem.
> 
> Yeah, my stab at it seems busted.  I'll take another poke at it to see
> if I can find out why (post 725522b5453dd680412f2b6463a988e4fd148757)
> I'm left with a reference.

Ok, stab #2.

My reference count woes stem from module_remove_driver() not removing
the link created in module_add_driver().  With the below, my box boots
fine.  Since I obviously know spit about driver layer glue, I'll just
call this one a diagnostic, and head for the hills :)

--- linux-2.6.20-rc3/kernel/module.c.org	2007-03-10 15:16:47.000000000 +0100
+++ linux-2.6.20-rc3/kernel/module.c	2007-03-10 15:43:09.000000000 +0100
@@ -2411,14 +2411,28 @@ void module_remove_driver(struct device_
 		return;
 
 	sysfs_remove_link(&drv->kobj, "module");
-	if (drv->owner && drv->owner->mkobj.drivers_dir) {
-		driver_name = make_driver_name(drv);
-		if (driver_name) {
-			sysfs_remove_link(drv->owner->mkobj.drivers_dir,
+	driver_name = make_driver_name(drv);
+	if (!driver_name)
+		return;
+	if (drv->owner && drv->owner->mkobj.drivers_dir)
+		sysfs_remove_link(drv->owner->mkobj.drivers_dir,
 					  driver_name);
-			kfree(driver_name);
-		}
+	else if (drv->mod_name) {
+		struct module_kobject *mk;
+		struct kobject *mkobj;
+
+		/* Lookup built-in module entry in /sys/modules */
+		mkobj = kset_find_obj(&module_subsys.kset, drv->mod_name);
+		if (!mkobj)
+			goto out_free;
+		mk = container_of(mkobj, struct module_kobject, kobj);
+		module_create_drivers_dir(mk);
+		sysfs_remove_link(mk->drivers_dir, driver_name);
+		/* Release reference taken via lookup */
+		kobject_put(mkobj);
 	}
+out_free:
+	kfree(driver_name);
 }
 EXPORT_SYMBOL(module_remove_driver);
 #endif


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/