Merged revisions 5859, 5861-5864, 5866-5868, 5871 from iser branch

[r5871] by yanb123

isert: Fix page leak if alloc_page fails

[r5868] by yanb123

isert: Do not leak workqueues if ib_create_cq failed

[r5867] by yanb123

isert: Document performance considerations for iSER

[r5866] by yanb123

isert: Fix use-after-free when killing iscsi-scstd

Our portal may be destroyed while there are connections alive.
This means we are doing list_del() from list_head that no longer exists

[r5864] by yanb123

isert: Fix crash on device removal when iscsi-scstd is started with explicit address

[r5863] by yanb123

isert: Make sure we decrement CQ QP counters if QP creation fails

[r5862] by yanb123

isert: Fix closing devices when connection allocation fails

We need to dereference isert_device structures whether
we created them or not if connection creation fails

[r5861] by yanb123

isert: Warn user of potential compilation issue

[r5859] by yanb123

isert: Fix resource leak upon unreachable event

Unreachable event caused by crash of initiator while in
connection establishment flow, would cause leak of connections.





git-svn-id: http://svn.code.sf.net/p/scst/svn/branches/3.0.x-iser@5877 d57e44dd-8a1f-0410-8b47-8ef2f437770f

iscsi-scst/README.iser

@@ -15,6 +15,12 @@ In order to achieve better performance, it is recommended to specify
 "QueuedCommands 128" parameter per iSER target, since the transport
 is very fast and you usually want to connect it to fast backstorage.
 
+For performance tuning of initiator and target machines, see
+http://community.mellanox.com/docs/DOC-1483
+
+Note that if you have an SSD controller that is close to a particular
+NUMA node, you want the HCA to be close to the same node.
+
 Limitations:
 -------------
 * Bidirectional commands are not supported

iscsi-scst/kernel/isert-scst/iser.h

@@ -9,12 +9,18 @@
 
 #include "iser_hdr.h"
 
