LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/4] IB/mthca: Cleanup and optimize a few bitmap operation
@ 2021-11-24 20:40 Christophe JAILLET
  2021-11-24 20:40 ` [PATCH 1/4] IB/mthca: Use bitmap_zalloc() when applicable Christophe JAILLET
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Christophe JAILLET @ 2021-11-24 20:40 UTC (permalink / raw)
  To: dledford, jgg
  Cc: linux-rdma, linux-kernel, kernel-janitors, Christophe JAILLET

Patch 1 and 2 are just cleanups that uses 'bitmap_zalloc()' and 'bitmap_set()'
instead of hand-writing these functions.

Patch 3 and 4 are more questionable. They replace calls to '[set|clear]_bit()'
by their non-atomic '__[set|clear]_bit()' alternatives.
In both files, it looks safe to do so because accesses to the corresponding
bitmaps are protected by spinlocks.
However, these patches are compile tested only. It not sure it worth changing the
code just for saving a few atomic operations.
So review, test and apply only if it make sense.

Christophe JAILLET (4):
  IB/mthca: Use bitmap_zalloc() when applicable
  IB/mthca: Use bitmap_set() when applicable
  IB/mthca: Use non-atomic bitmap functions when possible in
    'mthca_allocator.c'
  IB/mthca: Use non-atomic bitmap functions when possible in
    'mthca_mr.c'

 drivers/infiniband/hw/mthca/mthca_allocator.c | 15 +++++--------
 drivers/infiniband/hw/mthca/mthca_mr.c        | 22 +++++++++----------
 2 files changed, 15 insertions(+), 22 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] IB/mthca: Use bitmap_zalloc() when applicable
  2021-11-24 20:40 [PATCH 0/4] IB/mthca: Cleanup and optimize a few bitmap operation Christophe JAILLET
@ 2021-11-24 20:40 ` Christophe JAILLET
  2021-11-24 20:41 ` [PATCH 2/4] IB/mthca: Use bitmap_set() " Christophe JAILLET
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Christophe JAILLET @ 2021-11-24 20:40 UTC (permalink / raw)
  To: dledford, jgg
  Cc: linux-rdma, linux-kernel, kernel-janitors, Christophe JAILLET

Use 'bitmap_zalloc()' to simplify code, improve the semantic and avoid some
open-coded arithmetic in allocator arguments.

Using the 'zalloc' version of the allocator also saves a now useless
'bitmap_zero()' call.

Also change the corresponding 'kfree()' into 'bitmap_free()' to keep
consistency.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/infiniband/hw/mthca/mthca_allocator.c |  6 ++----
 drivers/infiniband/hw/mthca/mthca_mr.c        | 12 +++++-------
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_allocator.c b/drivers/infiniband/hw/mthca/mthca_allocator.c
index aef1d274a14e..06fc8a2e0bd4 100644
--- a/drivers/infiniband/hw/mthca/mthca_allocator.c
+++ b/drivers/infiniband/hw/mthca/mthca_allocator.c
@@ -90,12 +90,10 @@ int mthca_alloc_init(struct mthca_alloc *alloc, u32 num, u32 mask,
 	alloc->max  = num;
 	alloc->mask = mask;
 	spin_lock_init(&alloc->lock);
-	alloc->table = kmalloc_array(BITS_TO_LONGS(num), sizeof(long),
-				     GFP_KERNEL);
+	alloc->table = bitmap_zalloc(num, GFP_KERNEL);
 	if (!alloc->table)
 		return -ENOMEM;
 
-	bitmap_zero(alloc->table, num);
 	for (i = 0; i < reserved; ++i)
 		set_bit(i, alloc->table);
 
@@ -104,7 +102,7 @@ int mthca_alloc_init(struct mthca_alloc *alloc, u32 num, u32 mask,
 
 void mthca_alloc_cleanup(struct mthca_alloc *alloc)
 {
-	kfree(alloc->table);
+	bitmap_free(alloc->table);
 }
 
 /*
diff --git a/drivers/infiniband/hw/mthca/mthca_mr.c b/drivers/infiniband/hw/mthca/mthca_mr.c
index ce0e0867e488..8892fcdbac4c 100644
--- a/drivers/infiniband/hw/mthca/mthca_mr.c
+++ b/drivers/infiniband/hw/mthca/mthca_mr.c
@@ -139,7 +139,7 @@ static void mthca_buddy_free(struct mthca_buddy *buddy, u32 seg, int order)
 
 static int mthca_buddy_init(struct mthca_buddy *buddy, int max_order)
 {
-	int i, s;
+	int i;
 
 	buddy->max_order = max_order;
 	spin_lock_init(&buddy->lock);
@@ -152,12 +152,10 @@ static int mthca_buddy_init(struct mthca_buddy *buddy, int max_order)
 		goto err_out;
 
 	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);
+		buddy->bits[i] = bitmap_zalloc(1 << (buddy->max_order - i),
+					       GFP_KERNEL);
 		if (!buddy->bits[i])
 			goto err_out_free;
-		bitmap_zero(buddy->bits[i],
-			    1 << (buddy->max_order - i));
 	}
 
 	set_bit(0, buddy->bits[buddy->max_order]);
@@ -167,7 +165,7 @@ static int mthca_buddy_init(struct mthca_buddy *buddy, int max_order)
 
 err_out_free:
 	for (i = 0; i <= buddy->max_order; ++i)
-		kfree(buddy->bits[i]);
+		bitmap_free(buddy->bits[i]);
 
 err_out:
 	kfree(buddy->bits);
@@ -181,7 +179,7 @@ static void mthca_buddy_cleanup(struct mthca_buddy *buddy)
 	int i;
 
 	for (i = 0; i <= buddy->max_order; ++i)
-		kfree(buddy->bits[i]);
+		bitmap_free(buddy->bits[i]);
 
 	kfree(buddy->bits);
 	kfree(buddy->num_free);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/4] IB/mthca: Use bitmap_set() when applicable
  2021-11-24 20:40 [PATCH 0/4] IB/mthca: Cleanup and optimize a few bitmap operation Christophe JAILLET
  2021-11-24 20:40 ` [PATCH 1/4] IB/mthca: Use bitmap_zalloc() when applicable Christophe JAILLET
