[Open-FCoE] [RFC PATCH V3 11/13] fcoe: use RTNL mutex to serialize create/destroy and protect hostlist

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


Rather than rely on the hostlist_lock to be held while creating exchange
managers, serialize fcoe instance creation and destruction with a mutex.
This will allow the hostlist addition to be moved out of fcoe_if_create(),
which will simplify NPIV support.

RTNL mutex was used because it needs to be held in parts of both the create
and destroy path anyway, and because the netdevice notifier will be called
with RTNL held.  If a new mutex was added and fine-grained use of RTNL was
maintained, then destroying the interface when the netdevice was unregistered
would deadlock on RTNL unless it was deferred using a workqueue or similar.

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

 drivers/scsi/fcoe/fcoe.c |   68 ++++++++++++++++++++++++++++------------------
 1 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 49a9d5b..a4b8991 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -49,9 +49,9 @@ MODULE_AUTHOR("Open-FCoE.org");
 MODULE_DESCRIPTION("FCoE");
 MODULE_LICENSE("GPL v2");
 
-/* fcoe host list */
+/* fcoe host list
+ * must be accessed under rtnl_lock */
 LIST_HEAD(fcoe_hostlist);
-DEFINE_RWLOCK(fcoe_hostlist_lock);
 DEFINE_PER_CPU(struct fcoe_percpu_s, fcoe_percpu);
 
 /* Function Prototypes */
@@ -186,13 +186,11 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
 	 * or enter promiscuous mode if not capable of listening
 	 * for multiple unicast MACs.
 	 */
-	rtnl_lock();
 	memcpy(flogi_maddr, (u8[6]) FC_FCOE_FLOGI_MAC, ETH_ALEN);
 	dev_unicast_add(netdev, flogi_maddr);
 	if (fip->spma)
 		dev_unicast_add(netdev, fip->ctl_src_addr);
 	dev_mc_add(netdev, FIP_ALL_ENODE_MACS, ETH_ALEN, 0);
-	rtnl_unlock();
 
 	/*
 	 * setup the receive function from ethernet driver
@@ -259,7 +257,6 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
 	dev_remove_pack(&fcoe->fip_packet_type);
 
 	/* Delete secondary MAC addresses */
-	rtnl_lock();
 	memcpy(flogi_maddr, (u8[6]) FC_FCOE_FLOGI_MAC, ETH_ALEN);
 	dev_unicast_delete(netdev, flogi_maddr);
 	if (!is_zero_ether_addr(fip->data_src_addr))
@@ -267,7 +264,6 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
 	if (fip->spma)
 		dev_unicast_delete(netdev, fip->ctl_src_addr);
 	dev_mc_delete(netdev, FIP_ALL_ENODE_MACS, ETH_ALEN, 0);
-	rtnl_unlock();
 }
 
 /**
@@ -516,8 +512,6 @@ bool fcoe_oem_match(struct fc_frame *fp)
  * fcoe_em_config() - allocates em for this lport
  * @lp: the fcoe that em is to allocated for
  *
- * Called with write fcoe_hostlist_lock held.
- *
  * Returns : 0 on success
  */
 static inline int fcoe_em_config(struct fc_lport *lp)
