Thread (23 messages) flat view 23 messages, 4 authors, 2016-11-21

Re: [PATCH for-next 03/11] IB/hns: Optimize the logic of allocating memory using APIs

From: Leon Romanovsky <hidden>
Date: 2016-11-16 08:36:13
Also in: linux-rdma, lkml

On Tue, Nov 15, 2016 at 03:52:46PM +0000, Salil Mehta wrote:
quoted
-----Original Message-----
From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
Sent: Wednesday, November 09, 2016 7:22 AM
To: Salil Mehta
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Huwei (Xavier); oulijun;
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Linuxarm;
Zhangping (ZP)
Subject: Re: [PATCH for-next 03/11] IB/hns: Optimize the logic of
allocating memory using APIs

On Fri, Nov 04, 2016 at 04:36:25PM +0000, Salil Mehta wrote:
quoted
From: "Wei Hu (Xavier)" <redacted>

This patch modified the logic of allocating memory using APIs in
hns RoCE driver. We used kcalloc instead of kmalloc_array and
bitmap_zero. And When kcalloc failed, call vzalloc to alloc
memory.

Signed-off-by: Wei Hu (Xavier) <redacted>
Signed-off-by: Ping Zhang <redacted>
Signed-off-by: Salil Mehta  <redacted>
---
 drivers/infiniband/hw/hns/hns_roce_mr.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/infiniband/hw/hns/hns_roce_mr.c
b/drivers/infiniband/hw/hns/hns_roce_mr.c
quoted
index fb87883..d3dfb5f 100644
--- a/drivers/infiniband/hw/hns/hns_roce_mr.c
+++ b/drivers/infiniband/hw/hns/hns_roce_mr.c
@@ -137,11 +137,12 @@ static int hns_roce_buddy_init(struct
hns_roce_buddy *buddy, int max_order)
quoted
 	for (i = 0; i <= buddy->max_order; ++i) {
 		s = BITS_TO_LONGS(1 << (buddy->max_order - i));
-		buddy->bits[i] = kmalloc_array(s, sizeof(long),
GFP_KERNEL);
quoted
-		if (!buddy->bits[i])
-			goto err_out_free;
-
-		bitmap_zero(buddy->bits[i], 1 << (buddy->max_order - i));
+		buddy->bits[i] = kcalloc(s, sizeof(long), GFP_KERNEL);
+		if (!buddy->bits[i]) {
+			buddy->bits[i] = vzalloc(s * sizeof(long));
I wonder, why don't you use directly vzalloc instead of kcalloc
fallback?
As we know we will have physical contiguous pages if the kcalloc
call succeeds. This will give us a chance to have better performance
over the allocations which are just virtually contiguous through the
function vzalloc(). Therefore, later has only been used as a fallback
when our memory request cannot be entertained through kcalloc.

Are you suggesting that there will not be much performance penalty
if we use just vzalloc ?
Not exactly,
I asked it, because we have similar code in our drivers and this
construction looks strange to me.

1. If performance is critical, we will use kmalloc.
2. If performance is not critical, we will use vmalloc.

But in this case, such construction shows me that we can live with
vmalloc performance and kmalloc allocation are not really needed.

In your specific case, I'm not sure that kcalloc will ever fail.

Thanks

quoted
quoted
+			if (!buddy->bits[i])
+				goto err_out_free;
+		}
 	}

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help