[PATCH 0/3] Cleanup and optimize a few bitmap operations

[PATCH 0/3] Cleanup and optimize a few bitmap operations

[Christophe JAILLET]

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

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

Christophe JAILLET (3):
  RDMA/cxgb4: Use bitmap_zalloc() when applicable
  RDMA/cxgb4: Use bitmap_set() when applicable
  RDMA/cxgb4: Use non-atomic bitmap functions when possible

 drivers/infiniband/hw/cxgb4/id_table.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

-- 
2.30.2

[PATCH 1/3] RDMA/cxgb4: Use bitmap_zalloc() when applicable

[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.

While at it, remove an extra space in a statement just a few lines above.

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

diff --git a/drivers/infiniband/hw/cxgb4/id_table.c b/drivers/infiniband/hw/cxgb4/id_table.c
index 724d23297b35..9d08a48c4926 100644
--- a/drivers/infiniband/hw/cxgb4/id_table.c
+++ b/drivers/infiniband/hw/cxgb4/id_table.c
@@ -90,14 +90,12 @@ int c4iw_id_table_alloc(struct c4iw_id_table *alloc, u32 start, u32 num,
 		alloc->last = prandom_u32() % RANDOM_SKIP;
 	else
 		alloc->last = 0;
-	alloc->max  = num;
+	alloc->max = num;
 	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);
 	if (!(alloc->flags & C4IW_ID_TABLE_F_EMPTY))
 		for (i = 0; i < reserved; ++i)
 			set_bit(i, alloc->table);
@@ -107,5 +105,5 @@ int c4iw_id_table_alloc(struct c4iw_id_table *alloc, u32 start, u32 num,
 
 void c4iw_id_table_free(struct c4iw_id_table *alloc)
 {
-	kfree(alloc->table);
+	bitmap_free(alloc->table);
 }
-- 
2.30.2

[PATCH 2/3] RDMA/cxgb4: Use bitmap_set() when applicable

[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/cxgb4/id_table.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/id_table.c b/drivers/infiniband/hw/cxgb4/id_table.c
index 9d08a48c4926..e09faa659d68 100644
--- a/drivers/infiniband/hw/cxgb4/id_table.c
+++ b/drivers/infiniband/hw/cxgb4/id_table.c
@@ -82,8 +82,6 @@ void c4iw_id_free(struct c4iw_id_table *alloc, u32 obj)
 int c4iw_id_table_alloc(struct c4iw_id_table *alloc, u32 start, u32 num,
 			u32 reserved, u32 flags)
 {
-	int i;
-
 	alloc->start = start;
 	alloc->flags = flags;
 	if (flags & C4IW_ID_TABLE_F_RANDOM)
@@ -97,8 +95,7 @@ int c4iw_id_table_alloc(struct c4iw_id_table *alloc, u32 start, u32 num,
 		return -ENOMEM;
 
 	if (!(alloc->flags & C4IW_ID_TABLE_F_EMPTY))
-		for (i = 0; i < reserved; ++i)
-			set_bit(i, alloc->table);
+		bitmap_set(alloc->table, 0, reserved);
 
 	return 0;
 }
-- 
2.30.2

[PATCH 3/3] RDMA/cxgb4: Use non-atomic bitmap functions when possible

[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/cxgb4/id_table.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/id_table.c b/drivers/infiniband/hw/cxgb4/id_table.c
index e09faa659d68..f64e7e02b129 100644
--- a/drivers/infiniband/hw/cxgb4/id_table.c
+++ b/drivers/infiniband/hw/cxgb4/id_table.c
@@ -59,7 +59,7 @@ u32 c4iw_id_alloc(struct c4iw_id_table *alloc)
 			alloc->last = obj + 1;
 		if (alloc->last >= alloc->max)
 			alloc->last = 0;
-		set_bit(obj, alloc->table);
+		__set_bit(obj, alloc->table);
 		obj += alloc->start;
 	} else
 		obj = -1;
@@ -75,7 +75,7 @@ void c4iw_id_free(struct c4iw_id_table *alloc, u32 obj)
 	obj -= alloc->start;
 
 	spin_lock_irqsave(&alloc->lock, flags);
-	clear_bit(obj, alloc->table);
+	__clear_bit(obj, alloc->table);
 	spin_unlock_irqrestore(&alloc->lock, flags);
 }
 
-- 
2.30.2

Re: [PATCH 0/3] Cleanup and optimize a few bitmap operations

[Jason Gunthorpe]

On Wed, Nov 24, 2021 at 10:30:08PM +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 is more questionable. It replaces calls to '[set|clear]_bit()' by their
> non-atomic '__[set|clear]_bit()' alternatives.
> It looks safe to do so because accesses to the corresponding bitmaps are
> protected by spinlocks.
> However, this patch is compile-tested only. It is not sure that it worth
> changing the code just for saving a few atomic operations.
> So review, test and apply only if it make sense.
> 
> Christophe JAILLET (3):
>   RDMA/cxgb4: Use bitmap_zalloc() when applicable
>   RDMA/cxgb4: Use bitmap_set() when applicable
>   RDMA/cxgb4: Use non-atomic bitmap functions when possible

Applied to for-next, thanks

Jason