@@ -739,11 +733,14 @@ static struct fc_lport *fcoe_if_create(struct fcoe_interface *fcoe,
 
 	/*
 	 * fcoe_em_alloc() and fcoe_hostlist_add() both
-	 * need to be atomic under fcoe_hostlist_lock
+	 * need to be atomic with respect to other changes to the hostlist
 	 * since fcoe_em_alloc() looks for an existing EM
 	 * instance on host list updated by fcoe_hostlist_add().
+	 *
+	 * This is OK because the entire create and destroy paths are called
+	 * with the RTNL mutex held.
 	 */
-	write_lock(&fcoe_hostlist_lock);
+
 	/* lport exch manager allocation */
 	rc = fcoe_em_config(lport);
 	if (rc) {
@@ -754,7 +751,6 @@ static struct fc_lport *fcoe_if_create(struct fcoe_interface *fcoe,
 
 	/* add to lports list */
 	fcoe_hostlist_add(lport);
-	write_unlock(&fcoe_hostlist_lock);
 
 	dev_hold(netdev);
 	fcoe_interface_get(fcoe);
@@ -795,6 +791,7 @@ static int __init fcoe_if_init(void)
 int __exit fcoe_if_exit(void)
 {
 	fc_release_transport(scsi_transport_fcoe_sw);
+	scsi_transport_fcoe_sw = NULL;
 	return 0;
 }
 
@@ -1524,14 +1521,14 @@ static int fcoe_device_notification(struct notifier_block *notifier,
 	u32 mfs;
 	int rc = NOTIFY_OK;
 
-	read_lock(&fcoe_hostlist_lock);
+	ASSERT_RTNL();
+
 	list_for_each_entry(fcoe, &fcoe_hostlist, list) {
 		if (fcoe->real_dev == real_dev) {
 			lp = fcoe->ctlr.lp;
 			break;
 		}
 	}
-	read_unlock(&fcoe_hostlist_lock);
 	if (lp == NULL) {
 		rc = NOTIFY_DONE;
 		goto out;
@@ -1674,6 +1671,18 @@ static int fcoe_destroy(const char *buffer, struct kernel_param *kp)
 	struct fc_lport *lport;
 	int rc;
 
+	rtnl_lock();
+
+	/*
+	 * Make sure the module has been initialized, and is not about to be
+	 * removed.  Module paramter sysfs files are writable before the
+	 * module_init function is called and after module_exit.
+	 */
+	if (!scsi_transport_fcoe_sw) {
+		rc = -ENODEV;
+		goto out_nodev;
+	}
+
 	netdev = fcoe_if_to_netdev(buffer);
 	if (!netdev) {
 		rc = -ENODEV;
@@ -1693,6 +1702,7 @@ static int fcoe_destroy(const char *buffer, struct kernel_param *kp)
 out_putdev:
 	dev_put(netdev);
 out_nodev:
+	rtnl_unlock();
 	return rc;
 }
 
@@ -1710,6 +1720,18 @@ static int fcoe_create(const char *buffer, struct kernel_param *kp)
 	struct fc_lport *lport;
 	struct net_device *netdev;
 
+	rtnl_lock();
+
+	/*
+	 * Make sure the module has been initialized, and is not about to be
+	 * removed.  Module paramter sysfs files are writable before the
+	 * module_init function is called and after module_exit.
+	 */
+	if (!scsi_transport_fcoe_sw) {
+		rc = -ENODEV;
+		goto out_nodev;
+	}
+
 	netdev = fcoe_if_to_netdev(buffer);
 	if (!netdev) {
 		rc = -ENODEV;
@@ -1746,14 +1768,15 @@ 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);
-	return 0;
+	rc = 0;
+	goto out_putdev;
 
 out_free:
 	fcoe_interface_put(fcoe);
 out_putdev:
 	dev_put(netdev);
 out_nodev:
+	rtnl_unlock();
 	return rc;
 }
 
@@ -1872,8 +1895,6 @@ int fcoe_reset(struct Scsi_Host *shost)
  * fcoe_hostlist_lookup_port() - find the corresponding lport by a given device
  * @dev: this is currently ptr to net_device
  *
- * Called with fcoe_hostlist_lock held.
- *
  * Returns: NULL or the located fcoe_port
  */
 static struct fcoe_interface *
@@ -1898,10 +1919,7 @@ struct fc_lport *fcoe_hostlist_lookup(const struct net_device *netdev)
 {
 	struct fcoe_interface *fcoe;
 
-	read_lock(&fcoe_hostlist_lock);
 	fcoe = fcoe_hostlist_lookup_port(netdev);
-	read_unlock(&fcoe_hostlist_lock);
-
 	return (fcoe) ? fcoe->ctlr.lp : NULL;
 }
 
@@ -1909,8 +1927,6 @@ struct fc_lport *fcoe_hostlist_lookup(const struct net_device *netdev)
  * fcoe_hostlist_add() - Add a lport to lports list
  * @lp: ptr to the fc_lport to be added
  *
- * Called with write fcoe_hostlist_lock held.
- *
  * Returns: 0 for success
  */
 int fcoe_hostlist_add(const struct fc_lport *lport)
@@ -1937,11 +1953,9 @@ 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;
 }
@@ -1957,9 +1971,6 @@ static int __init fcoe_init(void)
 	int rc = 0;
 	struct fcoe_percpu_s *p;
 
-	INIT_LIST_HEAD(&fcoe_hostlist);
-	rwlock_init(&fcoe_hostlist_lock);
-
 	for_each_possible_cpu(cpu) {
 		p = &per_cpu(fcoe_percpu, cpu);
 		skb_queue_head_init(&p->fcoe_rx_list);
@@ -2000,6 +2011,7 @@ static void __exit fcoe_exit(void)
 	struct fcoe_interface *fcoe, *tmp;
 
 	fcoe_dev_cleanup();
+	rtnl_lock();
 
 	/* releases the associated fcoe hosts */
 	list_for_each_entry_safe(fcoe, tmp, &fcoe_hostlist, list)
@@ -2012,5 +2024,7 @@ static void __exit fcoe_exit(void)
 
 	/* detach from scsi transport */
 	fcoe_if_exit();
+
+	rtnl_unlock();
 }
 module_exit(fcoe_exit);




More information about the devel mailing list