[PATCH 0/2 v2] srp/srpt - some useful patches

[PATCH 0/2 v2] srp/srpt - some useful patches

[Max Gurtovoy]

No bug fixes, but few improvements that might help.

changes from v1:
 - update commit messages
 - initialize struct ib_qp_attr to zero in the beginning of function

Max Gurtovoy (2):
  IB/srp: allocate ib_qp_attr on the stack
  IB/srpt: allocate ib_qp_attr on the stack

 drivers/infiniband/ulp/srp/ib_srp.c   |   51 ++++++++++++--------------------
 drivers/infiniband/ulp/srpt/ib_srpt.c |   17 ++++-------
 2 files changed, 25 insertions(+), 43 deletions(-)

--
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/2 v2] IB/srp: allocate ib_qp_attr on the stack

[Max Gurtovoy]

No reason to use kmalloc that may increase memory
fragmentation. This also simplifies the code in means of
memory alloc/free in initialization flow.

Tested-by: Israel Rukshin <israelr-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Max Gurtovoy <maxg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |   51 +++++++++++++----------------------
 1 files changed, 19 insertions(+), 32 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 8ddc071..c7f3e1e 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -268,33 +268,28 @@ static void srp_qp_event(struct ib_event *event, void *context)
 static int srp_init_qp(struct srp_target_port *target,
 		       struct ib_qp *qp)
 {
-	struct ib_qp_attr *attr;
+	struct ib_qp_attr attr = { 0 };
 	int ret;
 
-	attr = kmalloc(sizeof *attr, GFP_KERNEL);
-	if (!attr)
-		return -ENOMEM;
-
 	ret = ib_find_cached_pkey(target->srp_host->srp_dev->dev,
 				  target->srp_host->port,
 				  be16_to_cpu(target->pkey),
-				  &attr->pkey_index);
+				  &attr.pkey_index);
 	if (ret)
 		goto out;
 
