[PATCH 0/7] ib_srpt patches

[PATCH 0/7] ib_srpt patches

[Bart Van Assche]

The following patches apply on top of the ib_srpt source code as posted on
December 20, 2010:
1. Add the attribute hw_address to sysfs such that udev rules can be developed
   that allow to make the relationship between an SCST target name and a HCA
   node GUID persistent.
2. Convert I/O context state from an atomic variable to a spinlock-protected
   regular variable.
3. Make the RDMA channel state names more brief.
4. Introduce the function srpt_ch_qp_err().
5. Fix a bug in a session creation error path.
6. Convert the channel state from an atomic variable to a spinlock-protected
   regular variable.
7. Fix races related to session unregistration.
--
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

[PATCH 1/7] ib_srpt: Add hw_address and service_id to sysfs

[Bart Van Assche]

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

[PATCH 2/7] ib_srpt: Protect I/O context state by a spinlock

[Bart Van Assche]

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

[PATCH 3/7] ib_srpt: Make channel state names more brief

[Bart Van Assche]

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

[PATCH 4/7] ib_srpt: Introduce srpt_ch_qp_err()

[Bart Van Assche]

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

[PATCH 5/7] ib_srpt: Fix a bug in an error path

[Bart Van Assche]

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

[PATCH 6/7] ib_srpt: Use locking to protect channel state

[Bart Van Assche]

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

[PATCH 7/7] ib_srpt: Fix session unregistration races

[Bart Van Assche]

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