[Open-FCoE] [RFC PATCH V3 13/13] fcoe: Fix module ref count bug by adding NETDEV UNREGISTER handling

Chris Leech christopher.leech at intel.com
Thu Jul 9 00:06:31 UTC 2009


Fixes reference counting on fcoe_instance and net_device, and adds
NETDEV_UNREGISTER notifier handling so that you can unload network drivers.
FCoE no longer increments the module use count for the network driver.

Based in part by an earlier patch from John Fastabend

John's patch description:
Currently, the netdev module ref count is not decremented with module_put()
when the module is unloaded while fcoe instances are present. To fix this
removed reference count on netdev module completely and added functionality to
netdev event handling for NETDEV_UNREGISTER events.

This allows fcoe to remove devices cleanly when the netdev module is unloaded
so we no longer need to hold a reference count for the netdev module.

Signed-off-by: Chris Leech <christopher.leech at intel.com>
---

 drivers/scsi/fcoe/fcoe.c |  118 +++++++++++-----------------------------------
 1 files changed, 28 insertions(+), 90 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 3e5fb2a..c5b8f75 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -72,6 +72,7 @@ static void fcoe_check_wait_queue(struct fc_lport *, struct sk_buff *);
 static int fcoe_device_notification(struct notifier_block *, ulong, void *);
 static void fcoe_dev_setup(void);
 static void fcoe_dev_cleanup(void);
+static int __fcoe_destroy(struct net_device *netdev);
 
 /* notification function from net device */
 static struct notifier_block fcoe_notifier = {
@@ -228,6 +229,7 @@ static struct fcoe_interface *fcoe_interface_create(struct net_device *netdev)
 		return NULL;
 	}
 
+	dev_hold(netdev);
 	kref_init(&fcoe->kref);
 
 	/*
@@ -273,10 +275,13 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
 static void fcoe_interface_release(struct kref *kref)
 {
 	struct fcoe_interface *fcoe;
+	struct net_device *netdev;
 
 	fcoe = container_of(kref, struct fcoe_interface, kref);
+	netdev = fcoe->real_dev;
 	fcoe_interface_cleanup(fcoe);
 	kfree(fcoe);
+	dev_put(netdev);
 }
 
 /**
@@ -617,8 +622,7 @@ static void fcoe_if_destroy(struct fc_lport *lport)
 	/* Free memory used by statistical counters */
 	fc_lport_free_stats(lport);
 
-	/* Release the net_device and Scsi_Host */
-	dev_put(fcoe->real_dev);
+	/* Release the Scsi_Host */
 	scsi_host_put(lport->host);
 	fcoe_interface_put(fcoe);
 }
@@ -746,7 +750,6 @@ static struct fc_lport *fcoe_if_create(struct fcoe_interface *fcoe,
 		goto out_lp_destroy;
 	}
 
-	dev_hold(netdev);
 	fcoe_interface_get(fcoe);
 	return lport;
 
@@ -1545,6 +1548,10 @@ static int fcoe_device_notification(struct notifier_block *notifier,
 		break;
 	case NETDEV_REGISTER:
 		break;
+	case NETDEV_UNREGISTER:
+		__fcoe_destroy(fcoe->real_dev);
+		goto out;
+		break;
 	default:
 		FCOE_NETDEV_DBG(real_dev, "Unknown event %ld "
 				"from netdev netlink\n", event);
@@ -1581,73 +1588,18 @@ static struct net_device *fcoe_if_to_netdev(const char *buffer)
 	return NULL;
 }
 
