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.

On an NETDEV_UNREGISTER event, destroying the FCoE instance is deferred to a
workqueue context to avoid RTNL deadlocks.

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 <[email protected]>
---

 drivers/scsi/fcoe/fcoe.c |  175 +++++++++++++++-------------------------------
 drivers/scsi/fcoe/fcoe.h |    2 +
 2 files changed, 58 insertions(+), 119 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 2330403..290baaa 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -74,12 +74,13 @@ static int fcoe_link_ok(struct fc_lport *lp);
 
 static struct fc_lport *fcoe_hostlist_lookup(const struct net_device *);
 static int fcoe_hostlist_add(const struct fc_lport *);
-static int fcoe_hostlist_remove(const struct fc_lport *);
 
 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 struct fcoe_interface *
+       fcoe_hostlist_lookup_port(const struct net_device *dev);
 
 /* notification function from net device */
 static struct notifier_block fcoe_notifier = {
@@ -216,6 +217,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
 
 static void fcoe_fip_send(struct fcoe_ctlr *fip, struct sk_buff *skb);
 static void fcoe_update_src_mac(struct fcoe_ctlr *fip, u8 *old, u8 *new);
+static void fcoe_destroy_work(struct work_struct *work);
 
 /**
  * fcoe_interface_create()
@@ -233,6 +235,7 @@ static struct fcoe_interface *fcoe_interface_create(struct 
net_device *netdev)
                return NULL;
        }
 
+       dev_hold(netdev);
        kref_init(&fcoe->kref);
 
        /*
@@ -289,10 +292,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->netdev;
        fcoe_interface_cleanup(fcoe);
        kfree(fcoe);
+       dev_put(netdev);
 }
 
 /**
@@ -643,8 +649,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(netdev);
+       /* Release the Scsi_Host */
        scsi_host_put(lport->host);
 }
 
@@ -719,7 +724,9 @@ static struct fc_lport *fcoe_if_create(struct 
fcoe_interface *fcoe,
        }
        lport = shost_priv(shost);
        port = lport_priv(lport);
+       port->lport = lport;
        port->fcoe = fcoe;
+       INIT_WORK(&port->destroy_work, fcoe_destroy_work);
 
        /* configure fc_lport, e.g., em */
        rc = fcoe_lport_config(lport);
@@ -770,7 +777,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;
 