@ 2021-11-24 20:41 ` Christophe JAILLET
  2021-11-24 20:42 ` [PATCH 3/4] IB/mthca: Use non-atomic bitmap functions when possible in 'mthca_allocator.c' Christophe JAILLET
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Christophe JAILLET @ 2021-11-24 20:41 UTC (permalink / raw)
  To: dledford, jgg
  Cc: linux-rdma, linux-kernel, kernel-janitors, Christophe JAILLET

The 'alloc->table' bitmap has just been allocated, so this is safe to use
the faster and non-atomic 'bitmap_set()' function. There is no need to
hand-write it.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/infiniband/hw/mthca/mthca_allocator.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_allocator.c b/drivers/infiniband/hw/mthca/mthca_allocator.c
index 06fc8a2e0bd4..57fa1cc202bc 100644
--- a/drivers/infiniband/hw/mthca/mthca_allocator.c
+++ b/drivers/infiniband/hw/mthca/mthca_allocator.c
@@ -79,8 +79,6 @@ void mthca_free(struct mthca_alloc *alloc, u32 obj)
 int mthca_alloc_init(struct mthca_alloc *alloc, u32 num, u32 mask,
 		     u32 reserved)
 {
-	int i;
-
 	/* num must be a power of 2 */
 	if (num != 1 << (ffs(num) - 1))
 		return -EINVAL;
@@ -94,8 +92,7 @@ int mthca_alloc_init(struct mthca_alloc *alloc, u32 num, u32 mask,
 	if (!alloc->table)
 		return -ENOMEM;
 
-	for (i = 0; i < reserved; ++i)
-		set_bit(i, alloc->table);
+	bitmap_set(alloc->table, 0, reserved);
 
 	return 0;
 }
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/4] IB/mthca: Use non-atomic bitmap functions when possible in 'mthca_allocator.c'
  2021-11-24 20:40 [PATCH 0/4] IB/mthca: Cleanup and optimize a few bitmap operation Christophe JAILLET
  2021-11-24 20:40 ` [PATCH 1/4] IB/mthca: Use bitmap_zalloc() when applicable Christophe JAILLET
  2021-11-24 20:41 ` [PATCH 2/4] IB/mthca: Use bitmap_set() " Christophe JAILLET