-/**
- * fcoe_netdev_to_module_owner() - finds out the driver module of the netdev
- * @netdev: the target netdev
- *
- * Returns: ptr to the struct module, NULL for failure
- */
-static struct module *
-fcoe_netdev_to_module_owner(const struct net_device *netdev)
-{
-	struct device *dev;
-
-	if (!netdev)
-		return NULL;
-
-	dev = netdev->dev.parent;
-	if (!dev)
-		return NULL;
-
-	if (!dev->driver)
-		return NULL;
-
-	return dev->driver->owner;
-}
-
-/**
- * fcoe_ethdrv_get() - Hold the Ethernet driver
- * @netdev: the target netdev
- *
- * Holds the Ethernet driver module by try_module_get() for
- * the corresponding netdev.
- *
- * Returns: 0 for success
- */
-static int fcoe_ethdrv_get(const struct net_device *netdev)
-{
-	struct module *owner;
-
-	owner = fcoe_netdev_to_module_owner(netdev);
-	if (owner) {
-		FCOE_NETDEV_DBG(netdev, "Hold driver module %s\n",
-				module_name(owner));
-		return  try_module_get(owner);
-	}
-	return -ENODEV;
-}
-
-/**
- * fcoe_ethdrv_put() - Release the Ethernet driver
- * @netdev: the target netdev
- *
- * Releases the Ethernet driver module by module_put for
- * the corresponding netdev.
- *
- * Returns: 0 for success
- */
-static int fcoe_ethdrv_put(const struct net_device *netdev)
+static int __fcoe_destroy(struct net_device *netdev)
 {
-	struct module *owner;
+	struct fc_lport *lport;
 
-	owner = fcoe_netdev_to_module_owner(netdev);
-	if (owner) {
-		FCOE_NETDEV_DBG(netdev, "Release driver module %s\n",
-				module_name(owner));
-		module_put(owner);
-		return 0;
-	}
-	return -ENODEV;
+	/* look for existing lport */
+	lport = fcoe_hostlist_lookup(netdev);
+	if (!lport)
+		return -ENODEV;
+	/* Remove the instance from fcoe's list */
+	fcoe_hostlist_remove(lport);
+	fcoe_if_destroy(lport);
+	return 0;
 }
 
 /**
@@ -1660,9 +1612,6 @@ static int fcoe_ethdrv_put(const struct net_device *netdev)
 static int fcoe_destroy(const char *buffer, struct kernel_param *kp)
 {
 	struct net_device *netdev;
-	struct fcoe_interface *fcoe;
-	struct fcoe_port *port;
-	struct fc_lport *lport;
 	int rc;
 
 	rtnl_lock();
@@ -1682,20 +1631,8 @@ static int fcoe_destroy(const char *buffer, struct kernel_param *kp)
 		rc = -ENODEV;
 		goto out_nodev;
 	}
-	/* look for existing lport */
-	lport = fcoe_hostlist_lookup(netdev);
-	if (!lport) {
-		rc = -ENODEV;
-		goto out_putdev;
-	}
-	/* Remove the instance from fcoe's list */
-	fcoe_hostlist_remove(lport);
-	port = lport_priv(lport);
-	fcoe = port->fcoe;
-	fcoe_if_destroy(lport);
-	fcoe_ethdrv_put(netdev);
-	rc = 0;
-out_putdev:
+
+	rc = __fcoe_destroy(netdev);
 	dev_put(netdev);
 out_nodev:
 	rtnl_unlock();
@@ -1733,12 +1670,12 @@ static int fcoe_create(const char *buffer, struct kernel_param *kp)
 		rc = -ENODEV;
 		goto out_nodev;
 	}
+
 	/* look for existing lport */
 	if (fcoe_hostlist_lookup(netdev)) {
 		rc = -EEXIST;
 		goto out_putdev;
 	}
-	fcoe_ethdrv_get(netdev);
 
 	fcoe = fcoe_interface_create(netdev);
 	if (!fcoe) {
@@ -1750,7 +1687,6 @@ static int fcoe_create(const char *buffer, struct kernel_param *kp)
 	if (IS_ERR(lport)) {
 		printk(KERN_ERR "fcoe: Failed to create interface (%s)\n",
 		       netdev->name);
-		fcoe_ethdrv_put(netdev);
 		rc = -EIO;
 		goto out_free;
 	}
@@ -1768,9 +1704,11 @@ static int fcoe_create(const char *buffer, struct kernel_param *kp)
 		fcoe_ctlr_link_up(&fcoe->ctlr);
 
 	rc = 0;
-	goto out_putdev;
-
 out_free:
+	/*
+	 * Release from init in fcoe_interface_create(), on success lport
+	 * should be holding a reference taken in fcoe_if_create().
+	 */
 	fcoe_interface_put(fcoe);
 out_putdev:
 	dev_put(netdev);
@@ -2014,7 +1952,7 @@ static void __exit fcoe_exit(void)
 
 	/* releases the associated fcoe hosts */
 	list_for_each_entry_safe(fcoe, tmp, &fcoe_hostlist, list)
-		fcoe_if_destroy(fcoe->ctlr.lp);
+		__fcoe_destroy(fcoe->real_dev);
 
 	unregister_hotcpu_notifier(&fcoe_cpu_notifier);
 




More information about the devel mailing list