From c707a4fd2d180ee9d46e72944f245a43d34dda3c Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 10 Oct 2016 18:09:12 +0000 Subject: [PATCH 1/6] ib_srpt: Use tabs instead of spaces to indent git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@6998 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- srpt/src/ib_srpt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/srpt/src/ib_srpt.c b/srpt/src/ib_srpt.c index 660d6d7be..e92c6b746 100644 --- a/srpt/src/ib_srpt.c +++ b/srpt/src/ib_srpt.c @@ -129,7 +129,7 @@ MODULE_PARM_DESC(srp_max_rsp_size, "Maximum size of SRP response messages in bytes."); #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 31) \ - || defined(RHEL_MAJOR) && RHEL_MAJOR -0 <= 5 + || defined(RHEL_MAJOR) && RHEL_MAJOR -0 <= 5 static int use_srq; #else static bool use_srq; @@ -149,7 +149,7 @@ MODULE_PARM_DESC(srpt_sq_size, "Per-channel send queue (SQ) size."); #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 31) \ - || defined(RHEL_MAJOR) && RHEL_MAJOR -0 <= 5 + || defined(RHEL_MAJOR) && RHEL_MAJOR -0 <= 5 static int use_port_guid_in_session_name; #else static bool use_port_guid_in_session_name; @@ -160,7 +160,7 @@ MODULE_PARM_DESC(use_port_guid_in_session_name, " redundant paths between multiport systems can be masked."); #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 31) \ - || defined(RHEL_MAJOR) && RHEL_MAJOR -0 <= 5 + || defined(RHEL_MAJOR) && RHEL_MAJOR -0 <= 5 static int use_node_guid_in_target_name; #else static bool use_node_guid_in_target_name; From ec59b087a7d1f754c07633ccc9b86c9a5e260628 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 10 Oct 2016 18:09:32 +0000 Subject: [PATCH 2/6] ib_srpt README: Correct default srpt_sq_size git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@6999 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- srpt/README | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/srpt/README b/srpt/README index 1f0c91594..6b0ce56b5 100644 --- a/srpt/README +++ b/srpt/README @@ -72,11 +72,10 @@ The ib_srpt kernel module supports the following parameters: ib_srpt uses a shared receive queue (SRQ) for processing incoming SRP requests. This number may have to be increased when a large number of initiator systems is accessing a single SRP target system. -* srpt_sq_size (number, default 4096) - Per-channel InfiniBand send queue size. The default setting is sufficient - for a credit limit of 128. Changing this parameter to a smaller value may - cause RDMA requests to be retried and hence may slow down data transfer - severely. +* srpt_sq_size (number, default 256) + Per-channel InfiniBand send queue size. Depending on the queue depth, + changing this parameter to a smaller value may cause RDMA requests to be + retried and hence may slow down data transfer severely. * trace_flag (unsigned integer, only available in debug builds) The individual bits of the trace_flag parameter define which categories of trace messages should be sent to the kernel log and which ones not. From 9c466a504a7dc505745c1481591db6a2bd2cc49b Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 10 Oct 2016 18:09:51 +0000 Subject: [PATCH 3/6] ib_srpt: Rework QP failure reporting code git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7000 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- srpt/src/ib_srpt.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/srpt/src/ib_srpt.c b/srpt/src/ib_srpt.c index e92c6b746..27c8093bd 100644 --- a/srpt/src/ib_srpt.c +++ b/srpt/src/ib_srpt.c @@ -2272,24 +2272,20 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch) if (ch->using_rdma_cm) { ret = rdma_create_qp(ch->rdma_cm.cm_id, sdev->pd, qp_init); ch->qp = ch->rdma_cm.cm_id->qp; - if (ret) - pr_err("failed to create queue pair (%d)\n", ret); } else { ch->qp = ib_create_qp(sdev->pd, qp_init); if (!IS_ERR(ch->qp)) { ret = srpt_init_ch_qp(ch, ch->qp); - if (ret) { - pr_err("srpt_init_ch_qp(%#x) failed (%d)\n", - ch->qp->qp_num, ret); + if (ret) ib_destroy_qp(ch->qp); - } } else { ret = PTR_ERR(ch->qp); - pr_err("failed to create queue pair (%d)\n", ret); } } - if (ret) + if (ret) { + pr_err("failed to create queue pair (%d)\n", ret); goto err_destroy_cq; + } pr_debug("qp_num = %#x\n", ch->qp->qp_num); From 95979e90cf554cbb3389f47b943775551120a9a7 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 10 Oct 2016 18:10:13 +0000 Subject: [PATCH 4/6] ib_srpt: Retry with a smaller send queue size if QP creation fails git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7001 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- srpt/src/ib_srpt.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/srpt/src/ib_srpt.c b/srpt/src/ib_srpt.c index 27c8093bd..0a492f7f0 100644 --- a/srpt/src/ib_srpt.c +++ b/srpt/src/ib_srpt.c @@ -2209,7 +2209,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch) { struct ib_qp_init_attr *qp_init; struct srpt_device *sdev = ch->sport->sdev; - int i, ret; + int sq_size = srpt_sq_size, i, ret; EXTRACHECKS_WARN_ON(ch->rq_size < 1); @@ -2218,20 +2218,21 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch) if (!qp_init) goto out; +retry: #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 20) && \ !defined(RHEL_RELEASE_CODE) ch->cq = ib_create_cq(sdev->device, srpt_completion, NULL, ch, - ch->rq_size + srpt_sq_size); + ch->rq_size + sq_size); #elif (LINUX_VERSION_CODE < KERNEL_VERSION(4, 2, 0) || \ defined(MOFED_MAJOR)) && \ !defined(IB_CREATE_CQ_HAS_INIT_ATTR) ch->cq = ib_create_cq(sdev->device, srpt_completion, NULL, ch, - ch->rq_size + srpt_sq_size, ch->comp_vector); + ch->rq_size + sq_size, ch->comp_vector); #else { struct ib_cq_init_attr ia = { }; - ia.cqe = ch->rq_size + srpt_sq_size; + ia.cqe = ch->rq_size + sq_size; ia.comp_vector = ch->comp_vector; ch->cq = ib_create_cq(sdev->device, srpt_completion, NULL, ch, &ia); } @@ -2239,7 +2240,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch) if (IS_ERR(ch->cq)) { ret = PTR_ERR(ch->cq); pr_err("failed to create CQ: cqe %d; c.v. %d; ret %d\n", - ch->rq_size + srpt_sq_size, ch->comp_vector, ret); + ch->rq_size + sq_size, ch->comp_vector, ret); goto out; } @@ -2250,7 +2251,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch) qp_init->recv_cq = ch->cq; qp_init->sq_sig_type = IB_SIGNAL_REQ_WR; qp_init->qp_type = IB_QPT_RC; - qp_init->cap.max_send_wr = srpt_sq_size; + qp_init->cap.max_send_wr = sq_size; /* * For max_sge values > 2 * max_sge_delta, subtract max_sge_delta. For * max_sge values < max_sge_delta, use max_sge. For intermediate @@ -2283,8 +2284,17 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch) } } if (ret) { - pr_err("failed to create queue pair (%d)\n", ret); - goto err_destroy_cq; + bool retry = sq_size > MIN_SRPT_SQ_SIZE; + + pr_err("failed to create queue pair with sq_size = %d (%d)%s\n", + sq_size, ret, retry ? " - retrying" : ""); + if (retry) { + ib_destroy_cq(ch->cq); + sq_size = max(sq_size / 2, MIN_SRPT_SQ_SIZE); + goto retry; + } else { + goto err_destroy_cq; + } } pr_debug("qp_num = %#x\n", ch->qp->qp_num); From 47183f2b16adf54b4aa3af781f94ea705e7be195 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 10 Oct 2016 18:51:06 +0000 Subject: [PATCH 5/6] ib_srpt: Fix a race condition in the initialization code Initialize port information before registering the CM ID. One of the functions called by the CM handler (srpt_cm_req_recv()) namely uses the SCST target pointer so that pointer must be initialized before the CM ID is registered. This patch avoids that SRP login during ib_srpt initialization sporadically triggers the following bug: ib_srpt: Rejecting login with reason 0x10001 BUG: unable to handle kernel NULL pointer dereference at 0000000000000100 IP: [] srpt_cm_req_recv+0xf3f/0xf60 [ib_srpt] Call Trace: [] srpt_ib_cm_req_recv+0xe5/0x110 [ib_srpt] [] cm_process_work+0x1e/0x130 [ib_cm] [] cm_req_handler+0x398/0x460 [ib_cm] [] cm_work_handler+0xb5/0x208 [ib_cm] [] process_one_work+0x16c/0x350 [] worker_thread+0x17a/0x410 [] kthread+0x96/0xa0 [] kernel_thread_helper+0x4/0x10 git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7003 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- srpt/src/ib_srpt.c | 48 ++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/srpt/src/ib_srpt.c b/srpt/src/ib_srpt.c index 0a492f7f0..8e6d92855 100644 --- a/srpt/src/ib_srpt.c +++ b/srpt/src/ib_srpt.c @@ -4401,6 +4401,29 @@ static void srpt_add_one(struct ib_device *device) } } + WARN_ON(sdev->device->phys_port_cnt > ARRAY_SIZE(sdev->port)); + + for (i = 1; i <= sdev->device->phys_port_cnt; i++) { + sport = &sdev->port[i - 1]; + sport->sdev = sdev; + sport->port = i; + srpt_init_sport(sport, sdev->device); +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 20) && !defined(BACKPORT_LINUX_WORKQUEUE_TO_2_6_19) + /* + * A vanilla 2.6.19 or older kernel without backported OFED + * kernel headers. + */ + INIT_WORK(&sport->work, srpt_refresh_port_work, sport); +#else + INIT_WORK(&sport->work, srpt_refresh_port_work); +#endif + if (srpt_refresh_port(sport)) { + pr_err("MAD registration failed for %s-%d.\n", + sdev->device->name, i); + goto err_ring; + } + } + if (!srpt_service_guid) srpt_service_guid = be64_to_cpu(device->node_guid) & ~be64_to_cpu(IB_SERVICE_ID_AGN_MASK); @@ -4441,37 +4464,12 @@ static void srpt_add_one(struct ib_device *device) goto err_cm; } - WARN_ON(sdev->device->phys_port_cnt > ARRAY_SIZE(sdev->port)); - - for (i = 1; i <= sdev->device->phys_port_cnt; i++) { - sport = &sdev->port[i - 1]; - sport->sdev = sdev; - sport->port = i; - srpt_init_sport(sport, sdev->device); -#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 20) && !defined(BACKPORT_LINUX_WORKQUEUE_TO_2_6_19) - /* - * A vanilla 2.6.19 or older kernel without backported OFED - * kernel headers. - */ - INIT_WORK(&sport->work, srpt_refresh_port_work, sport); -#else - INIT_WORK(&sport->work, srpt_refresh_port_work); -#endif - if (srpt_refresh_port(sport)) { - pr_err("MAD registration failed for %s-%d.\n", - sdev->device->name, i); - goto err_event; - } - } - atomic_inc(&srpt_device_count); out: ib_set_client_data(device, &srpt_client, sdev); return; -err_event: - ib_unregister_event_handler(&sdev->event_handler); err_cm: ib_destroy_cm_id(sdev->cm_id); err_ring: From 74a537dcfdacfc9df0bf220fe1dc3f71d3185758 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 10 Oct 2016 18:51:42 +0000 Subject: [PATCH 6/6] ib_srpt: Enable S/G-list clustering git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7004 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- srpt/src/ib_srpt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/srpt/src/ib_srpt.c b/srpt/src/ib_srpt.c index 8e6d92855..3e70ac758 100644 --- a/srpt/src/ib_srpt.c +++ b/srpt/src/ib_srpt.c @@ -4261,6 +4261,7 @@ static const struct attribute *srpt_sess_attrs[] = { static struct scst_tgt_template srpt_template = { .name = DRV_NAME, .sg_tablesize = 1 << 16, + .use_clustering = true, .max_hw_pending_time = RDMA_COMPL_TIMEOUT_S, #if !defined(CONFIG_SCST_PROC) .enable_target = srpt_enable_target,