Commit 2e70e241 authored by Chris Leech's avatar Chris Leech Committed by James Bottomley

[SCSI] fcoe: Fix module ref count bug by adding NETDEV UNREGISTER handling

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: default avatarChris Leech <christopher.leech@intel.com>
Signed-off-by: default avatarRobert Love <robert.w.love@intel.com>
Signed-off-by: default avatarJames Bottomley <James.Bottomley@suse.de>
parent c863df33
...@@ -74,12 +74,13 @@ static int fcoe_link_ok(struct fc_lport *lp); ...@@ -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 struct fc_lport *fcoe_hostlist_lookup(const struct net_device *);
static int fcoe_hostlist_add(const struct fc_lport *); 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 void fcoe_check_wait_queue(struct fc_lport *, struct sk_buff *);
static int fcoe_device_notification(struct notifier_block *, ulong, void *); static int fcoe_device_notification(struct notifier_block *, ulong, void *);
static void fcoe_dev_setup(void); static void fcoe_dev_setup(void);
static void fcoe_dev_cleanup(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 */ /* notification function from net device */
static struct notifier_block fcoe_notifier = { static struct notifier_block fcoe_notifier = {
...@@ -149,6 +150,7 @@ static int fcoe_fip_recv(struct sk_buff *skb, struct net_device *dev, ...@@ -149,6 +150,7 @@ static int fcoe_fip_recv(struct sk_buff *skb, struct net_device *dev,
* @netdev : ptr to the associated netdevice struct * @netdev : ptr to the associated netdevice struct
* *
* Returns : 0 for success * Returns : 0 for success
* Locking: must be called with the RTNL mutex held
*/ */
static int fcoe_interface_setup(struct fcoe_interface *fcoe, static int fcoe_interface_setup(struct fcoe_interface *fcoe,
struct net_device *netdev) struct net_device *netdev)
...@@ -188,13 +190,11 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe, ...@@ -188,13 +190,11 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
* or enter promiscuous mode if not capable of listening * or enter promiscuous mode if not capable of listening
* for multiple unicast MACs. * for multiple unicast MACs.
*/ */
rtnl_lock();
memcpy(flogi_maddr, (u8[6]) FC_FCOE_FLOGI_MAC, ETH_ALEN); memcpy(flogi_maddr, (u8[6]) FC_FCOE_FLOGI_MAC, ETH_ALEN);
dev_unicast_add(netdev, flogi_maddr); dev_unicast_add(netdev, flogi_maddr);
if (fip->spma) if (fip->spma)
dev_unicast_add(netdev, fip->ctl_src_addr); dev_unicast_add(netdev, fip->ctl_src_addr);
dev_mc_add(netdev, FIP_ALL_ENODE_MACS, ETH_ALEN, 0); dev_mc_add(netdev, FIP_ALL_ENODE_MACS, ETH_ALEN, 0);
rtnl_unlock();
/* /*
* setup the receive function from ethernet driver * setup the receive function from ethernet driver
...@@ -215,6 +215,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe, ...@@ -215,6 +215,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_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_update_src_mac(struct fcoe_ctlr *fip, u8 *old, u8 *new);
static void fcoe_destroy_work(struct work_struct *work);
/** /**
* fcoe_interface_create() * fcoe_interface_create()
...@@ -232,6 +233,7 @@ static struct fcoe_interface *fcoe_interface_create(struct net_device *netdev) ...@@ -232,6 +233,7 @@ static struct fcoe_interface *fcoe_interface_create(struct net_device *netdev)
return NULL; return NULL;
} }
dev_hold(netdev);
kref_init(&fcoe->kref); kref_init(&fcoe->kref);
/* /*
...@@ -249,6 +251,8 @@ static struct fcoe_interface *fcoe_interface_create(struct net_device *netdev) ...@@ -249,6 +251,8 @@ static struct fcoe_interface *fcoe_interface_create(struct net_device *netdev)
/** /**
* fcoe_interface_cleanup() - clean up netdev configurations * fcoe_interface_cleanup() - clean up netdev configurations
* @fcoe: * @fcoe:
*
* Caller must be holding the RTNL mutex
*/ */
void fcoe_interface_cleanup(struct fcoe_interface *fcoe) void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
{ {
...@@ -266,11 +270,7 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe) ...@@ -266,11 +270,7 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
__dev_remove_pack(&fcoe->fip_packet_type); __dev_remove_pack(&fcoe->fip_packet_type);
synchronize_net(); synchronize_net();
/* tear-down the FCoE controller */
fcoe_ctlr_destroy(&fcoe->ctlr);
/* Delete secondary MAC addresses */ /* Delete secondary MAC addresses */
rtnl_lock();
memcpy(flogi_maddr, (u8[6]) FC_FCOE_FLOGI_MAC, ETH_ALEN); memcpy(flogi_maddr, (u8[6]) FC_FCOE_FLOGI_MAC, ETH_ALEN);
dev_unicast_delete(netdev, flogi_maddr); dev_unicast_delete(netdev, flogi_maddr);
if (!is_zero_ether_addr(fip->data_src_addr)) if (!is_zero_ether_addr(fip->data_src_addr))
...@@ -278,7 +278,6 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe) ...@@ -278,7 +278,6 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
if (fip->spma) if (fip->spma)
dev_unicast_delete(netdev, fip->ctl_src_addr); dev_unicast_delete(netdev, fip->ctl_src_addr);
dev_mc_delete(netdev, FIP_ALL_ENODE_MACS, ETH_ALEN, 0); dev_mc_delete(netdev, FIP_ALL_ENODE_MACS, ETH_ALEN, 0);
rtnl_unlock();
} }
/** /**
...@@ -288,10 +287,14 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe) ...@@ -288,10 +287,14 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
static void fcoe_interface_release(struct kref *kref) static void fcoe_interface_release(struct kref *kref)
{ {
struct fcoe_interface *fcoe; struct fcoe_interface *fcoe;
struct net_device *netdev;
fcoe = container_of(kref, struct fcoe_interface, kref); fcoe = container_of(kref, struct fcoe_interface, kref);
fcoe_interface_cleanup(fcoe); netdev = fcoe->netdev;
/* tear-down the FCoE controller */
fcoe_ctlr_destroy(&fcoe->ctlr);
kfree(fcoe); kfree(fcoe);
dev_put(netdev);
} }
/** /**
...@@ -642,8 +645,7 @@ static void fcoe_if_destroy(struct fc_lport *lport) ...@@ -642,8 +645,7 @@ static void fcoe_if_destroy(struct fc_lport *lport)
/* Free memory used by statistical counters */ /* Free memory used by statistical counters */
fc_lport_free_stats(lport); fc_lport_free_stats(lport);
/* Release the net_device and Scsi_Host */ /* Release the Scsi_Host */
dev_put(netdev);
scsi_host_put(lport->host); scsi_host_put(lport->host);
} }
...@@ -718,7 +720,9 @@ static struct fc_lport *fcoe_if_create(struct fcoe_interface *fcoe, ...@@ -718,7 +720,9 @@ static struct fc_lport *fcoe_if_create(struct fcoe_interface *fcoe,
} }
lport = shost_priv(shost); lport = shost_priv(shost);
port = lport_priv(lport); port = lport_priv(lport);
port->lport = lport;
port->fcoe = fcoe; port->fcoe = fcoe;
INIT_WORK(&port->destroy_work, fcoe_destroy_work);
/* configure fc_lport, e.g., em */ /* configure fc_lport, e.g., em */
rc = fcoe_lport_config(lport); rc = fcoe_lport_config(lport);
...@@ -769,7 +773,6 @@ static struct fc_lport *fcoe_if_create(struct fcoe_interface *fcoe, ...@@ -769,7 +773,6 @@ static struct fc_lport *fcoe_if_create(struct fcoe_interface *fcoe,
goto out_lp_destroy; goto out_lp_destroy;
} }
dev_hold(netdev);
fcoe_interface_get(fcoe); fcoe_interface_get(fcoe);
return lport; return lport;
...@@ -1530,19 +1533,19 @@ static int fcoe_device_notification(struct notifier_block *notifier, ...@@ -1530,19 +1533,19 @@ static int fcoe_device_notification(struct notifier_block *notifier,
struct fc_lport *lp = NULL; struct fc_lport *lp = NULL;
struct net_device *netdev = ptr; struct net_device *netdev = ptr;
struct fcoe_interface *fcoe; struct fcoe_interface *fcoe;
struct fcoe_port *port;
struct fcoe_dev_stats *stats; struct fcoe_dev_stats *stats;
u32 link_possible = 1; u32 link_possible = 1;
u32 mfs; u32 mfs;
int rc = NOTIFY_OK; int rc = NOTIFY_OK;
read_lock(&fcoe_hostlist_lock); write_lock(&fcoe_hostlist_lock);
list_for_each_entry(fcoe, &fcoe_hostlist, list) { list_for_each_entry(fcoe, &fcoe_hostlist, list) {
if (fcoe->netdev == netdev) { if (fcoe->netdev == netdev) {
lp = fcoe->ctlr.lp; lp = fcoe->ctlr.lp;
break; break;
} }
} }
read_unlock(&fcoe_hostlist_lock);
if (lp == NULL) { if (lp == NULL) {
rc = NOTIFY_DONE; rc = NOTIFY_DONE;
goto out; goto out;
...@@ -1564,6 +1567,13 @@ static int fcoe_device_notification(struct notifier_block *notifier, ...@@ -1564,6 +1567,13 @@ static int fcoe_device_notification(struct notifier_block *notifier,
break; break;
case NETDEV_REGISTER: case NETDEV_REGISTER:
break; break;
case NETDEV_UNREGISTER:
list_del(&fcoe->list);
port = lport_priv(fcoe->ctlr.lp);
fcoe_interface_cleanup(fcoe);
schedule_work(&port->destroy_work);
goto out;
break;
default: default:
FCOE_NETDEV_DBG(netdev, "Unknown event %ld " FCOE_NETDEV_DBG(netdev, "Unknown event %ld "
"from netdev netlink\n", event); "from netdev netlink\n", event);
...@@ -1576,6 +1586,7 @@ static int fcoe_device_notification(struct notifier_block *notifier, ...@@ -1576,6 +1586,7 @@ static int fcoe_device_notification(struct notifier_block *notifier,
fcoe_clean_pending_queue(lp); fcoe_clean_pending_queue(lp);
} }
out: out:
write_unlock(&fcoe_hostlist_lock);
return rc; return rc;
} }
...@@ -1600,75 +1611,6 @@ static struct net_device *fcoe_if_to_netdev(const char *buffer) ...@@ -1600,75 +1611,6 @@ static struct net_device *fcoe_if_to_netdev(const char *buffer)
return NULL; 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)
{
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 * fcoe_destroy() - handles the destroy from sysfs
* @buffer: expected to be an eth if name * @buffer: expected to be an eth if name
...@@ -1678,10 +1620,8 @@ static int fcoe_ethdrv_put(const struct net_device *netdev) ...@@ -1678,10 +1620,8 @@ static int fcoe_ethdrv_put(const struct net_device *netdev)
*/ */
static int fcoe_destroy(const char *buffer, struct kernel_param *kp) static int fcoe_destroy(const char *buffer, struct kernel_param *kp)
{ {
struct net_device *netdev;
struct fcoe_interface *fcoe; struct fcoe_interface *fcoe;
struct fcoe_port *port; struct net_device *netdev;
struct fc_lport *lport;
int rc; int rc;
mutex_lock(&fcoe_config_mutex); mutex_lock(&fcoe_config_mutex);
...@@ -1702,19 +1642,20 @@ static int fcoe_destroy(const char *buffer, struct kernel_param *kp) ...@@ -1702,19 +1642,20 @@ static int fcoe_destroy(const char *buffer, struct kernel_param *kp)
rc = -ENODEV; rc = -ENODEV;
goto out_nodev; goto out_nodev;
} }
/* look for existing lport */
lport = fcoe_hostlist_lookup(netdev); write_lock(&fcoe_hostlist_lock);
if (!lport) { fcoe = fcoe_hostlist_lookup_port(netdev);
if (!fcoe) {
write_unlock(&fcoe_hostlist_lock);
rc = -ENODEV; rc = -ENODEV;
goto out_putdev; goto out_putdev;
} }
/* Remove the instance from fcoe's list */ list_del(&fcoe->list);
fcoe_hostlist_remove(lport); write_unlock(&fcoe_hostlist_lock);
port = lport_priv(lport); rtnl_lock();
fcoe = port->fcoe; fcoe_interface_cleanup(fcoe);
fcoe_if_destroy(lport); rtnl_unlock();
fcoe_ethdrv_put(netdev); fcoe_if_destroy(fcoe->ctlr.lp);
rc = 0;
out_putdev: out_putdev:
dev_put(netdev); dev_put(netdev);
out_nodev: out_nodev:
...@@ -1722,6 +1663,16 @@ static int fcoe_destroy(const char *buffer, struct kernel_param *kp) ...@@ -1722,6 +1663,16 @@ static int fcoe_destroy(const char *buffer, struct kernel_param *kp)
return rc; 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 * fcoe_create() - Handles the create call from sysfs
* @buffer: expected to be an eth if name * @buffer: expected to be an eth if name
...@@ -1749,17 +1700,18 @@ static int fcoe_create(const char *buffer, struct kernel_param *kp) ...@@ -1749,17 +1700,18 @@ static int fcoe_create(const char *buffer, struct kernel_param *kp)
} }
#endif #endif
rtnl_lock();
netdev = fcoe_if_to_netdev(buffer); netdev = fcoe_if_to_netdev(buffer);
if (!netdev) { if (!netdev) {
rc = -ENODEV; rc = -ENODEV;
goto out_nodev; goto out_nodev;
} }
/* look for existing lport */ /* look for existing lport */
if (fcoe_hostlist_lookup(netdev)) { if (fcoe_hostlist_lookup(netdev)) {
rc = -EEXIST; rc = -EEXIST;
goto out_putdev; goto out_putdev;
} }
fcoe_ethdrv_get(netdev);
fcoe = fcoe_interface_create(netdev); fcoe = fcoe_interface_create(netdev);
if (!fcoe) { if (!fcoe) {
...@@ -1771,8 +1723,8 @@ static int fcoe_create(const char *buffer, struct kernel_param *kp) ...@@ -1771,8 +1723,8 @@ static int fcoe_create(const char *buffer, struct kernel_param *kp)
if (IS_ERR(lport)) { if (IS_ERR(lport)) {
printk(KERN_ERR "fcoe: Failed to create interface (%s)\n", printk(KERN_ERR "fcoe: Failed to create interface (%s)\n",
netdev->name); netdev->name);
fcoe_ethdrv_put(netdev);
rc = -EIO; rc = -EIO;
fcoe_interface_cleanup(fcoe);
goto out_free; goto out_free;
} }
...@@ -1798,6 +1750,7 @@ static int fcoe_create(const char *buffer, struct kernel_param *kp) ...@@ -1798,6 +1750,7 @@ static int fcoe_create(const char *buffer, struct kernel_param *kp)
out_putdev: out_putdev:
dev_put(netdev); dev_put(netdev);
out_nodev: out_nodev:
rtnl_unlock();
mutex_unlock(&fcoe_config_mutex); mutex_unlock(&fcoe_config_mutex);
return rc; return rc;
} }
...@@ -1972,25 +1925,6 @@ int fcoe_hostlist_add(const struct fc_lport *lport) ...@@ -1972,25 +1925,6 @@ int fcoe_hostlist_add(const struct fc_lport *lport)
return 0; return 0;
} }
/**
* 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 * fcoe_init() - fcoe module loading initialization
* *
...@@ -2046,6 +1980,7 @@ static void __exit fcoe_exit(void) ...@@ -2046,6 +1980,7 @@ static void __exit fcoe_exit(void)
unsigned int cpu; unsigned int cpu;
struct fcoe_interface *fcoe, *tmp; struct fcoe_interface *fcoe, *tmp;
LIST_HEAD(local_list); LIST_HEAD(local_list);
struct fcoe_port *port;
mutex_lock(&fcoe_config_mutex); mutex_lock(&fcoe_config_mutex);
...@@ -2058,7 +1993,11 @@ static void __exit fcoe_exit(void) ...@@ -2058,7 +1993,11 @@ static void __exit fcoe_exit(void)
list_for_each_entry_safe(fcoe, tmp, &local_list, list) { list_for_each_entry_safe(fcoe, tmp, &local_list, list) {
list_del(&fcoe->list); list_del(&fcoe->list);
fcoe_if_destroy(fcoe->ctlr.lp); port = lport_priv(fcoe->ctlr.lp);
rtnl_lock();
fcoe_interface_cleanup(fcoe);
rtnl_unlock();
schedule_work(&port->destroy_work);
} }
unregister_hotcpu_notifier(&fcoe_cpu_notifier); unregister_hotcpu_notifier(&fcoe_cpu_notifier);
...@@ -2066,9 +2005,14 @@ static void __exit fcoe_exit(void) ...@@ -2066,9 +2005,14 @@ static void __exit fcoe_exit(void)
for_each_online_cpu(cpu) for_each_online_cpu(cpu)
fcoe_percpu_thread_destroy(cpu); fcoe_percpu_thread_destroy(cpu);
/* detach from scsi transport */
fcoe_if_exit();
mutex_unlock(&fcoe_config_mutex); 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); module_exit(fcoe_exit);
...@@ -93,9 +93,11 @@ struct fcoe_interface { ...@@ -93,9 +93,11 @@ struct fcoe_interface {
*/ */
struct fcoe_port { struct fcoe_port {
struct fcoe_interface *fcoe; struct fcoe_interface *fcoe;
struct fc_lport *lport;
struct sk_buff_head fcoe_pending_queue; struct sk_buff_head fcoe_pending_queue;
u8 fcoe_pending_queue_active; u8 fcoe_pending_queue_active;
struct timer_list timer; /* queue timer */ 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) #define fcoe_from_ctlr(fip) container_of(fip, struct fcoe_interface, ctlr)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment