* [PATCH 1/7] ib_srpt: Add hw_address and service_id to sysfs
[not found] ` <201101091723.41399.bvanassche-HInyCGIudOg@public.gmane.org>
@ 2011-01-09 16:24 ` Bart Van Assche
2011-01-09 16:25 ` [PATCH 2/7] ib_srpt: Protect I/O context state by a spinlock Bart Van Assche
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2011-01-09 16:24 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Roland Dreier
Add the attribute 'service_id' at driver level and add the attribute
'hw_address' at device level. Remove the sysfs attribute 'login_info'
because all information it contains is now redundant with other sysfs
attributes. Document the newly added attributes.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
Documentation/ABI/stable/sysfs-devices-scst_target | 11 ++++
.../ABI/stable/sysfs-driver-scst_target-ib_srpt | 9 +++
drivers/scst/srpt/ib_srpt.c | 56 +++++++++-----------
3 files changed, 46 insertions(+), 30 deletions(-)
create mode 100644 Documentation/ABI/stable/sysfs-driver-scst_target-ib_srpt
diff --git a/Documentation/ABI/stable/sysfs-devices-scst_target b/Documentation/ABI/stable/sysfs-devices-scst_target
index 1792aa3..b5e3980 100644
--- a/Documentation/ABI/stable/sysfs-devices-scst_target
+++ b/Documentation/ABI/stable/sysfs-devices-scst_target
@@ -56,6 +56,17 @@ Description:
$ echo "in target_driver/ib_srpt/ib_srpt_target_0 enable" >/sys/devices/scst/mgmt
$ cat /sys/devices/ib_srpt_target_0/enabled
+What: /sys/bus/scst_target/devices/<target>/hw_address
+Date: December 2010
+Contact: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
+Description:
+ Address that uniquely identifies the hardware entity that
+ corresponds to a target. For ib_srpt this is the HCA node
+ GUID. Read-only. An example:
+
+ $ cat /sys/bus/scst_target/devices/ib_srpt_target_0/address
+ 0002c9030005f34e
+
What: /sys/bus/scst_target/devices/*/hw_target
Date: December 2010
Contact: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
diff --git a/Documentation/ABI/stable/sysfs-driver-scst_target-ib_srpt b/Documentation/ABI/stable/sysfs-driver-scst_target-ib_srpt
new file mode 100644
index 0000000..4e54cb9
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-scst_target-ib_srpt
@@ -0,0 +1,9 @@
+What: /sys/bus/scst_target/drivers/ib_srpt/service_id
+Date: December 2010
+Contact: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
+Description:
+ SRP Service ID used by ib_srpt on this server. Read-only. An
+ example:
+
+ $ cat /sys/bus/scst_target/drivers/ib_srpt/service_id
+ 0002c9030005f34e
diff --git a/drivers/scst/srpt/ib_srpt.c b/drivers/scst/srpt/ib_srpt.c
index 28f860f..4c9c700 100644
--- a/drivers/scst/srpt/ib_srpt.c
+++ b/drivers/scst/srpt/ib_srpt.c
@@ -3222,46 +3222,41 @@ static uint16_t srpt_get_scsi_transport_version(struct scst_tgt *scst_tgt)
return 0x0940; /* SRP */
}
-static ssize_t show_login_info(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t tgtt_service_id_show(struct device_driver *drv, char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%016llx\n", srpt_service_guid);
+}
+
+static struct driver_attribute srpt_tgtt_service_id_attr =
+ __ATTR(service_id, S_IRUGO, tgtt_service_id_show, NULL);
+
+static const struct driver_attribute *srpt_tgtt_attrs[] = {
+ &srpt_tgtt_service_id_attr,
+ NULL
+};
+
+static ssize_t hw_address_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
struct scst_tgt *scst_tgt;
struct srpt_device *sdev;
- struct srpt_port *sport;
- int i;
- int len;
+ struct ib_device *device;
scst_tgt = scst_dev_to_tgt(dev);
sdev = scst_tgt_get_tgt_priv(scst_tgt);
- len = 0;
- for (i = 0; i < sdev->device->phys_port_cnt; i++) {
- sport = &sdev->port[i];
-
- len += sprintf(buf + len,
- "tid_ext=%016llx,ioc_guid=%016llx,pkey=ffff,"
- "dgid=%04x%04x%04x%04x%04x%04x%04x%04x,"
- "service_id=%016llx\n",
- srpt_service_guid,
- srpt_service_guid,
- be16_to_cpu(((__be16 *) sport->gid.raw)[0]),
- be16_to_cpu(((__be16 *) sport->gid.raw)[1]),
- be16_to_cpu(((__be16 *) sport->gid.raw)[2]),
- be16_to_cpu(((__be16 *) sport->gid.raw)[3]),
- be16_to_cpu(((__be16 *) sport->gid.raw)[4]),
- be16_to_cpu(((__be16 *) sport->gid.raw)[5]),
- be16_to_cpu(((__be16 *) sport->gid.raw)[6]),
- be16_to_cpu(((__be16 *) sport->gid.raw)[7]),
- srpt_service_guid);
- }
-
- return len;
+ device = sdev->device;
+ return scnprintf(buf, PAGE_SIZE, "%04x%04x%04x%04x\n",
+ be16_to_cpu(((__be16 *)&device->node_guid)[0]),
+ be16_to_cpu(((__be16 *)&device->node_guid)[1]),
+ be16_to_cpu(((__be16 *)&device->node_guid)[2]),
+ be16_to_cpu(((__be16 *)&device->node_guid)[3]));
}
-static struct device_attribute srpt_show_login_info_attr =
- __ATTR(login_info, S_IRUGO, show_login_info, NULL);
+static struct device_attribute srpt_hw_address_attr =
+ __ATTR(hw_address, S_IRUGO, hw_address_show, NULL);
static const struct device_attribute *srpt_tgt_attrs[] = {
- &srpt_show_login_info_attr,
+ &srpt_hw_address_attr,
NULL
};
@@ -3310,6 +3305,7 @@ static struct scst_tgt_template srpt_template = {
.max_hw_pending_time = 60/*seconds*/,
.enable_target = srpt_enable_target,
.is_target_enabled = srpt_is_target_enabled,
+ .tgtt_attrs = srpt_tgtt_attrs,
.tgt_attrs = srpt_tgt_attrs,
.sess_attrs = srpt_sess_attrs,
#if defined(CONFIG_SCST_DEBUG) || defined(CONFIG_SCST_TRACING)
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/7] ib_srpt: Protect I/O context state by a spinlock
[not found] ` <201101091723.41399.bvanassche-HInyCGIudOg@public.gmane.org>
2011-01-09 16:24 ` [PATCH 1/7] ib_srpt: Add hw_address and service_id to sysfs Bart Van Assche
@ 2011-01-09 16:25 ` Bart Van Assche
2011-01-09 16:26 ` [PATCH 3/7] ib_srpt: Make channel state names more brief Bart Van Assche
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2011-01-09 16:25 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Roland Dreier
Convert the I/O context state from an atomic variable into a regular
variable protected by a spinlock. Modify srpt_test_and_set_cmd_state()
such that it returns a boolean instead of a state.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
drivers/scst/srpt/ib_srpt.c | 96 +++++++++++++++++++++++++++---------------
drivers/scst/srpt/ib_srpt.h | 3 +-
2 files changed, 64 insertions(+), 35 deletions(-)
diff --git a/drivers/scst/srpt/ib_srpt.c b/drivers/scst/srpt/ib_srpt.c
index 4c9c700..5dc072e 100644
--- a/drivers/scst/srpt/ib_srpt.c
+++ b/drivers/scst/srpt/ib_srpt.c
@@ -722,9 +722,15 @@ static void srpt_free_ioctx_ring(struct srpt_ioctx **ioctx_ring,
*/
static enum srpt_command_state srpt_get_cmd_state(struct srpt_send_ioctx *ioctx)
{
+ enum srpt_command_state state;
+ unsigned long flags;
+
BUG_ON(!ioctx);
- return atomic_read(&ioctx->state);
+ spin_lock_irqsave(&ioctx->spinlock, flags);
+ state = ioctx->state;
+ spin_unlock_irqrestore(&ioctx->spinlock, flags);
+ return state;
}
/**
@@ -738,13 +744,15 @@ static enum srpt_command_state srpt_set_cmd_state(struct srpt_send_ioctx *ioctx,
enum srpt_command_state new)
{
enum srpt_command_state previous;
+ unsigned long flags;
BUG_ON(!ioctx);
- do {
- previous = atomic_read(&ioctx->state);
- } while (previous != SRPT_STATE_DONE
- && atomic_cmpxchg(&ioctx->state, previous, new) != previous);
+ spin_lock_irqsave(&ioctx->spinlock, flags);
+ previous = ioctx->state;
+ if (previous != SRPT_STATE_DONE)
+ ioctx->state = new;
+ spin_unlock_irqrestore(&ioctx->spinlock, flags);
return previous;
}
@@ -754,18 +762,25 @@ static enum srpt_command_state srpt_set_cmd_state(struct srpt_send_ioctx *ioctx,
* @old: State to compare against.
* @new: New state to be set if the current state matches 'old'.
*
- * Returns the previous command state.
+ * Returns true if and only if the previous command state was equal to 'old'.
*/
-static enum srpt_command_state
-srpt_test_and_set_cmd_state(struct srpt_send_ioctx *ioctx,
- enum srpt_command_state old,
- enum srpt_command_state new)
+static bool srpt_test_and_set_cmd_state(struct srpt_send_ioctx *ioctx,
+ enum srpt_command_state old,
+ enum srpt_command_state new)
{
+ enum srpt_command_state previous;
+ unsigned long flags;
+
WARN_ON(!ioctx);
WARN_ON(old == SRPT_STATE_DONE);
WARN_ON(new == SRPT_STATE_NEW);
- return atomic_cmpxchg(&ioctx->state, old, new);
+ spin_lock_irqsave(&ioctx->spinlock, flags);
+ previous = ioctx->state;
+ if (previous == old)
+ ioctx->state = new;
+ spin_unlock_irqrestore(&ioctx->spinlock, flags);
+ return previous == old;
}
/**
@@ -1052,7 +1067,8 @@ static struct srpt_send_ioctx *srpt_get_send_ioctx(struct srpt_rdma_ch *ch)
return ioctx;
BUG_ON(ioctx->ch != ch);
- atomic_set(&ioctx->state, SRPT_STATE_NEW);
+ spin_lock_init(&ioctx->spinlock);
+ ioctx->state = SRPT_STATE_NEW;
ioctx->n_rbuf = 0;
ioctx->rbufs = NULL;
ioctx->n_rdma = 0;
@@ -1108,6 +1124,7 @@ static void srpt_abort_scst_cmd(struct srpt_send_ioctx *ioctx,
{
struct scst_cmd *scmnd;
enum srpt_command_state state;
+ unsigned long flags;
BUG_ON(!ioctx);
@@ -1118,16 +1135,22 @@ static void srpt_abort_scst_cmd(struct srpt_send_ioctx *ioctx,
* ensures that srpt_xmit_response() will call this function a second
* time.
*/
- state = srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_NEED_DATA,
- SRPT_STATE_DATA_IN);
- if (state != SRPT_STATE_NEED_DATA) {
- state = srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_DATA_IN,
- SRPT_STATE_DONE);
- if (state != SRPT_STATE_DATA_IN) {
- state = srpt_test_and_set_cmd_state(ioctx,
- SRPT_STATE_CMD_RSP_SENT, SRPT_STATE_DONE);
- }
+
+ spin_lock_irqsave(&ioctx->spinlock, flags);
+ state = ioctx->state;
+ switch (state) {
+ case SRPT_STATE_NEED_DATA:
+ ioctx->state = SRPT_STATE_DATA_IN;
+ break;
+ case SRPT_STATE_DATA_IN:
+ case SRPT_STATE_CMD_RSP_SENT:
+ ioctx->state = SRPT_STATE_DONE;
+ break;
+ default:
+ break;
}
+ spin_unlock_irqrestore(&ioctx->spinlock, flags);
+
if (state == SRPT_STATE_DONE)
goto out;
@@ -1263,7 +1286,6 @@ static void srpt_handle_rdma_comp(struct srpt_rdma_ch *ch,
struct srpt_send_ioctx *ioctx,
enum scst_exec_context context)
{
- enum srpt_command_state state;
struct scst_cmd *scmnd;
EXTRACHECKS_WARN_ON(ioctx->n_rdma <= 0);
@@ -1271,14 +1293,13 @@ static void srpt_handle_rdma_comp(struct srpt_rdma_ch *ch,
scmnd = ioctx->scmnd;
if (scmnd) {
- state = srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_NEED_DATA,
- SRPT_STATE_DATA_IN);
- if (state == SRPT_STATE_NEED_DATA)
+ if (srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_NEED_DATA,
+ SRPT_STATE_DATA_IN))
scst_rx_data(ioctx->scmnd, SCST_RX_STATUS_SUCCESS,
context);
else
PRINT_ERROR("%s[%d]: wrong state = %d", __func__,
- __LINE__, state);
+ __LINE__, srpt_get_cmd_state(ioctx));
} else
PRINT_ERROR("%s[%d]: scmnd == NULL", __func__, __LINE__);
}
@@ -2986,6 +3007,7 @@ static int srpt_xmit_response(struct scst_cmd *scmnd)
struct srpt_rdma_ch *ch;
struct srpt_send_ioctx *ioctx;
enum srpt_command_state state;
+ unsigned long flags;
int ret;
scst_data_direction dir;
int resp_len;
@@ -2998,15 +3020,21 @@ static int srpt_xmit_response(struct scst_cmd *scmnd)
ch = scst_sess_get_tgt_priv(scst_cmd_get_session(scmnd));
BUG_ON(!ch);
- state = srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_NEW,
- SRPT_STATE_CMD_RSP_SENT);
- if (state != SRPT_STATE_NEW) {
- state = srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_DATA_IN,
- SRPT_STATE_CMD_RSP_SENT);
- if (state != SRPT_STATE_DATA_IN)
- PRINT_ERROR("Unexpected command state %d",
- srpt_get_cmd_state(ioctx));
+ spin_lock_irqsave(&ioctx->spinlock, flags);
+ state = ioctx->state;
+ switch (state) {
+ case SRPT_STATE_NEW:
+ ioctx->state = SRPT_STATE_CMD_RSP_SENT;
+ case SRPT_STATE_DATA_IN:
+ ioctx->state = SRPT_STATE_CMD_RSP_SENT;
+ break;
+ default:
+ PRINT_ERROR("Unexpected command state %d",
+ srpt_get_cmd_state(ioctx));
+ break;
}
+ spin_unlock_irqrestore(&ioctx->spinlock, flags);
+
if (unlikely(scst_cmd_aborted(scmnd))) {
atomic_inc(&ch->req_lim_delta);
diff --git a/drivers/scst/srpt/ib_srpt.h b/drivers/scst/srpt/ib_srpt.h
index 3e893d9..b8f14d4 100644
--- a/drivers/scst/srpt/ib_srpt.h
+++ b/drivers/scst/srpt/ib_srpt.h
@@ -209,7 +209,8 @@ struct srpt_send_ioctx {
struct scst_cmd *scmnd;
scst_data_direction dir;
- atomic_t state;
+ spinlock_t spinlock;
+ enum srpt_command_state state;
};
/**
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/7] ib_srpt: Make channel state names more brief
[not found] ` <201101091723.41399.bvanassche-HInyCGIudOg@public.gmane.org>
2011-01-09 16:24 ` [PATCH 1/7] ib_srpt: Add hw_address and service_id to sysfs Bart Van Assche
2011-01-09 16:25 ` [PATCH 2/7] ib_srpt: Protect I/O context state by a spinlock Bart Van Assche
@ 2011-01-09 16:26 ` Bart Van Assche
2011-01-09 16:27 ` [PATCH 4/7] ib_srpt: Introduce srpt_ch_qp_err() Bart Van Assche
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2011-01-09 16:26 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Roland Dreier
Make the channel state names more brief. Change the return value of
srpt_test_and_set_channel_state() from enum into bool.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
drivers/scst/srpt/ib_srpt.c | 53 ++++++++++++++++++++----------------------
drivers/scst/srpt/ib_srpt.h | 6 ++--
2 files changed, 28 insertions(+), 31 deletions(-)
diff --git a/drivers/scst/srpt/ib_srpt.c b/drivers/scst/srpt/ib_srpt.c
index 5dc072e..a308ce1 100644
--- a/drivers/scst/srpt/ib_srpt.c
+++ b/drivers/scst/srpt/ib_srpt.c
@@ -159,14 +159,14 @@ static struct ib_client srpt_client = {
* @new: state to change the channel state to if the current state matches the
* argument 'old'.
*
- * Returns the previous channel state.
+ * Returns true if and only if the channel state has been set to the new state.
*/
-static enum rdma_ch_state
+static bool
srpt_test_and_set_channel_state(struct srpt_rdma_ch *ch,
enum rdma_ch_state old,
enum rdma_ch_state new)
{
- return atomic_cmpxchg(&ch->state, old, new);
+ return atomic_cmpxchg(&ch->state, old, new) == old;
}
/**
@@ -244,8 +244,8 @@ static void srpt_qp_event(struct ib_event *event, struct srpt_rdma_ch *ch)
ib_cm_notify(ch->cm_id, event->event);
break;
case IB_EVENT_QP_LAST_WQE_REACHED:
- if (srpt_test_and_set_channel_state(ch, RDMA_CHANNEL_LIVE,
- RDMA_CHANNEL_DISCONNECTING) == RDMA_CHANNEL_LIVE) {
+ if (srpt_test_and_set_channel_state(ch, CH_LIVE,
+ CH_DISCONNECTING)) {
PRINT_INFO("disconnected session %s.", ch->sess_name);
ib_send_cm_dreq(ch->cm_id, NULL, 0);
}
@@ -1659,12 +1659,12 @@ static void srpt_handle_new_iu(struct srpt_rdma_ch *ch,
ch_state = atomic_read(&ch->state);
srp_cmd = recv_ioctx->ioctx.buf;
- if (unlikely(ch_state == RDMA_CHANNEL_CONNECTING)) {
+ if (unlikely(ch_state == CH_CONNECTING)) {
list_add_tail(&recv_ioctx->wait_list, &ch->cmd_wait_list);
goto out;
}
- if (unlikely(ch_state == RDMA_CHANNEL_DISCONNECTING))
+ if (unlikely(ch_state == CH_DISCONNECTING))
goto post_recv;
if (srp_cmd->opcode == SRP_CMD || srp_cmd->opcode == SRP_TSK_MGMT) {
@@ -1677,7 +1677,7 @@ static void srpt_handle_new_iu(struct srpt_rdma_ch *ch,
}
}
- WARN_ON(ch_state != RDMA_CHANNEL_LIVE);
+ WARN_ON(ch_state != CH_LIVE);
switch (srp_cmd->opcode) {
case SRP_CMD:
@@ -1785,7 +1785,7 @@ static void srpt_process_send_completion(struct ib_cq *cq,
while (unlikely(opcode == IB_WC_SEND
&& !list_empty(&ch->cmd_wait_list)
- && atomic_read(&ch->state) == RDMA_CHANNEL_LIVE
+ && atomic_read(&ch->state) == CH_LIVE
&& (send_ioctx = srpt_get_send_ioctx(ch)) != NULL)) {
struct srpt_recv_ioctx *recv_ioctx;
@@ -1978,7 +1978,7 @@ static void srpt_unregister_channel(struct srpt_rdma_ch *ch)
sdev = ch->sport->sdev;
list_del(&ch->list);
- atomic_set(&ch->state, RDMA_CHANNEL_DISCONNECTING);
+ atomic_set(&ch->state, CH_DISCONNECTING);
spin_unlock_irq(&sdev->spinlock);
qp_attr.qp_state = IB_QPS_ERR;
@@ -2079,7 +2079,7 @@ static void srpt_release_channel(struct scst_session *scst_sess)
ch = scst_sess_get_tgt_priv(scst_sess);
BUG_ON(!ch);
- WARN_ON(atomic_read(&ch->state) != RDMA_CHANNEL_DISCONNECTING);
+ WARN_ON(atomic_read(&ch->state) != CH_DISCONNECTING);
TRACE_DBG("destroying cm_id %p", ch->cm_id);
BUG_ON(!ch->cm_id);
@@ -2224,8 +2224,8 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
atomic_read(&ch->state));
prev_state = atomic_xchg(&ch->state,
- RDMA_CHANNEL_DISCONNECTING);
- if (prev_state == RDMA_CHANNEL_CONNECTING)
+ CH_DISCONNECTING);
+ if (prev_state == CH_CONNECTING)
srpt_unregister_channel(ch);
spin_unlock_irq(&sdev->spinlock);
@@ -2233,14 +2233,13 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
rsp->rsp_flags =
SRP_LOGIN_RSP_MULTICHAN_TERMINATED;
- if (prev_state == RDMA_CHANNEL_LIVE) {
+ if (prev_state == CH_LIVE) {
ib_send_cm_dreq(ch->cm_id, NULL, 0);
PRINT_INFO("disconnected"
" session %s because a new"
" SRP_LOGIN_REQ has been received.",
ch->sess_name);
- } else if (prev_state ==
- RDMA_CHANNEL_CONNECTING) {
+ } else if (prev_state == CH_CONNECTING) {
PRINT_ERROR("%s", "rejected"
" SRP_LOGIN_REQ because another login"
" request is being processed.");
@@ -2289,7 +2288,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
*/
ch->rq_size = min(SRPT_RQ_SIZE, scst_get_max_lun_commands(NULL, 0));
atomic_set(&ch->processing_compl, 0);
- atomic_set(&ch->state, RDMA_CHANNEL_CONNECTING);
+ atomic_set(&ch->state, CH_CONNECTING);
INIT_LIST_HEAD(&ch->cmd_wait_list);
spin_lock_init(&ch->spinlock);
@@ -2400,7 +2399,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
goto out;
release_channel:
- atomic_set(&ch->state, RDMA_CHANNEL_DISCONNECTING);
+ atomic_set(&ch->state, CH_DISCONNECTING);
scst_unregister_session(ch->scst_sess, 0, NULL);
ch->scst_sess = NULL;
@@ -2454,8 +2453,7 @@ static void srpt_cm_rtu_recv(struct ib_cm_id *cm_id)
if (!ch)
goto out;
- if (srpt_test_and_set_channel_state(ch, RDMA_CHANNEL_CONNECTING,
- RDMA_CHANNEL_LIVE) == RDMA_CHANNEL_CONNECTING) {
+ if (srpt_test_and_set_channel_state(ch, CH_CONNECTING, CH_LIVE)) {
struct srpt_recv_ioctx *ioctx, *ioctx_tmp;
ret = srpt_ch_qp_rts(ch, ch->qp);
@@ -2466,9 +2464,8 @@ static void srpt_cm_rtu_recv(struct ib_cm_id *cm_id)
srpt_handle_new_iu(ch, ioctx, NULL,
SCST_CONTEXT_THREAD);
}
- if (ret && srpt_test_and_set_channel_state(ch,
- RDMA_CHANNEL_LIVE,
- RDMA_CHANNEL_DISCONNECTING) == RDMA_CHANNEL_LIVE) {
+ if (ret && srpt_test_and_set_channel_state(ch, CH_LIVE,
+ CH_DISCONNECTING)) {
TRACE_DBG("cm_id=%p sess_name=%s state=%d",
cm_id, ch->sess_name,
atomic_read(&ch->state));
@@ -2509,13 +2506,13 @@ static void srpt_cm_dreq_recv(struct ib_cm_id *cm_id)
TRACE_DBG("cm_id= %p ch->state= %d", cm_id, atomic_read(&ch->state));
switch (atomic_read(&ch->state)) {
- case RDMA_CHANNEL_LIVE:
- case RDMA_CHANNEL_CONNECTING:
+ case CH_LIVE:
+ case CH_CONNECTING:
ib_send_cm_drep(ch->cm_id, NULL, 0);
PRINT_INFO("Received DREQ and sent DREP for session %s.",
ch->sess_name);
break;
- case RDMA_CHANNEL_DISCONNECTING:
+ case CH_DISCONNECTING:
default:
break;
}
@@ -2980,13 +2977,13 @@ static int srpt_rdy_to_xfer(struct scst_cmd *scmnd)
BUG_ON(!ch);
ch_state = atomic_read(&ch->state);
- if (ch_state == RDMA_CHANNEL_DISCONNECTING) {
+ if (ch_state == CH_DISCONNECTING) {
TRACE_DBG("cmd with tag %lld: channel disconnecting",
scst_cmd_get_tag(scmnd));
srpt_set_cmd_state(ioctx, SRPT_STATE_DATA_IN);
ret = SCST_TGT_RES_FATAL_ERROR;
goto out;
- } else if (ch_state == RDMA_CHANNEL_CONNECTING) {
+ } else if (ch_state == CH_CONNECTING) {
ret = SCST_TGT_RES_QUEUE_FULL;
goto out;
}
diff --git a/drivers/scst/srpt/ib_srpt.h b/drivers/scst/srpt/ib_srpt.h
index b8f14d4..8f69345 100644
--- a/drivers/scst/srpt/ib_srpt.h
+++ b/drivers/scst/srpt/ib_srpt.h
@@ -227,9 +227,9 @@ struct srpt_mgmt_ioctx {
* enum rdma_ch_state - SRP channel state.
*/
enum rdma_ch_state {
- RDMA_CHANNEL_CONNECTING,
- RDMA_CHANNEL_LIVE,
- RDMA_CHANNEL_DISCONNECTING
+ CH_CONNECTING,
+ CH_LIVE,
+ CH_DISCONNECTING
};
/**
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/7] ib_srpt: Introduce srpt_ch_qp_err()
[not found] ` <201101091723.41399.bvanassche-HInyCGIudOg@public.gmane.org>
` (2 preceding siblings ...)
2011-01-09 16:26 ` [PATCH 3/7] ib_srpt: Make channel state names more brief Bart Van Assche
@ 2011-01-09 16:27 ` Bart Van Assche
2011-01-09 16:27 ` [PATCH 5/7] ib_srpt: Fix a bug in an error path Bart Van Assche
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2011-01-09 16:27 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Roland Dreier
Move the code for setting the state of a queue pair into the error state
to a separate function.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
drivers/scst/srpt/ib_srpt.c | 15 ++++++++++++---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/scst/srpt/ib_srpt.c b/drivers/scst/srpt/ib_srpt.c
index a308ce1..08f1de0 100644
--- a/drivers/scst/srpt/ib_srpt.c
+++ b/drivers/scst/srpt/ib_srpt.c
@@ -1045,6 +1045,17 @@ out:
}
/**
+ * srpt_ch_qp_err() - Set the channel queue pair state to 'error'.
+ */
+static int srpt_ch_qp_err(struct srpt_rdma_ch *ch)
+{
+ struct ib_qp_attr qp_attr;
+
+ qp_attr.qp_state = IB_QPS_ERR;
+ return ib_modify_qp(ch->qp, &qp_attr, IB_QP_STATE);
+}
+
+/**
* srpt_get_send_ioctx() - Obtain an I/O context for sending to the initiator.
*/
static struct srpt_send_ioctx *srpt_get_send_ioctx(struct srpt_rdma_ch *ch)
@@ -1973,7 +1984,6 @@ static void srpt_unregister_channel(struct srpt_rdma_ch *ch)
__releases(&ch->sport->sdev->spinlock)
{
struct srpt_device *sdev;
- struct ib_qp_attr qp_attr;
int ret;
sdev = ch->sport->sdev;
@@ -1981,8 +1991,7 @@ static void srpt_unregister_channel(struct srpt_rdma_ch *ch)
atomic_set(&ch->state, CH_DISCONNECTING);
spin_unlock_irq(&sdev->spinlock);
- qp_attr.qp_state = IB_QPS_ERR;
- ret = ib_modify_qp(ch->qp, &qp_attr, IB_QP_STATE);
+ ret = srpt_ch_qp_err(ch);
if (ret < 0)
PRINT_ERROR("Setting queue pair in error state failed: %d",
ret);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 5/7] ib_srpt: Fix a bug in an error path
[not found] ` <201101091723.41399.bvanassche-HInyCGIudOg@public.gmane.org>
` (3 preceding siblings ...)
2011-01-09 16:27 ` [PATCH 4/7] ib_srpt: Introduce srpt_ch_qp_err() Bart Van Assche
@ 2011-01-09 16:27 ` Bart Van Assche
2011-01-09 16:28 ` [PATCH 6/7] ib_srpt: Use locking to protect channel state Bart Van Assche
2011-01-09 16:28 ` [PATCH 7/7] ib_srpt: Fix session unregistration races Bart Van Assche
6 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2011-01-09 16:27 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Roland Dreier
Do not invoke scst_unregister_session() if session registration failed
because invoking scst_unregister_session() with a NULL session pointer
triggers a kernel oops.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
drivers/scst/srpt/ib_srpt.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/scst/srpt/ib_srpt.c b/drivers/scst/srpt/ib_srpt.c
index 08f1de0..66f11f6 100644
--- a/drivers/scst/srpt/ib_srpt.c
+++ b/drivers/scst/srpt/ib_srpt.c
@@ -2365,7 +2365,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
rej->reason = __constant_cpu_to_be32(
SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
TRACE_DBG("%s", "Failed to create SCST session");
- goto release_channel;
+ goto destroy_ib;
}
TRACE_DBG("Establish connection sess=%p name=%s cm_id=%p",
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 6/7] ib_srpt: Use locking to protect channel state
[not found] ` <201101091723.41399.bvanassche-HInyCGIudOg@public.gmane.org>
` (4 preceding siblings ...)
2011-01-09 16:27 ` [PATCH 5/7] ib_srpt: Fix a bug in an error path Bart Van Assche
@ 2011-01-09 16:28 ` Bart Van Assche
2011-01-09 16:28 ` [PATCH 7/7] ib_srpt: Fix session unregistration races Bart Van Assche
6 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2011-01-09 16:28 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Roland Dreier
Protect the channel state against concurrent access via locking instead
of atomic operations.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
drivers/scst/srpt/ib_srpt.c | 62 ++++++++++++++++++++++++++++++++----------
drivers/scst/srpt/ib_srpt.h | 4 +-
2 files changed, 49 insertions(+), 17 deletions(-)
diff --git a/drivers/scst/srpt/ib_srpt.c b/drivers/scst/srpt/ib_srpt.c
index 66f11f6..edd90f1 100644
--- a/drivers/scst/srpt/ib_srpt.c
+++ b/drivers/scst/srpt/ib_srpt.c
@@ -151,6 +151,30 @@ static struct ib_client srpt_client = {
.remove = srpt_remove_one
};
+static enum rdma_ch_state srpt_get_ch_state(struct srpt_rdma_ch *ch)
+{
+ unsigned long flags;
+ enum rdma_ch_state state;
+
+ spin_lock_irqsave(&ch->spinlock, flags);
+ state = ch->state;
+ spin_unlock_irqrestore(&ch->spinlock, flags);
+ return state;
+}
+
+static enum rdma_ch_state
+srpt_set_ch_state(struct srpt_rdma_ch *ch, enum rdma_ch_state new_state)
+{
+ unsigned long flags;
+ enum rdma_ch_state prev;
+
+ spin_lock_irqsave(&ch->spinlock, flags);
+ prev = ch->state;
+ ch->state = new_state;
+ spin_unlock_irqrestore(&ch->spinlock, flags);
+ return prev;
+}
+
/**
* srpt_test_and_set_channel_state() - Test and set the channel state.
*
@@ -166,7 +190,15 @@ srpt_test_and_set_channel_state(struct srpt_rdma_ch *ch,
enum rdma_ch_state old,
enum rdma_ch_state new)
{
- return atomic_cmpxchg(&ch->state, old, new) == old;
+ unsigned long flags;
+ enum rdma_ch_state prev;
+
+ spin_lock_irqsave(&ch->spinlock, flags);
+ prev = ch->state;
+ if (prev == old)
+ ch->state = new;
+ spin_unlock_irqrestore(&ch->spinlock, flags);
+ return prev == old;
}
/**
@@ -237,7 +269,7 @@ static void srpt_qp_event(struct ib_event *event, struct srpt_rdma_ch *ch)
{
TRACE_DBG("QP event %d on cm_id=%p sess_name=%s state=%d",
event->event, ch->cm_id, ch->sess_name,
- atomic_read(&ch->state));
+ srpt_get_ch_state(ch));
switch (event->event) {
case IB_EVENT_COMM_EST:
@@ -1668,7 +1700,7 @@ static void srpt_handle_new_iu(struct srpt_rdma_ch *ch,
recv_ioctx->ioctx.dma, srp_max_req_size,
DMA_FROM_DEVICE);
- ch_state = atomic_read(&ch->state);
+ ch_state = srpt_get_ch_state(ch);
srp_cmd = recv_ioctx->ioctx.buf;
if (unlikely(ch_state == CH_CONNECTING)) {
list_add_tail(&recv_ioctx->wait_list, &ch->cmd_wait_list);
@@ -1796,7 +1828,7 @@ static void srpt_process_send_completion(struct ib_cq *cq,
while (unlikely(opcode == IB_WC_SEND
&& !list_empty(&ch->cmd_wait_list)
- && atomic_read(&ch->state) == CH_LIVE
+ && srpt_get_ch_state(ch) == CH_LIVE
&& (send_ioctx = srpt_get_send_ioctx(ch)) != NULL)) {
struct srpt_recv_ioctx *recv_ioctx;
@@ -1988,7 +2020,7 @@ static void srpt_unregister_channel(struct srpt_rdma_ch *ch)
sdev = ch->sport->sdev;
list_del(&ch->list);
- atomic_set(&ch->state, CH_DISCONNECTING);
+ srpt_set_ch_state(ch, CH_DISCONNECTING);
spin_unlock_irq(&sdev->spinlock);
ret = srpt_ch_qp_err(ch);
@@ -2088,7 +2120,7 @@ static void srpt_release_channel(struct scst_session *scst_sess)
ch = scst_sess_get_tgt_priv(scst_sess);
BUG_ON(!ch);
- WARN_ON(atomic_read(&ch->state) != CH_DISCONNECTING);
+ WARN_ON(srpt_get_ch_state(ch) != CH_DISCONNECTING);
TRACE_DBG("destroying cm_id %p", ch->cm_id);
BUG_ON(!ch->cm_id);
@@ -2230,9 +2262,9 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
TRACE_DBG("Found existing channel name= %s"
" cm_id= %p state= %d",
ch->sess_name, ch->cm_id,
- atomic_read(&ch->state));
+ srpt_get_ch_state(ch));
- prev_state = atomic_xchg(&ch->state,
+ prev_state = srpt_set_ch_state(ch,
CH_DISCONNECTING);
if (prev_state == CH_CONNECTING)
srpt_unregister_channel(ch);
@@ -2297,10 +2329,10 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
*/
ch->rq_size = min(SRPT_RQ_SIZE, scst_get_max_lun_commands(NULL, 0));
atomic_set(&ch->processing_compl, 0);
- atomic_set(&ch->state, CH_CONNECTING);
+ spin_lock_init(&ch->spinlock);
+ ch->state = CH_CONNECTING;
INIT_LIST_HEAD(&ch->cmd_wait_list);
- spin_lock_init(&ch->spinlock);
ch->ioctx_ring = (struct srpt_send_ioctx **)
srpt_alloc_ioctx_ring(ch->sport->sdev, ch->rq_size,
sizeof(*ch->ioctx_ring[0]),
@@ -2408,7 +2440,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
goto out;
release_channel:
- atomic_set(&ch->state, CH_DISCONNECTING);
+ srpt_set_ch_state(ch, CH_DISCONNECTING);
scst_unregister_session(ch->scst_sess, 0, NULL);
ch->scst_sess = NULL;
@@ -2477,7 +2509,7 @@ static void srpt_cm_rtu_recv(struct ib_cm_id *cm_id)
CH_DISCONNECTING)) {
TRACE_DBG("cm_id=%p sess_name=%s state=%d",
cm_id, ch->sess_name,
- atomic_read(&ch->state));
+ srpt_get_ch_state(ch));
ib_send_cm_dreq(ch->cm_id, NULL, 0);
}
}
@@ -2512,9 +2544,9 @@ static void srpt_cm_dreq_recv(struct ib_cm_id *cm_id)
goto out;
}
- TRACE_DBG("cm_id= %p ch->state= %d", cm_id, atomic_read(&ch->state));
+ TRACE_DBG("cm_id= %p ch->state= %d", cm_id, srpt_get_ch_state(ch));
- switch (atomic_read(&ch->state)) {
+ switch (srpt_get_ch_state(ch)) {
case CH_LIVE:
case CH_CONNECTING:
ib_send_cm_drep(ch->cm_id, NULL, 0);
@@ -2985,7 +3017,7 @@ static int srpt_rdy_to_xfer(struct scst_cmd *scmnd)
WARN_ON(ch != scst_sess_get_tgt_priv(scst_cmd_get_session(scmnd)));
BUG_ON(!ch);
- ch_state = atomic_read(&ch->state);
+ ch_state = srpt_get_ch_state(ch);
if (ch_state == CH_DISCONNECTING) {
TRACE_DBG("cmd with tag %lld: channel disconnecting",
scst_cmd_get_tag(scmnd));
diff --git a/drivers/scst/srpt/ib_srpt.h b/drivers/scst/srpt/ib_srpt.h
index 8f69345..e14d192 100644
--- a/drivers/scst/srpt/ib_srpt.h
+++ b/drivers/scst/srpt/ib_srpt.h
@@ -256,7 +256,7 @@ enum rdma_ch_state {
* @cmd_wait_list: list of SCST commands that arrived before the RTU event. This
* list contains struct srpt_ioctx elements and is protected
* against concurrent modification by the cm_id spinlock.
- * @spinlock: Protects free_list.
+ * @spinlock: Protects free_list and state.
* @free_list: Head of list with free send I/O contexts.
* @scst_sess: SCST session information associated with this SRP channel.
* @sess_name: SCST session name.
@@ -280,7 +280,7 @@ struct srpt_rdma_ch {
struct list_head free_list;
struct srpt_send_ioctx **ioctx_ring;
struct ib_wc wc[16];
- atomic_t state;
+ enum rdma_ch_state state;
struct list_head list;
struct list_head cmd_wait_list;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 7/7] ib_srpt: Fix session unregistration races
[not found] ` <201101091723.41399.bvanassche-HInyCGIudOg@public.gmane.org>
` (5 preceding siblings ...)
2011-01-09 16:28 ` [PATCH 6/7] ib_srpt: Use locking to protect channel state Bart Van Assche
@ 2011-01-09 16:28 ` Bart Van Assche
6 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2011-01-09 16:28 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Roland Dreier
Fix the following race conditions related to session unregistration:
- If the memory subsystem does reorder the atomic accesses to the
variables ch->state and ch->processing_compl it can happen that a new
SCST command is issued for a session after unregistration of that
session did start.
- If a session was unregistered because a DREQ had been received it could
happen that the cm_id was destroyed before a DREP had been sent.
Code changes:
- Moved all code for IB disconnection into the function srpt_del_close_ch().
- Invoking scst_unregister_session() is now deferred until the last WQE event
has been received.
- Destroying the cm_id does now happen when the CM has reported that this is
safe instead of when scst_unregister_session() finished.
- Eliminated the variable ch->processing_compl.
- Document the meaning of the channel states.
- Fixed an incorrect comment.
- Do not attempt to invoke ib_post_recv() after the QP has been reset.
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
drivers/scst/srpt/ib_srpt.c | 190 +++++++++++++++++++++++++++----------------
drivers/scst/srpt/ib_srpt.h | 14 +++-
2 files changed, 132 insertions(+), 72 deletions(-)
diff --git a/drivers/scst/srpt/ib_srpt.c b/drivers/scst/srpt/ib_srpt.c
index edd90f1..73b3fd4 100644
--- a/drivers/scst/srpt/ib_srpt.c
+++ b/drivers/scst/srpt/ib_srpt.c
@@ -143,7 +143,8 @@ static void srpt_remove_one(struct ib_device *device);
static void srpt_unregister_mad_agent(struct srpt_device *sdev);
static void srpt_unmap_sg_to_ib_sge(struct srpt_rdma_ch *ch,
struct srpt_send_ioctx *ioctx);
-static void srpt_release_channel(struct scst_session *scst_sess);
+static void srpt_release_channel(struct kref *obj);
+static void srpt_release_channel_qp(struct scst_session *scst_sess);
static struct ib_client srpt_client = {
.name = DRV_NAME,
@@ -263,6 +264,31 @@ static void srpt_srq_event(struct ib_event *event, void *ctx)
}
/**
+ * srpt_unregister_channel() - Unregister channel resources.
+ *
+ * Notes:
+ * - Must only be called after the channel has been removed from the channel
+ * list.
+ * - Invokes ib_destroy_cm_id(ch->cm_id) and kfree(ch) asynchronously.
+ */
+static void srpt_unregister_channel(struct srpt_rdma_ch *ch)
+{
+ if (!srpt_test_and_set_channel_state(ch, CH_DRAINING,
+ CH_DISCONNECTING))
+ return;
+
+ PRINT_INFO("unregistering session %s.", ch->sess_name);
+
+ /*
+ * At this point it is guaranteed that no new commands will be sent to
+ * the SCST core for channel ch, which is a requirement for
+ * scst_unregister_session().
+ */
+
+ scst_unregister_session(ch->scst_sess, 0, srpt_release_channel_qp);
+}
+
+/**
* srpt_qp_event() - QP event callback function.
*/
static void srpt_qp_event(struct ib_event *event, struct srpt_rdma_ch *ch)
@@ -276,11 +302,7 @@ static void srpt_qp_event(struct ib_event *event, struct srpt_rdma_ch *ch)
ib_cm_notify(ch->cm_id, event->event);
break;
case IB_EVENT_QP_LAST_WQE_REACHED:
- if (srpt_test_and_set_channel_state(ch, CH_LIVE,
- CH_DISCONNECTING)) {
- PRINT_INFO("disconnected session %s.", ch->sess_name);
- ib_send_cm_dreq(ch->cm_id, NULL, 0);
- }
+ srpt_unregister_channel(ch);
break;
default:
PRINT_ERROR("received unrecognized IB QP event %d",
@@ -1707,8 +1729,8 @@ static void srpt_handle_new_iu(struct srpt_rdma_ch *ch,
goto out;
}
- if (unlikely(ch_state == CH_DISCONNECTING))
- goto post_recv;
+ if (unlikely(ch_state != CH_LIVE))
+ goto out;
if (srp_cmd->opcode == SRP_CMD || srp_cmd->opcode == SRP_TSK_MGMT) {
if (!send_ioctx)
@@ -1720,8 +1742,6 @@ static void srpt_handle_new_iu(struct srpt_rdma_ch *ch,
}
}
- WARN_ON(ch_state != CH_LIVE);
-
switch (srp_cmd->opcode) {
case SRP_CMD:
srpt_handle_cmd(ch, recv_ioctx, send_ioctx, context);
@@ -1747,7 +1767,6 @@ static void srpt_handle_new_iu(struct srpt_rdma_ch *ch,
break;
}
-post_recv:
srpt_post_recv(ch->sport->sdev, recv_ioctx);
out:
return;
@@ -1878,7 +1897,6 @@ static void srpt_completion(struct ib_cq *cq, void *ctx)
struct srpt_rdma_ch *ch = ctx;
BUG_ON(!ch);
- atomic_inc(&ch->processing_compl);
switch (thread) {
case MODE_IB_COMPLETION_IN_THREAD:
wake_up_interruptible(&ch->wait_queue);
@@ -1890,7 +1908,6 @@ static void srpt_completion(struct ib_cq *cq, void *ctx)
srpt_process_completion(cq, ch, SCST_CONTEXT_TASKLET);
break;
}
- atomic_dec(&ch->processing_compl);
}
static int srpt_compl_thread(void *arg)
@@ -2007,20 +2024,45 @@ static void srpt_destroy_ch_ib(struct srpt_rdma_ch *ch)
}
/**
- * srpt_unregister_channel() - Start RDMA channel disconnection.
+ * __srpt_del_close_ch() - Delete and close an RDMA channel.
+ *
+ * Delete a channel from the channel list, reset the QP and make sure
+ * all resources associated with the channel will be deallocated at an
+ * appropriate time.
*
- * Note: The caller must hold ch->sdev->spinlock.
+ * Notes:
+ * - The caller must hold ch->sport->sdev->spinlock.
+ * - Invokes ib_destroy_cm_id(ch->cm_id) and kfree(ch) either synchronously
+ * or asynchronously.
*/
-static void srpt_unregister_channel(struct srpt_rdma_ch *ch)
+static void __srpt_del_close_ch(struct srpt_rdma_ch *ch,
+ enum srpt_channel_close_action a)
__acquires(&ch->sport->sdev->spinlock)
__releases(&ch->sport->sdev->spinlock)
{
struct srpt_device *sdev;
+ enum rdma_ch_state prev_state;
int ret;
sdev = ch->sport->sdev;
list_del(&ch->list);
- srpt_set_ch_state(ch, CH_DISCONNECTING);
+ prev_state = srpt_set_ch_state(ch, CH_DRAINING);
+ if (prev_state == CH_DRAINING || prev_state == CH_DISCONNECTING)
+ return;
+ switch (a) {
+ case close_action_rej:
+ if (prev_state == CH_CONNECTING) {
+ ib_send_cm_rej(ch->cm_id, IB_CM_REJ_NO_RESOURCES,
+ NULL, 0, NULL, 0);
+ break;
+ }
+ /* fall through */
+ case close_action_send_dreq:
+ ib_send_cm_dreq(ch->cm_id, NULL, 0);
+ break;
+ case close_action_nop:
+ break;
+ }
spin_unlock_irq(&sdev->spinlock);
ret = srpt_ch_qp_err(ch);
@@ -2028,18 +2070,36 @@ static void srpt_unregister_channel(struct srpt_rdma_ch *ch)
PRINT_ERROR("Setting queue pair in error state failed: %d",
ret);
- while (atomic_read(&ch->processing_compl))
- ;
+ switch (prev_state) {
+ case CH_CONNECTING:
+ srpt_unregister_channel(ch);
+ case CH_LIVE:
+ /* Defer srpt_unregister_channel() until last WQE event */
+ break;
+ case CH_DRAINING:
+ case CH_DISCONNECTING:
+ __WARN();
+ break;
+ }
- /*
- * At this point it is guaranteed that no new commands will be sent to
- * the SCST core for channel ch, which is a requirement for
- * scst_unregister_session().
- */
+ spin_lock_irq(&sdev->spinlock);
+}
- TRACE_DBG("unregistering session %p", ch->scst_sess);
- scst_unregister_session(ch->scst_sess, 0, srpt_release_channel);
+/**
+ * srpt_del_close_ch() - Close an RDMA channel.
+ *
+ * Note: Invokes ib_destroy_cm_id(ch->cm_id) and kfree(ch) synchronously or
+ * asynchronously.
+ */
+static void srpt_del_close_ch(struct srpt_rdma_ch *ch,
+ enum srpt_channel_close_action a)
+{
+ struct srpt_device *sdev;
+
+ sdev = ch->sport->sdev;
spin_lock_irq(&sdev->spinlock);
+ __srpt_del_close_ch(ch, a);
+ spin_unlock_irq(&sdev->spinlock);
}
/**
@@ -2066,7 +2126,7 @@ static void srpt_release_channel_by_cmid(struct ib_cm_id *cm_id)
spin_lock_irq(&sdev->spinlock);
list_for_each_entry(ch, &sdev->rch_list, list) {
if (ch->cm_id == cm_id) {
- srpt_unregister_channel(ch);
+ kref_put(&ch->kref, srpt_release_channel);
break;
}
}
@@ -2102,7 +2162,21 @@ static struct srpt_rdma_ch *srpt_find_channel(struct srpt_device *sdev,
}
/**
- * srpt_release_channel() - Release all resources associated with an RDMA channel.
+ * srpt_release_channel() - Release non-QP channel resources.
+ */
+static void srpt_release_channel(struct kref *obj)
+{
+ struct srpt_rdma_ch *ch = container_of(obj, struct srpt_rdma_ch, kref);
+
+ TRACE_DBG("%s(%p)", __func__, ch);
+
+ ib_destroy_cm_id(ch->cm_id);
+
+ kfree(ch);
+}
+
+/**
+ * srpt_release_channel_qp() - Release QP channel resources.
*
* Notes:
* - The caller must have removed the channel from the channel list before
@@ -2110,11 +2184,12 @@ static struct srpt_rdma_ch *srpt_find_channel(struct srpt_device *sdev,
* - Must be called as a callback function via scst_unregister_session(). Never
* call this function directly because doing so would trigger several race
* conditions.
- * - Do not access ch->sport or ch->sport->sdev in this function because the
- * memory that was allocated for the sport and/or sdev data structures may
- * already have been freed at the time this function is called.
+ * - Accessing ch->sport or ch->sport->sdev in this function is safe because
+ * these data structure are deallocated after scst_unregister_target()
+ * has returned, and that function only returns after unregistration of all
+ * associated sessions has finished.
*/
-static void srpt_release_channel(struct scst_session *scst_sess)
+static void srpt_release_channel_qp(struct scst_session *scst_sess)
{
struct srpt_rdma_ch *ch;
@@ -2122,9 +2197,7 @@ static void srpt_release_channel(struct scst_session *scst_sess)
BUG_ON(!ch);
WARN_ON(srpt_get_ch_state(ch) != CH_DISCONNECTING);
- TRACE_DBG("destroying cm_id %p", ch->cm_id);
- BUG_ON(!ch->cm_id);
- ib_destroy_cm_id(ch->cm_id);
+ TRACE_DBG("%s(%s/%p)", __func__, scst_sess->initiator_name, ch->cm_id);
srpt_destroy_ch_ib(ch);
@@ -2132,7 +2205,7 @@ static void srpt_release_channel(struct scst_session *scst_sess)
ch->sport->sdev, ch->rq_size,
srp_max_rsp_size, DMA_TO_DEVICE);
- kfree(ch);
+ kref_put(&ch->kref, srpt_release_channel);
}
/**
@@ -2256,40 +2329,16 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
&& param->port == ch->sport->port
&& param->listen_id == ch->sport->sdev->cm_id
&& ch->cm_id) {
- enum rdma_ch_state prev_state;
-
/* found an existing channel */
TRACE_DBG("Found existing channel name= %s"
" cm_id= %p state= %d",
ch->sess_name, ch->cm_id,
srpt_get_ch_state(ch));
- prev_state = srpt_set_ch_state(ch,
- CH_DISCONNECTING);
- if (prev_state == CH_CONNECTING)
- srpt_unregister_channel(ch);
-
- spin_unlock_irq(&sdev->spinlock);
+ __srpt_del_close_ch(ch, close_action_rej);
rsp->rsp_flags =
SRP_LOGIN_RSP_MULTICHAN_TERMINATED;
-
- if (prev_state == CH_LIVE) {
- ib_send_cm_dreq(ch->cm_id, NULL, 0);
- PRINT_INFO("disconnected"
- " session %s because a new"
- " SRP_LOGIN_REQ has been received.",
- ch->sess_name);
- } else if (prev_state == CH_CONNECTING) {
- PRINT_ERROR("%s", "rejected"
- " SRP_LOGIN_REQ because another login"
- " request is being processed.");
- ib_send_cm_rej(ch->cm_id,
- IB_CM_REJ_NO_RESOURCES,
- NULL, 0, NULL, 0);
- }
-
- spin_lock_irq(&sdev->spinlock);
}
}
@@ -2328,9 +2377,15 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
* for the SRP protocol to the SCST SCSI command queue size.
*/
ch->rq_size = min(SRPT_RQ_SIZE, scst_get_max_lun_commands(NULL, 0));
- atomic_set(&ch->processing_compl, 0);
spin_lock_init(&ch->spinlock);
ch->state = CH_CONNECTING;
+ /*
+ * Initialize kref to 2 such that the session resources are only
+ * released after the connection manager has released the cm_id and
+ * SCST session unregistration finished.
+ */
+ kref_init(&ch->kref);
+ kref_get(&ch->kref);
INIT_LIST_HEAD(&ch->cmd_wait_list);
ch->ioctx_ring = (struct srpt_send_ioctx **)
@@ -2505,13 +2560,8 @@ static void srpt_cm_rtu_recv(struct ib_cm_id *cm_id)
srpt_handle_new_iu(ch, ioctx, NULL,
SCST_CONTEXT_THREAD);
}
- if (ret && srpt_test_and_set_channel_state(ch, CH_LIVE,
- CH_DISCONNECTING)) {
- TRACE_DBG("cm_id=%p sess_name=%s state=%d",
- cm_id, ch->sess_name,
- srpt_get_ch_state(ch));
- ib_send_cm_dreq(ch->cm_id, NULL, 0);
- }
+ if (ret)
+ srpt_del_close_ch(ch, close_action_send_dreq);
}
out:
@@ -3269,7 +3319,7 @@ static int srpt_release(struct scst_tgt *scst_tgt)
spin_lock_irq(&sdev->spinlock);
while (!list_empty(&sdev->rch_list)) {
ch = list_first_entry(&sdev->rch_list, typeof(*ch), list);
- srpt_unregister_channel(ch);
+ __srpt_del_close_ch(ch, close_action_send_dreq);
}
spin_unlock_irq(&sdev->spinlock);
diff --git a/drivers/scst/srpt/ib_srpt.h b/drivers/scst/srpt/ib_srpt.h
index e14d192..26e213e 100644
--- a/drivers/scst/srpt/ib_srpt.h
+++ b/drivers/scst/srpt/ib_srpt.h
@@ -225,13 +225,24 @@ struct srpt_mgmt_ioctx {
/**
* enum rdma_ch_state - SRP channel state.
+ * @CH_CONNECTING: QP is in RTR state; waiting for RTU.
+ * @CH_LIVE: QP is in RTS state.
+ * @CH_DRAINING: QP is in ERR state; waiting for last WQE event.
+ * @CH_DISCONNECTING: Last WQE event has been received.
*/
enum rdma_ch_state {
CH_CONNECTING,
CH_LIVE,
+ CH_DRAINING,
CH_DISCONNECTING
};
+enum srpt_channel_close_action {
+ close_action_nop,
+ close_action_send_dreq,
+ close_action_rej,
+};
+
/**
* struct srpt_rdma_ch - RDMA channel.
* @wait_queue: Allows the kernel thread to wait for more work.
@@ -239,7 +250,6 @@ enum rdma_ch_state {
* the channel.
* @cm_id: IB CM ID associated with the channel.
* @rq_size: IB receive queue size.
- * @processing_compl: whether or not an IB completion is being processed.
* @qp: IB queue pair used for communicating over this channel.
* @sq_wr_avail: number of work requests available in the send queue.
* @cq: IB completion queue for this channel.
@@ -267,7 +277,6 @@ struct srpt_rdma_ch {
struct ib_cm_id *cm_id;
struct ib_qp *qp;
int rq_size;
- atomic_t processing_compl;
struct ib_cq *cq;
atomic_t sq_wr_avail;
struct srpt_port *sport;
@@ -281,6 +290,7 @@ struct srpt_rdma_ch {
struct srpt_send_ioctx **ioctx_ring;
struct ib_wc wc[16];
enum rdma_ch_state state;
+ struct kref kref;
struct list_head list;
struct list_head cmd_wait_list;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 8+ messages in thread