-	attr->qp_state        = IB_QPS_INIT;
-	attr->qp_access_flags = (IB_ACCESS_REMOTE_READ |
+	attr.qp_state        = IB_QPS_INIT;
+	attr.qp_access_flags = (IB_ACCESS_REMOTE_READ |
 				    IB_ACCESS_REMOTE_WRITE);
-	attr->port_num        = target->srp_host->port;
+	attr.port_num        = target->srp_host->port;
 
-	ret = ib_modify_qp(qp, attr,
+	ret = ib_modify_qp(qp, &attr,
 			   IB_QP_STATE		|
 			   IB_QP_PKEY_INDEX	|
 			   IB_QP_ACCESS_FLAGS	|
 			   IB_QP_PORT);
 
 out:
-	kfree(attr);
 	return ret;
 }
 
@@ -2292,7 +2287,7 @@ static void srp_cm_rep_handler(struct ib_cm_id *cm_id,
 			       struct srp_rdma_ch *ch)
 {
 	struct srp_target_port *target = ch->target;
-	struct ib_qp_attr *qp_attr = NULL;
+	struct ib_qp_attr qp_attr = { 0 };
 	int attr_mask = 0;
 	int ret;
 	int i;
@@ -2324,44 +2319,36 @@ static void srp_cm_rep_handler(struct ib_cm_id *cm_id,
 			goto error;
 	}
 
-	ret = -ENOMEM;
-	qp_attr = kmalloc(sizeof *qp_attr, GFP_KERNEL);
-	if (!qp_attr)
-		goto error;
-
-	qp_attr->qp_state = IB_QPS_RTR;
-	ret = ib_cm_init_qp_attr(cm_id, qp_attr, &attr_mask);
+	qp_attr.qp_state = IB_QPS_RTR;
+	ret = ib_cm_init_qp_attr(cm_id, &qp_attr, &attr_mask);
 	if (ret)
-		goto error_free;
+		goto error;
 
-	ret = ib_modify_qp(ch->qp, qp_attr, attr_mask);
+	ret = ib_modify_qp(ch->qp, &qp_attr, attr_mask);
 	if (ret)
-		goto error_free;
+		goto error;
 
 	for (i = 0; i < target->queue_size; i++) {
 		struct srp_iu *iu = ch->rx_ring[i];
 
 		ret = srp_post_recv(ch, iu);
 		if (ret)
-			goto error_free;
+			goto error;
 	}
 
-	qp_attr->qp_state = IB_QPS_RTS;
-	ret = ib_cm_init_qp_attr(cm_id, qp_attr, &attr_mask);
+	qp_attr.qp_state = IB_QPS_RTS;
+	ret = ib_cm_init_qp_attr(cm_id, &qp_attr, &attr_mask);
 	if (ret)
-		goto error_free;
+		goto error;
 
-	target->rq_tmo_jiffies = srp_compute_rq_tmo(qp_attr, attr_mask);
+	target->rq_tmo_jiffies = srp_compute_rq_tmo(&qp_attr, attr_mask);
 
-	ret = ib_modify_qp(ch->qp, qp_attr, attr_mask);
+	ret = ib_modify_qp(ch->qp, &qp_attr, attr_mask);
 	if (ret)
-		goto error_free;
+		goto error;
 
 	ret = ib_send_cm_rtu(cm_id, NULL, 0);
 
-error_free:
-	kfree(qp_attr);
-
 error:
 	ch->status = 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 2/2 v2] IB/srpt: allocate ib_qp_attr on the stack

[Max Gurtovoy]

No reason to use kzalloc that may increase memory
fragmentation. This also simplifies the code in means of
memory alloc/free in initialization flow.

Signed-off-by: Max Gurtovoy <maxg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index d21ba9d..9117ca6 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -984,24 +984,19 @@ static int srpt_get_desc_tbl(struct srpt_send_ioctx *ioctx,
  */
 static int srpt_init_ch_qp(struct srpt_rdma_ch *ch, struct ib_qp *qp)
 {
-	struct ib_qp_attr *attr;
+	struct ib_qp_attr attr = { 0 };
 	int ret;
 
-	attr = kzalloc(sizeof(*attr), GFP_KERNEL);
-	if (!attr)
-		return -ENOMEM;
-
-	attr->qp_state = IB_QPS_INIT;
-	attr->qp_access_flags = IB_ACCESS_LOCAL_WRITE | IB_ACCESS_REMOTE_READ |
+	attr.qp_state = IB_QPS_INIT;
+	attr.qp_access_flags = IB_ACCESS_LOCAL_WRITE | IB_ACCESS_REMOTE_READ |
 	    IB_ACCESS_REMOTE_WRITE;
-	attr->port_num = ch->sport->port;
-	attr->pkey_index = 0;
+	attr.port_num = ch->sport->port;
+	attr.pkey_index = 0;
 
-	ret = ib_modify_qp(qp, attr,
+	ret = ib_modify_qp(qp, &attr,
 			   IB_QP_STATE | IB_QP_ACCESS_FLAGS | IB_QP_PORT |
 			   IB_QP_PKEY_INDEX);
 
-	kfree(attr);
 	return 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

Re: [PATCH 0/2 v2] srp/srpt - some useful patches

[Bart Van Assche]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 835 bytes --]

On Sun, 2017-01-01 at 16:17 +0200, Max Gurtovoy wrote:
> No bug fixes, but few improvements that might help.

Hello Max,

These patches are not useful to anyone so please drop these patches. More in
detail:
- Linux kernel patches should either be useful to Linux users or make the job
  of kernel maintainers easier. Your patches realize neither of these goals.
- You write that using kmalloc() / kfree() may increase memory fragmentation
  compared to stack allocation. I do not agree with this. And even if your
  claim would be correct, this shouldn't be fixed by modifying the SRP
  initiator and target drivers but this should be fixed by modifying the slab
  allocators.

Bart.

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH 0/2 v2] srp/srpt - some useful patches

[Max Gurtovoy]

ok.
just wanted to align to all other ULPs and simplify the code.
you can ignore these patches.

Max.

On 1/2/2017 11:02 AM, Bart Van Assche wrote:
> On Sun, 2017-01-01 at 16:17 +0200, Max Gurtovoy wrote:
>> No bug fixes, but few improvements that might help.
>
> Hello Max,
>
> These patches are not useful to anyone so please drop these patches. More in
> detail:
> - Linux kernel patches should either be useful to Linux users or make the job
>   of kernel maintainers easier. Your patches realize neither of these goals.
> - You write that using kmalloc() / kfree() may increase memory fragmentation
>   compared to stack allocation. I do not agree with this. And even if your
>   claim would be correct, this shouldn't be fixed by modifying the SRP
>   initiator and target drivers but this should be fixed by modifying the slab
>   allocators.
>
> Bart.
>
--
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