netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3 v2] cnic fixes.
@ 2014-06-03  6:08 Michael Chan
  2014-06-03  6:08 ` [PATCH 1/3 v2] cnic: Don't take rcu_read_lock in cnic_rcv_netevent() Michael Chan
  2014-06-03  6:17 ` [PATCH 0/3 v2] cnic fixes David Miller
  0 siblings, 2 replies; 5+ messages in thread
From: Michael Chan @ 2014-06-03  6:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, nhorman, Michael Chan

Fix 2 sleeping function from invalid context bugs and 1 missing iscsi netlink
message bug.

v2: Fixed typo in rcu_dereference_protected() and tested with CONFIG_PROVE_RCU

Michael Chan (3):
  cnic: Don't take rcu_read_lock in cnic_rcv_netevent()
  cnic: Don't take cnic_dev_lock in cnic_alloc_uio_rings()
  cnic: Fix missing ISCSI_KEVENT_IF_DOWN message

 drivers/net/ethernet/broadcom/cnic.c |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3 v2] cnic: Don't take rcu_read_lock in cnic_rcv_netevent()
  2014-06-03  6:08 [PATCH 0/3 v2] cnic fixes Michael Chan
@ 2014-06-03  6:08 ` Michael Chan
  2014-06-03  6:08   ` [PATCH 2/3 v2] cnic: Don't take cnic_dev_lock in cnic_alloc_uio_rings() Michael Chan
  2014-06-03  6:17 ` [PATCH 0/3 v2] cnic fixes David Miller
  1 sibling, 1 reply; 5+ messages in thread
From: Michael Chan @ 2014-06-03  6:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, nhorman, Michael Chan

Because the called function, such as bnx2fc_indicate_netevent(), can sleep,
we cannot take rcu_lock().  To prevent the rcu protected ulp_ops from going
away, we use the cnic_lock mutex and set the ULP_F_CALL_PENDING flag.
The code already waits for ULP_F_CALL_PENDING flag to clear in
cnic_unregister_device().

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/cnic.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/cnic.c b/drivers/net/ethernet/broadcom/cnic.c
index 09f3fef..a0aae72 100644
--- a/drivers/net/ethernet/broadcom/cnic.c
+++ b/drivers/net/ethernet/broadcom/cnic.c
@@ -5624,20 +5624,27 @@ static void cnic_rcv_netevent(struct cnic_local *cp, unsigned long event,
 {
 	int if_type;
 
-	rcu_read_lock();
 	for (if_type = 0; if_type < MAX_CNIC_ULP_TYPE; if_type++) {
 		struct cnic_ulp_ops *ulp_ops;
 		void *ctx;
 
-		ulp_ops = rcu_dereference(cp->ulp_ops[if_type]);
-		if (!ulp_ops || !ulp_ops->indicate_netevent)
+		mutex_lock(&cnic_lock);
+		ulp_ops = rcu_dereference_protected(cp->ulp_ops[if_type],
+						lockdep_is_held(&cnic_lock));
+		if (!ulp_ops || !ulp_ops->indicate_netevent) {
+			mutex_unlock(&cnic_lock);
 			continue;
+		}
 
 		ctx = cp->ulp_handle[if_type];
 
+		set_bit(ULP_F_CALL_PENDING, &cp->ulp_flags[if_type]);
+		mutex_unlock(&cnic_lock);
+
 		ulp_ops->indicate_netevent(ctx, event, vlan_id);
+
+		clear_bit(ULP_F_CALL_PENDING, &cp->ulp_flags[if_type]);
 	}
-	rcu_read_unlock();
 }
 
 /* netdev event handler */
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3 v2] cnic: Don't take cnic_dev_lock in cnic_alloc_uio_rings()
  2014-06-03  6:08 ` [PATCH 1/3 v2] cnic: Don't take rcu_read_lock in cnic_rcv_netevent() Michael Chan
@ 2014-06-03  6:08   ` Michael Chan
  2014-06-03  6:08     ` [PATCH 3/3 v2] cnic: Fix missing ISCSI_KEVENT_IF_DOWN message Michael Chan
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Chan @ 2014-06-03  6:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, nhorman, Michael Chan

We are allocating memory with GFP_KERNEL under spinlock.  Since this is
the only call manipulating the cnic_udev_list and it is always under
rtnl_lock, cnic_dev_lock can be safely removed.

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/cnic.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/cnic.c b/drivers/net/ethernet/broadcom/cnic.c
index a0aae72..8da1060 100644
--- a/drivers/net/ethernet/broadcom/cnic.c
+++ b/drivers/net/ethernet/broadcom/cnic.c
@@ -1039,21 +1039,17 @@ static int cnic_alloc_uio_rings(struct cnic_dev *dev, int pages)
 	struct cnic_local *cp = dev->cnic_priv;
 	struct cnic_uio_dev *udev;
 