@ 2021-11-24 20:42 ` Christophe JAILLET
  2021-11-24 20:43 ` [PATCH 4/4] IB/mthca: Use non-atomic bitmap functions when possible in 'mthca_mr.c' Christophe JAILLET
  2021-11-25 17:30 ` [PATCH 0/4] IB/mthca: Cleanup and optimize a few bitmap operation Jason Gunthorpe
  4 siblings, 0 replies; 6+ messages in thread
From: Christophe JAILLET @ 2021-11-24 20:42 UTC (permalink / raw)
  To: dledford, jgg
  Cc: linux-rdma, linux-kernel, kernel-janitors, Christophe JAILLET

The accesses to the 'alloc->table' bitmap are protected by the
'alloc->lock' spinlock, so no concurrent accesses can happen.

So prefer the non-atomic '__[set|clear]_bit()' functions to save a few
cycles.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/infiniband/hw/mthca/mthca_allocator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_allocator.c b/drivers/infiniband/hw/mthca/mthca_allocator.c
index 57fa1cc202bc..9f0f79d02d3c 100644
--- a/drivers/infiniband/hw/mthca/mthca_allocator.c
+++ b/drivers/infiniband/hw/mthca/mthca_allocator.c
@@ -51,7 +51,7 @@ u32 mthca_alloc(struct mthca_alloc *alloc)
 	}
 
 	if (obj < alloc->max) {
-		set_bit(obj, alloc->table);
+		__set_bit(obj, alloc->table);
 		obj |= alloc->top;
 	} else
 		obj = -1;
@@ -69,7 +69,7 @@ void mthca_free(struct mthca_alloc *alloc, u32 obj)
 
 	spin_lock_irqsave(&alloc->lock, flags);
 
-	clear_bit(obj, alloc->table);
+	__clear_bit(obj, alloc->table);
 	alloc->last = min(alloc->last, obj);
 	alloc->top = (alloc->top + alloc->max) & alloc->mask;
 
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 4/4] IB/mthca: Use non-atomic bitmap functions when possible in 'mthca_mr.c'
  2021-11-24 20:40 [PATCH 0/4] IB/mthca: Cleanup and optimize a few bitmap operation Christophe JAILLET
                   ` (2 preceding siblings ...)
  2021-11-24 20:42 ` [PATCH 3/4] IB/mthca: Use non-atomic bitmap functions when possible in 'mthca_allocator.c' Christophe JAILLET
@ 2021-11-24 20:43 ` Christophe JAILLET
  2021-11-25 17:30 ` [PATCH 0/4] IB/mthca: Cleanup and optimize a few bitmap operation Jason Gunthorpe
  4 siblings, 0 replies; 6+ messages in thread
From: Christophe JAILLET @ 2021-11-24 20:43 UTC (permalink / raw)
  To: dledford, jgg
  Cc: linux-rdma, linux-kernel, kernel-janitors, Christophe JAILLET

In 'mthca_buddy_init()', the 'buddy->bits[n]' bitmap has just been
allocated, so no concurrent accesses can occur.

The other accesses to the 'buddy->bits[n]' bitmap are protected by the
'buddy->lock' spinlock, so no concurrent accesses can occur.

So prefer the non-atomic '__[set|clear]_bit()' functions to save a few
cycles.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/infiniband/hw/mthca/mthca_mr.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_mr.c b/drivers/infiniband/hw/mthca/mthca_mr.c
index 8892fcdbac4c..a59100c496b4 100644
--- a/drivers/infiniband/hw/mthca/mthca_mr.c
+++ b/drivers/infiniband/hw/mthca/mthca_mr.c
@@ -101,13 +101,13 @@ static u32 mthca_buddy_alloc(struct mthca_buddy *buddy, int order)
 	return -1;
 
  found:
-	clear_bit(seg, buddy->bits[o]);
+	__clear_bit(seg, buddy->bits[o]);
 	--buddy->num_free[o];
 
 	while (o > order) {
 		--o;
 		seg <<= 1;
-		set_bit(seg ^ 1, buddy->bits[o]);
+		__set_bit(seg ^ 1, buddy->bits[o]);
 		++buddy->num_free[o];
 	}
 
@@ -125,13 +125,13 @@ static void mthca_buddy_free(struct mthca_buddy *buddy, u32 seg, int order)
 	spin_lock(&buddy->lock);
 
 	while (test_bit(seg ^ 1, buddy->bits[order])) {
-		clear_bit(seg ^ 1, buddy->bits[order]);
+		__clear_bit(seg ^ 1, buddy->bits[order]);
 		--buddy->num_free[order];
 		seg >>= 1;
 		++order;
 	}
 
-	set_bit(seg, buddy->bits[order]);
+	__set_bit(seg, buddy->bits[order]);
 	++buddy->num_free[order];
 
 	spin_unlock(&buddy->lock);
@@ -158,7 +158,7 @@ static int mthca_buddy_init(struct mthca_buddy *buddy, int max_order)
 			goto err_out_free;
 	}
 
-	set_bit(0, buddy->bits[buddy->max_order]);
+	__set_bit(0, buddy->bits[buddy->max_order]);
 	buddy->num_free[buddy->max_order] = 1;
 
 	return 0;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/4] IB/mthca: Cleanup and optimize a few bitmap operation
  2021-11-24 20:40 [PATCH 0/4] IB/mthca: Cleanup and optimize a few bitmap operation Christophe JAILLET
                   ` (3 preceding siblings ...)
  2021-11-24 20:43 ` [PATCH 4/4] IB/mthca: Use non-atomic bitmap functions when possible in 'mthca_mr.c' Christophe JAILLET
@ 2021-11-25 17:30 ` Jason Gunthorpe
  4 siblings, 0 replies; 6+ messages in thread