+enum isert_portal_state {
+	ISERT_PORTAL_ACTIVE,
+	ISERT_PORTAL_INACTIVE
+};
+
 struct isert_portal {
 	struct rdma_cm_id	*cm_id;
 	struct sockaddr_storage	addr;
 	struct list_head	list_node; /* in portals list */
 	/* protected by dev_list_mutex */
 	struct list_head	conn_list; /* head of conns list */
+	enum isert_portal_state	state;
 };
 
 struct isert_buf {
@@ -165,6 +171,7 @@ struct isert_connection {
 	struct isert_wr		drain_wr;
 	struct kref		kref;
 
+	struct isert_portal	*portal;
 	void			*priv_data; /* for connection tracking */
 };
 

iscsi-scst/kernel/isert-scst/iser_buf.c

@@ -84,7 +84,7 @@ static int isert_buf_alloc_pg(struct ib_device *ib_dev,
 	goto out;
 
 out_map_failed:
-	for (; i > 0; --i)
+	for (; i >= 0; --i)
 		__free_page(sg_page(&isert_buf->sg[i]));
 	kfree(isert_buf->sg);
 	isert_buf->sg = NULL;

iscsi-scst/kernel/isert-scst/iser_rdma.c

@@ -45,6 +45,8 @@
 
 static DEFINE_MUTEX(dev_list_mutex);
 
+void isert_portal_free(struct isert_portal *portal);
+
 static int isert_num_recv_posted_on_err(struct ib_recv_wr *first_ib_wr,
 					struct ib_recv_wr *bad_wr)
 {
@@ -813,6 +815,11 @@ static struct isert_device *isert_device_create(struct ib_device *ib_dev)
 	cqe_num = min(isert_dev->device_attr.max_cqe, ISER_CQ_ENTRIES);
 	cqe_num = cqe_num / isert_dev->num_cqs;
 
+#ifdef CONFIG_SCST_EXTRACHECKS
+	if (isert_dev->device_attr.max_cqe == 0)
+		pr_err("Zero max_cqe encountered: you may have a compilation problem\n");
+#endif
+
 	for (i = 0; i < isert_dev->num_cqs; ++i) {
 		struct isert_cq *cq_desc = &isert_dev->cq_desc[i];
 
@@ -852,6 +859,7 @@ static struct isert_device *isert_device_create(struct ib_device *ib_dev)
 				  cqe_num,
 				  i); /* completion vector */
 		if (IS_ERR(cq)) {
+			cq_desc->cq = NULL;
 			err = PTR_ERR(cq);
 			pr_err("Failed to create iser dev cq, err:%d\n", err);
 			goto fail_cq;
@@ -879,7 +887,7 @@ static struct isert_device *isert_device_create(struct ib_device *ib_dev)
 	return isert_dev;
 
 fail_cq:
-	for (j = 0; j < i; ++j) {
+	for (j = 0; j <= i; ++j) {
 		if (isert_dev->cq_desc[j].cq)
 			ib_destroy_cq(isert_dev->cq_desc[j].cq);
 		if (isert_dev->cq_desc[j].cq_workqueue)
@@ -1009,7 +1017,7 @@ static int isert_conn_qp_create(struct isert_connection *isert_conn)
 	err = rdma_create_qp(cm_id, isert_dev->pd, &qp_attr);
 	if (unlikely(err)) {
 		pr_err("Failed to create qp, err:%d\n", err);
-		goto out;
+		goto fail_create_qp;
 	}
 	isert_conn->qp = cm_id->qp;
 
@@ -1018,6 +1026,12 @@ static int isert_conn_qp_create(struct isert_connection *isert_conn)
 out:
 	TRACE_EXIT_RES(err);
 	return err;
+
+fail_create_qp:
+	mutex_lock(&dev_list_mutex);
+	isert_dev->cq_qps[cq_idx]--;
+	mutex_unlock(&dev_list_mutex);
+	goto out;
 }
 
 static void isert_conn_qp_destroy(struct isert_connection *isert_conn)
@@ -1031,6 +1045,7 @@ static struct isert_connection *isert_conn_create(struct rdma_cm_id *cm_id,
 {
 	struct isert_connection *isert_conn;
 	int err;
+	struct isert_cq *cq;
 
 	TRACE_ENTRY();
 
@@ -1082,12 +1097,15 @@ static struct isert_connection *isert_conn_create(struct rdma_cm_id *cm_id,
 	}
 
 	kref_init(&isert_conn->kref);
-	kref_get(&isert_conn->kref);
 
 	TRACE_EXIT();
 	return isert_conn;
 
 fail_post_recv:
+	cq = isert_conn->qp->recv_cq->cq_context;
+	mutex_lock(&dev_list_mutex);
+	isert_dev->cq_qps[cq->idx]--;
+	mutex_unlock(&dev_list_mutex);
 	isert_conn_qp_destroy(isert_conn);
 fail_qp:
 	isert_pdu_free(isert_conn->login_rsp_pdu);
@@ -1102,6 +1120,13 @@ fail_get:
 	return ERR_PTR(err);
 }
 
+static void isert_deref_device(struct isert_device *isert_dev)
+{
+	isert_dev->refcnt--;
+	if (isert_dev->refcnt == 0)
+		isert_device_release(isert_dev);
+}
+
 static void isert_kref_free(struct kref *kref)
 {
 	struct isert_connection *isert_conn = container_of(kref,
@@ -1122,9 +1147,9 @@ static void isert_kref_free(struct kref *kref)
 	isert_dev->cq_qps[cq->idx]--;
 	list_del(&isert_conn->portal_node);
 	list_del(&isert_conn->dev_node);
-	isert_dev->refcnt--;
-	if (isert_dev->refcnt == 0)
-		isert_device_release(isert_dev);
+	isert_deref_device(isert_dev);
+	if (unlikely(isert_conn->portal->state == ISERT_PORTAL_INACTIVE))
+		isert_portal_free(isert_conn->portal);
 	mutex_unlock(&dev_list_mutex);
 
 	rdma_destroy_id(isert_conn->cm_id);
@@ -1186,7 +1211,6 @@ static int isert_cm_conn_req_handler(struct rdma_cm_id *cm_id,
 	/* passed in rdma_create_id */
 	struct isert_portal *portal = cm_id->context;
 	struct ib_device *ib_dev = cm_id->device;
-	struct isert_device *new_isert_dev = NULL;
 	struct isert_device *isert_dev;
 	struct isert_connection *isert_conn;
 	struct rdma_conn_param *ini_conn_param;
@@ -1198,13 +1222,12 @@ static int isert_cm_conn_req_handler(struct rdma_cm_id *cm_id,
 	mutex_lock(&dev_list_mutex);
 	isert_dev = isert_device_find(ib_dev);
 	if (!isert_dev) {
-		new_isert_dev = isert_device_create(ib_dev);
-		if (unlikely(IS_ERR(new_isert_dev))) {
-			err = PTR_ERR(new_isert_dev);
+		isert_dev = isert_device_create(ib_dev);
+		if (unlikely(IS_ERR(isert_dev))) {
+			err = PTR_ERR(isert_dev);
 			mutex_unlock(&dev_list_mutex);
 			goto fail_dev_create;
 		}
-		isert_dev = new_isert_dev;
 	}
 	isert_dev->refcnt++;
 	mutex_unlock(&dev_list_mutex);
@@ -1216,6 +1239,7 @@ static int isert_cm_conn_req_handler(struct rdma_cm_id *cm_id,
 	}
 
 	isert_conn->state = ISER_CONN_HANDSHAKE;
+	isert_conn->portal = portal;
 
 	mutex_lock(&dev_list_mutex);
 	list_add_tail(&isert_conn->portal_node, &portal->conn_list);
@@ -1287,13 +1311,9 @@ fail_accept:
 	goto out;
 
 fail_conn_create:
-	if (new_isert_dev) {
-		mutex_lock(&dev_list_mutex);
-		new_isert_dev->refcnt--;
-		if (new_isert_dev->refcnt == 0)
-			isert_device_release(new_isert_dev);
-		mutex_unlock(&dev_list_mutex);
-	}
+	mutex_lock(&dev_list_mutex);
+	isert_deref_device(isert_dev);
+	mutex_unlock(&dev_list_mutex);
 fail_dev_create:
 	rdma_reject(cm_id, NULL, 0);
 	goto out;
@@ -1318,6 +1338,7 @@ static int isert_cm_connect_handler(struct rdma_cm_id *cm_id,
 	if (unlikely(ret))
 		goto out;
 
+	kref_get(&isert_conn->kref);
 	kref_get(&isert_conn->kref);
 	/* notify upper layer */
 	ret = isert_conn_established(&isert_conn->iscsi,
@@ -1396,6 +1417,31 @@ static int isert_handle_failure(struct isert_connection *conn)
 	return 0;
 }
 
+static int isert_cm_evt_listener_handler(struct rdma_cm_id *cm_id,
+					 struct rdma_cm_event *cm_ev)
+{
+	enum rdma_cm_event_type ev_type;
+	struct isert_portal *portal;
+	int err = 0;
+
+	ev_type = cm_ev->event;
+	portal = cm_id->context;
+
+	switch (ev_type) {
+	case RDMA_CM_EVENT_DEVICE_REMOVAL:
+		portal->cm_id = NULL;
+		err = -EINVAL;
+		break;
+
+	default:
+		pr_info("Listener event:%s(%d), ignored\n",
+			cm_event_type_str(ev_type), ev_type);
+		break;
+	}
+
+	return err;
+}
+
 static int isert_cm_evt_handler(struct rdma_cm_id *cm_id,
 				struct rdma_cm_event *cm_ev)
 {
@@ -1415,6 +1461,11 @@ static int isert_cm_evt_handler(struct rdma_cm_id *cm_id,
 		cm_event_type_str(ev_type), ev_type, cm_ev->status,
 		portal, cm_id);
 
+	if (portal->cm_id == cm_id) {
+		err = isert_cm_evt_listener_handler(cm_id, cm_ev);
+		goto out;
+	}
+
 	switch (ev_type) {
 	case RDMA_CM_EVENT_CONNECT_REQUEST:
 		err = isert_cm_conn_req_handler(cm_id, cm_ev);
@@ -1591,23 +1642,36 @@ out:
 	return err;
 }
 
+void isert_portal_free(struct isert_portal *portal)
+{
+	lockdep_assert_held(&dev_list_mutex);
+
+	if (!list_empty(&portal->conn_list))
+		return;
+
+	kfree(portal);
+	module_put(THIS_MODULE);
+}
+
 void isert_portal_release(struct isert_portal *portal)
 {
 	struct isert_connection *conn;
 
 	pr_info("iser portal cm_id:%p releasing\n", portal->cm_id);
 
-	rdma_destroy_id(portal->cm_id);
+	if (portal->cm_id) {
+		rdma_destroy_id(portal->cm_id);
+		portal->cm_id = NULL;
+	}
+
+	isert_portal_list_remove(portal);
 
 	mutex_lock(&dev_list_mutex);
 	list_for_each_entry(conn, &portal->conn_list, portal_node)
 		isert_conn_disconnect(conn);
+	portal->state = ISERT_PORTAL_INACTIVE;
+	isert_portal_free(portal);
 	mutex_unlock(&dev_list_mutex);
-
-	isert_portal_list_remove(portal);
-
-	kfree(portal);
-	module_put(THIS_MODULE);
 }
 
 struct isert_portal *isert_portal_start(struct sockaddr *sa, size_t addr_len)