-	read_lock(&cnic_dev_lock);
 	list_for_each_entry(udev, &cnic_udev_list, list) {
 		if (udev->pdev == dev->pcidev) {
 			udev->dev = dev;
 			if (__cnic_alloc_uio_rings(udev, pages)) {
 				udev->dev = NULL;
-				read_unlock(&cnic_dev_lock);
 				return -ENOMEM;
 			}
 			cp->udev = udev;
-			read_unlock(&cnic_dev_lock);
 			return 0;
 		}
 	}
-	read_unlock(&cnic_dev_lock);
 
 	udev = kzalloc(sizeof(struct cnic_uio_dev), GFP_ATOMIC);
 	if (!udev)
@@ -1067,9 +1063,7 @@ static int cnic_alloc_uio_rings(struct cnic_dev *dev, int pages)
 	if (__cnic_alloc_uio_rings(udev, pages))
 		goto err_udev;
 
-	write_lock(&cnic_dev_lock);
 	list_add(&udev->list, &cnic_udev_list);
-	write_unlock(&cnic_dev_lock);
 
 	pci_dev_get(udev->pdev);
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3 v2] cnic: Fix missing ISCSI_KEVENT_IF_DOWN message
  2014-06-03  6:08   ` [PATCH 2/3 v2] cnic: Don't take cnic_dev_lock in cnic_alloc_uio_rings() Michael Chan
@ 2014-06-03  6:08     ` Michael Chan
  0 siblings, 0 replies; 5+ messages in thread
From: Michael Chan @ 2014-06-03  6:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, nhorman, Michael Chan

The iSCSI netlink message needs to be sent before the ulp_ops is cleared
as it is sent through a function pointer in the ulp_ops.  This bug
causes iscsid to not get the message when the bnx2i driver is unloaded.

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/cnic.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/cnic.c b/drivers/net/ethernet/broadcom/cnic.c
index 8da1060..a4b25bc 100644
--- a/drivers/net/ethernet/broadcom/cnic.c
+++ b/drivers/net/ethernet/broadcom/cnic.c
@@ -608,6 +608,10 @@ static int cnic_unregister_device(struct cnic_dev *dev, int ulp_type)
 		pr_err("%s: Bad type %d\n", __func__, ulp_type);
 		return -EINVAL;
 	}
+
+	if (ulp_type == CNIC_ULP_ISCSI)
+		cnic_send_nlmsg(cp, ISCSI_KEVENT_IF_DOWN, NULL);
+
 	mutex_lock(&cnic_lock);
 	if (rcu_dereference(cp->ulp_ops[ulp_type])) {
 		RCU_INIT_POINTER(cp->ulp_ops[ulp_type], NULL);
@@ -620,9 +624,7 @@ static int cnic_unregister_device(struct cnic_dev *dev, int ulp_type)
 	}
 	mutex_unlock(&cnic_lock);
 
-	if (ulp_type == CNIC_ULP_ISCSI)
-		cnic_send_nlmsg(cp, ISCSI_KEVENT_IF_DOWN, NULL);
-	else if (ulp_type == CNIC_ULP_FCOE)
+	if (ulp_type == CNIC_ULP_FCOE)
 		dev->fcoe_cap = NULL;
 
 	synchronize_rcu();
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3 v2] cnic fixes.
  2014-06-03  6:08 [PATCH 0/3 v2] cnic fixes Michael Chan
  2014-06-03  6:08 ` [PATCH 1/3 v2] cnic: Don't take rcu_read_lock in cnic_rcv_netevent() Michael Chan
@ 2014-06-03  6:17 ` David Miller
  1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2014-06-03  6:17 UTC (permalink / raw)
  To: mchan; +Cc: netdev, nhorman

From: Michael Chan <mchan@broadcom.com>
Date: Mon, 2 Jun 2014 23:08:45 -0700

> Fix 2 sleeping function from invalid context bugs and 1 missing iscsi netlink
> message bug.
> 
> v2: Fixed typo in rcu_dereference_protected() and tested with CONFIG_PROVE_RCU

Looks a lot better, series applied, thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-06-03  6:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-03  6:08 [PATCH 0/3 v2] cnic fixes Michael Chan
2014-06-03  6:08 ` [PATCH 1/3 v2] cnic: Don't take rcu_read_lock in cnic_rcv_netevent() Michael Chan
2014-06-03  6:08   ` [PATCH 2/3 v2] cnic: Don't take cnic_dev_lock in cnic_alloc_uio_rings() Michael Chan
2014-06-03  6:08     ` [PATCH 3/3 v2] cnic: Fix missing ISCSI_KEVENT_IF_DOWN message Michael Chan
2014-06-03  6:17 ` [PATCH 0/3 v2] cnic fixes David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).