* [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).