@@ -1532,19 +1538,19 @@ static int fcoe_device_notification(struct 
notifier_block *notifier,
        struct fc_lport *lp = NULL;
        struct net_device *netdev = ptr;
        struct fcoe_interface *fcoe;
+       struct fcoe_port *port;
        struct fcoe_dev_stats *stats;
        u32 link_possible = 1;
        u32 mfs;
        int rc = NOTIFY_OK;
 
-       read_lock(&fcoe_hostlist_lock);
+       write_lock(&fcoe_hostlist_lock);
        list_for_each_entry(fcoe, &fcoe_hostlist, list) {
                if (fcoe->netdev == netdev) {
                        lp = fcoe->ctlr.lp;
                        break;
                }
        }
-       read_unlock(&fcoe_hostlist_lock);
        if (lp == NULL) {
                rc = NOTIFY_DONE;
                goto out;
@@ -1566,6 +1572,12 @@ static int fcoe_device_notification(struct 
notifier_block *notifier,
                break;
        case NETDEV_REGISTER:
                break;
+       case NETDEV_UNREGISTER:
+               list_del(&fcoe->list);
+               port = lport_priv(fcoe->ctlr.lp);
+               schedule_work(&port->destroy_work);
+               goto out;
+               break;
        default:
                FCOE_NETDEV_DBG(netdev, "Unknown event %ld "
                                "from netdev netlink\n", event);
@@ -1578,6 +1590,7 @@ static int fcoe_device_notification(struct notifier_block 
*notifier,
                fcoe_clean_pending_queue(lp);
        }
 out:
+       write_unlock(&fcoe_hostlist_lock);
        return rc;
 }
 
@@ -1603,75 +1616,6 @@ static struct net_device *fcoe_if_to_netdev(const char 
*buffer)
 }
 
 /**
- * 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)
-{
-       struct module *owner;
-
-       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;
-}
-
-/**
  * fcoe_destroy() - handles the destroy from sysfs
  * @buffer: expected to be an eth if name
  * @kp: associated kernel param
@@ -1680,10 +1624,8 @@ 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;
+       struct net_device *netdev;
        int rc;
 
        mutex_lock(&fcoe_config_mutex);
@@ -1704,26 +1646,33 @@ 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) {
+
+       write_lock(&fcoe_hostlist_lock);
+       fcoe = fcoe_hostlist_lookup_port(netdev);
+       if (!fcoe) {
+               write_unlock(&fcoe_hostlist_lock);
                rc = -ENODEV;
-               goto out_putdev;
+               goto out_nodev;
        }
-       /* 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:
+       list_del(&fcoe->list);
+       write_unlock(&fcoe_hostlist_lock);
+       fcoe_if_destroy(fcoe->ctlr.lp);
        dev_put(netdev);
 out_nodev:
        mutex_unlock(&fcoe_config_mutex);
        return rc;
 }
 
+static void fcoe_destroy_work(struct work_struct *work)
+{
+       struct fcoe_port *port;
+
+       port = container_of(work, struct fcoe_port, destroy_work);
+       mutex_lock(&fcoe_config_mutex);
+       fcoe_if_destroy(port->lport);
+       mutex_unlock(&fcoe_config_mutex);
+}
+
 /**
  * fcoe_create() - Handles the create call from sysfs
  * @buffer: expected to be an eth if name
@@ -1756,12 +1705,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) {
@@ -1773,7 +1722,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;
        }
@@ -1790,11 +1738,12 @@ static int fcoe_create(const char *buffer, struct 
kernel_param *kp)
        if (!fcoe_link_ok(lport))
                fcoe_ctlr_link_up(&fcoe->ctlr);
 
-       dev_put(netdev);
-       mutex_unlock(&fcoe_config_mutex);
-       return 0;
-
+       rc = 0;
 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);
@@ -1974,25 +1923,6 @@ int fcoe_hostlist_add(const struct fc_lport *lport)
 }
 
 /**
- * fcoe_hostlist_remove() - remove a lport from lports list
- * @lp: ptr to the fc_lport to be removed
- *
- * Returns: 0 for success
- */
-int fcoe_hostlist_remove(const struct fc_lport *lport)
-{
-       struct fcoe_interface *fcoe;
-
-       write_lock_bh(&fcoe_hostlist_lock);
-       fcoe = fcoe_hostlist_lookup_port(fcoe_netdev(lport));
-       BUG_ON(!fcoe);
-       list_del(&fcoe->list);
-       write_unlock_bh(&fcoe_hostlist_lock);
-
-       return 0;
-}
-
-/**
  * fcoe_init() - fcoe module loading initialization
  *
  * Returns 0 on success, negative on failure
@@ -2046,6 +1976,7 @@ static void __exit fcoe_exit(void)
 {
        unsigned int cpu;
        struct fcoe_interface *fcoe, *tmp;
+       struct fcoe_port *port;
 
        mutex_lock(&fcoe_config_mutex);
 
@@ -2055,7 +1986,8 @@ static void __exit fcoe_exit(void)
        write_lock_bh(&fcoe_hostlist_lock);
        list_for_each_entry_safe(fcoe, tmp, &fcoe_hostlist, list) {
                list_del(&fcoe->list);
-               fcoe_if_destroy(fcoe->ctlr.lp);
+               port = lport_priv(fcoe->ctlr.lp);
+               schedule_work(&port->destroy_work);
        }
        write_unlock_bh(&fcoe_hostlist_lock);
 
@@ -2064,9 +1996,14 @@ static void __exit fcoe_exit(void)
        for_each_online_cpu(cpu)
                fcoe_percpu_thread_destroy(cpu);
 
-       /* detach from scsi transport */
-       fcoe_if_exit();
-
        mutex_unlock(&fcoe_config_mutex);
+
+       /* flush any asyncronous interface destroys,
+        * this should happen after the netdev notifier is unregistered */
+       flush_scheduled_work();
+
+       /* detach from scsi transport
+        * must happen after all destroys are done, therefor after the flush */
+       fcoe_if_exit();
 }
 module_exit(fcoe_exit);
diff --git a/drivers/scsi/fcoe/fcoe.h b/drivers/scsi/fcoe/fcoe.h
index b9c9c7d..976321b 100644
--- a/drivers/scsi/fcoe/fcoe.h
+++ b/drivers/scsi/fcoe/fcoe.h
@@ -93,9 +93,11 @@ struct fcoe_interface {
  */
 struct fcoe_port {
        struct fcoe_interface *fcoe;
+       struct fc_lport *lport;
        struct sk_buff_head fcoe_pending_queue;
        u8      fcoe_pending_queue_active;
        struct timer_list timer;                /* queue timer */
+       struct work_struct destroy_work;        /* to prevent rtnl deadlocks */
 };
 
 #define fcoe_from_ctlr(fip) container_of(fip, struct fcoe_interface, ctlr)

_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel

Reply via email to