From: Jason Gunthorpe @ 2021-11-25 17:30 UTC (permalink / raw)
  To: Christophe JAILLET; +Cc: dledford, linux-rdma, linux-kernel, kernel-janitors

On Wed, Nov 24, 2021 at 09:40:09PM +0100, Christophe JAILLET wrote:
> Patch 1 and 2 are just cleanups that uses 'bitmap_zalloc()' and 'bitmap_set()'
> instead of hand-writing these functions.
> 
> Patch 3 and 4 are more questionable. They replace calls to '[set|clear]_bit()'
> by their non-atomic '__[set|clear]_bit()' alternatives.
> In both files, it looks safe to do so because accesses to the corresponding
> bitmaps are protected by spinlocks.
> However, these patches are compile tested only. It not sure it worth changing the
> code just for saving a few atomic operations.
> So review, test and apply only if it make sense.
> 
> Christophe JAILLET (4):
>   IB/mthca: Use bitmap_zalloc() when applicable
>   IB/mthca: Use bitmap_set() when applicable
>   IB/mthca: Use non-atomic bitmap functions when possible in
>     'mthca_allocator.c'
>   IB/mthca: Use non-atomic bitmap functions when possible in
>     'mthca_mr.c'

Applied to for-next, thanks

Jason

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-25 17:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-24 20:40 [PATCH 0/4] IB/mthca: Cleanup and optimize a few bitmap operation Christophe JAILLET
2021-11-24 20:40 ` [PATCH 1/4] IB/mthca: Use bitmap_zalloc() when applicable Christophe JAILLET
2021-11-24 20:41 ` [PATCH 2/4] IB/mthca: Use bitmap_set() " Christophe JAILLET
2021-11-24 20:42 ` [PATCH 3/4] IB/mthca: Use non-atomic bitmap functions when possible in 'mthca_allocator.c' Christophe JAILLET
2021-11-24 20:43 ` [PATCH 4/4] IB/mthca: Use non-atomic bitmap functions when possible in 'mthca_mr.c' Christophe JAILLET
2021-11-25 17:30 ` [PATCH 0/4] IB/mthca: Cleanup and optimize a few bitmap operation Jason Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).