From: Neil Horman <nhorman@tuxdriver.com>
To: netdev@vger.kernel.org
Cc: Neil Horman <nhorman@tuxdriver.com>,
Divy Le Ray <divy@chelsio.com>, Steve Wise <swise@chelsio.com>,
"David S. Miller" <davem@davemloft.net>
Subject: [PATCH] cxgb3i: ref count cdev access to prevent modification while in use
Date: Mon, 25 Jul 2011 15:56:57 -0400 [thread overview]
Message-ID: <1311623817-6417-1-git-send-email-nhorman@tuxdriver.com> (raw)
This oops was reported recently:
d:mon> e
cpu 0xd: Vector: 300 (Data Access) at [c0000000fd4c7120]
pc: d00000000076f194: .t3_l2t_get+0x44/0x524 [cxgb3]
lr: d000000000b02108: .init_act_open+0x150/0x3d4 [cxgb3i]
sp: c0000000fd4c73a0
msr: 8000000000009032
dar: 0
dsisr: 40000000
current = 0xc0000000fd640d40
paca = 0xc00000000054ff80
pid = 5085, comm = iscsid
d:mon> t
[c0000000fd4c7450] d000000000b02108 .init_act_open+0x150/0x3d4 [cxgb3i]
[c0000000fd4c7500] d000000000e45378 .cxgbi_ep_connect+0x784/0x8e8 [libcxgbi]
[c0000000fd4c7650] d000000000db33f0 .iscsi_if_rx+0x71c/0xb18
[scsi_transport_iscsi2]
[c0000000fd4c7740] c000000000370c9c .netlink_data_ready+0x40/0xa4
[c0000000fd4c77c0] c00000000036f010 .netlink_sendskb+0x4c/0x9c
[c0000000fd4c7850] c000000000370c18 .netlink_sendmsg+0x358/0x39c
[c0000000fd4c7950] c00000000033be24 .sock_sendmsg+0x114/0x1b8
[c0000000fd4c7b50] c00000000033d208 .sys_sendmsg+0x218/0x2ac
[c0000000fd4c7d70] c00000000033f55c .sys_socketcall+0x228/0x27c
[c0000000fd4c7e30] c0000000000086a4 syscall_exit+0x0/0x40
--- Exception: c01 (System Call) at 00000080da560cfc
The root cause was an EEH error, which sent us down the offload_close path in
the cxgb3 driver, which in turn sets cdev->lldev to NULL, without regard for
upper layer driver (like the cxgbi drivers) which might have execution contexts
in the middle of its use. The result is the oops above, when t3_l2t_get attempts
to dereference cdev->lldev right after the EEH error handler sets it to NULL.
The fix is to reference count the cdev structure. When an EEH error occurs, the
shutdown path:
t3_adapter_error->offload_close->cxgb3i_remove_clients->cxgb3i_dev_close
will now block until such time as the cdev pointer has a use count of zero.
This coupled with the fact that lookups will now skip finding any registered
cdev's in cxgbi_device_find_by_[lldev|netdev] with the CXGBI_FLAG_ADAPTER_RESET
bit set ensures that on an EEH, the setting of lldev to NULL in offload_close
will only happen after there are no longer any active users of the data
structure.
This has been tested by the reporter and shown to fix the reproted oops
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Divy Le Ray <divy@chelsio.com>
CC: Steve Wise <swise@chelsio.com>
CC: "David S. Miller" <davem@davemloft.net>
---
drivers/scsi/cxgbi/cxgb3i/cxgb3i.c | 8 +++++++-
drivers/scsi/cxgbi/libcxgbi.c | 9 +++++++++
drivers/scsi/cxgbi/libcxgbi.h | 3 +++
3 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
index abc7b12..7d752cd 100644
--- a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
+++ b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
@@ -1301,9 +1301,13 @@ static void cxgb3i_dev_close(struct t3cdev *t3dev)
if (!cdev || cdev->flags & CXGBI_FLAG_ADAPTER_RESET) {
pr_info("0x%p close, f 0x%x.\n", cdev, cdev ? cdev->flags : 0);
+ if (cdev)
+ cdev_put(cdev);
+ while (cdev && atomic_read(&cdev->use_count) != 0)
+ msleep(1);
return;
}
-
+ cdev_put(cdev);
cxgbi_device_unregister(cdev);
}
@@ -1318,6 +1322,7 @@ static void cxgb3i_dev_open(struct t3cdev *t3dev)
int i, err;
if (cdev) {
+ cdev_put(cdev);
pr_info("0x%p, updating.\n", cdev);
return;
}
@@ -1390,6 +1395,7 @@ static void cxgb3i_dev_event_handler(struct t3cdev *t3dev, u32 event, u32 port)
cdev->flags &= ~CXGBI_FLAG_ADAPTER_RESET;
break;
}
+ cdev_put(cdev);
}
/**
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index 77ac217..eb5625d 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -181,6 +181,9 @@ struct cxgbi_device *cxgbi_device_find_by_lldev(void *lldev)
mutex_lock(&cdev_mutex);
list_for_each_entry_safe(cdev, tmp, &cdev_list, list_head) {
if (cdev->lldev == lldev) {
+ if (cdev->flags & CXGBI_FLAG_ADAPTER_RESET)
+ continue;
+ cdev_hold(cdev);
mutex_unlock(&cdev_mutex);
return cdev;
}
@@ -210,7 +213,10 @@ static struct cxgbi_device *cxgbi_device_find_by_netdev(struct net_device *ndev,
list_for_each_entry_safe(cdev, tmp, &cdev_list, list_head) {
for (i = 0; i < cdev->nports; i++) {
if (ndev == cdev->ports[i]) {
+ if (cdev->flags & CXGBI_FLAG_ADAPTER_RESET)
+ continue;
cdev->hbas[i]->vdev = vdev;
+ cdev_hold(cdev);
mutex_unlock(&cdev_mutex);
if (port)
*port = i;
@@ -542,6 +548,8 @@ rel_rt:
if (csk)
cxgbi_sock_closed(csk);
err_out:
+ if (cdev)
+ cdev_put(cdev);
return ERR_PTR(err);
}
@@ -2491,6 +2499,7 @@ struct iscsi_endpoint *cxgbi_ep_connect(struct Scsi_Host *shost,
return ep;
release_conn:
+ cdev_put(&csk->cdev);
cxgbi_sock_put(csk);
cxgbi_sock_closed(csk);
err_out:
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 9267844..aad1749 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -514,6 +514,7 @@ struct cxgbi_device {
unsigned int flags;
struct net_device **ports;
void *lldev;
+ atomic_t use_count;
struct cxgbi_hba **hbas;
const unsigned short *mtus;
unsigned char nmtus;
@@ -557,6 +558,8 @@ struct cxgbi_device {
void *dd_data;
};
#define cxgbi_cdev_priv(cdev) ((cdev)->dd_data)
+#define cdev_hold(x) do {atomic_inc(&x->use_count);} while(0)
+#define cdev_put(x) do {atomic_dec(&x->use_count);} while(0)
struct cxgbi_conn {
struct cxgbi_endpoint *cep;
--
1.7.3.4
next reply other threads:[~2011-07-25 19:57 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-25 19:56 Neil Horman [this message]
2011-07-25 21:19 ` [PATCH] cxgb3i: ref count cdev access to prevent modification while in use Divy Le Ray
2011-07-26 10:50 ` Neil Horman
2011-07-26 16:42 ` Karen Xie
2011-08-01 11:18 ` Neil Horman
2011-08-01 14:42 ` Steve Wise
2011-08-02 0:36 ` David Miller
-- strict thread matches above, loose matches on Subject: below --
2011-08-02 2:00 Neil Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1311623817-6417-1-git-send-email-nhorman@tuxdriver.com \
--to=nhorman@tuxdriver.com \
--cc=davem@davemloft.net \
--cc=divy@chelsio.com \
--cc=netdev@vger.kernel.org \
--cc=swise@chelsio.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).