[PATCH 1/2] RDMA/rxe: Save a few bytes from struct rxe_pool

[PATCH 1/2] RDMA/rxe: Save a few bytes from struct rxe_pool

[Christophe JAILLET]

'table_size' is never read, it can be removed.

In fact, the only place that uses something that could be 'table_size' is
'alloc_index()'. In this function, it is re-computed from 'min_index' and
'max_index'.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/infiniband/sw/rxe/rxe_pool.c | 1 -
 drivers/infiniband/sw/rxe/rxe_pool.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
index 7b4cb46edfd9..271d4ac0e0aa 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.c
+++ b/drivers/infiniband/sw/rxe/rxe_pool.c
@@ -114,7 +114,6 @@ static int rxe_pool_init_index(struct rxe_pool *pool, u32 max, u32 min)
 		goto out;
 	}
 
-	pool->index.table_size = size;
 	bitmap_zero(pool->index.table, max - min + 1);
 
 out:
diff --git a/drivers/infiniband/sw/rxe/rxe_pool.h b/drivers/infiniband/sw/rxe/rxe_pool.h
index 1feca1bffced..1ff2250edf6d 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.h
+++ b/drivers/infiniband/sw/rxe/rxe_pool.h
@@ -74,7 +74,6 @@ struct rxe_pool {
 	struct {
 		struct rb_root		tree;
 		unsigned long		*table;
-		size_t			table_size;
 		u32			last;
 		u32			max_index;
 		u32			min_index;
-- 
2.30.2

[PATCH 2/2] RDMA/rxe: Use 'bitmap_zalloc()' when applicable

[Christophe JAILLET]

'index.table' is a bitmap. So use 'bitmap_zalloc()' to simplify code,
improve the semantic and avoid some open-coded arithmetic in allocator
arguments.

Using 'bitmap_zalloc()' also allows the removal of a now useless
'bitmap_zero()'.

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

Finally, while at it, axe the useless 'bitmap' variable and use
'mem->bitmap' directly.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/infiniband/sw/rxe/rxe_pool.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
index 271d4ac0e0aa..ed2427369c2c 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.c
+++ b/drivers/infiniband/sw/rxe/rxe_pool.c
@@ -96,7 +96,6 @@ static inline const char *pool_name(struct rxe_pool *pool)
 static int rxe_pool_init_index(struct rxe_pool *pool, u32 max, u32 min)
 {
 	int err = 0;
-	size_t size;
 
 	if ((max - min + 1) < pool->max_elem) {
 		pr_warn("not enough indices for max_elem\n");
@@ -107,15 +106,12 @@ static int rxe_pool_init_index(struct rxe_pool *pool, u32 max, u32 min)
 	pool->index.max_index = max;
 	pool->index.min_index = min;
 
-	size = BITS_TO_LONGS(max - min + 1) * sizeof(long);
-	pool->index.table = kmalloc(size, GFP_KERNEL);
+	pool->index.table = bitmap_zalloc(max - min + 1, GFP_KERNEL);
 	if (!pool->index.table) {
 		err = -ENOMEM;
 		goto out;
 	}
 
-	bitmap_zero(pool->index.table, max - min + 1);
-
 out:
 	return err;
 }
@@ -167,7 +163,7 @@ void rxe_pool_cleanup(struct rxe_pool *pool)
 		pr_warn("%s pool destroyed with unfree'd elem\n",
 			pool_name(pool));
 
-	kfree(pool->index.table);
+	bitmap_free(pool->index.table);
 }
 
 static u32 alloc_index(struct rxe_pool *pool)
-- 
2.30.2

Re: [PATCH 2/2] RDMA/rxe: Use 'bitmap_zalloc()' when applicable

[Christophe JAILLET]

Le 24/10/2021 à 18:43, Christophe JAILLET a écrit :
> 'index.table' is a bitmap. So use 'bitmap_zalloc()' to simplify code,
> improve the semantic and avoid some open-coded arithmetic in allocator
> arguments.
> 
> Using 'bitmap_zalloc()' also allows the removal of a now useless
> 'bitmap_zero()'.
> 
> Also change the corresponding 'kfree()' into 'bitmap_free()' to keep
> consistency.
> 
> Finally, while at it, axe the useless 'bitmap' variable and use
> 'mem->bitmap' directly.

This last sentence should not be there (cut'n'paste error)
It should be removed when the patch is applied, or I can resend if needed.

CJ

> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>   drivers/infiniband/sw/rxe/rxe_pool.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
> index 271d4ac0e0aa..ed2427369c2c 100644
> --- a/drivers/infiniband/sw/rxe/rxe_pool.c
> +++ b/drivers/infiniband/sw/rxe/rxe_pool.c
> @@ -96,7 +96,6 @@ static inline const char *pool_name(struct rxe_pool *pool)
>   static int rxe_pool_init_index(struct rxe_pool *pool, u32 max, u32 min)
>   {
>   	int err = 0;
> -	size_t size;
>   
>   	if ((max - min + 1) < pool->max_elem) {
>   		pr_warn("not enough indices for max_elem\n");
> @@ -107,15 +106,12 @@ static int rxe_pool_init_index(struct rxe_pool *pool, u32 max, u32 min)
>   	pool->index.max_index = max;
>   	pool->index.min_index = min;
>   
> -	size = BITS_TO_LONGS(max - min + 1) * sizeof(long);
> -	pool->index.table = kmalloc(size, GFP_KERNEL);
> +	pool->index.table = bitmap_zalloc(max - min + 1, GFP_KERNEL);
>   	if (!pool->index.table) {
>   		err = -ENOMEM;
>   		goto out;
>   	}
>   
> -	bitmap_zero(pool->index.table, max - min + 1);
> -
>   out:
>   	return err;
>   }
> @@ -167,7 +163,7 @@ void rxe_pool_cleanup(struct rxe_pool *pool)
>   		pr_warn("%s pool destroyed with unfree'd elem\n",
>   			pool_name(pool));
>   
> -	kfree(pool->index.table);
> +	bitmap_free(pool->index.table);
>   }
>   
>   static u32 alloc_index(struct rxe_pool *pool)
> 

[PATCH] RDMA/rxe: Make rxe_type_info static const

[Joe Perches]

Make struct rxe_type_info static const and local to the only uses.
Moves a bit of data to text.

$ size drivers/infiniband/sw/rxe/rxe_pool.o* (defconfig w/ infiniband swe)
   text	   data	    bss	    dec	    hex	filename
   4456	     12	      0	   4468	   1174	drivers/infiniband/sw/rxe/rxe_pool.o.new
   3817	    652	      0	   4469	   1175	drivers/infiniband/sw/rxe/rxe_pool.o.old

Signed-off-by: Joe Perches <joe@perches.com>
---

On Sun, 2021-10-24 at 18:43 +0200, Christophe JAILLET wrote:
> 'table_size' is never read, it can be removed.
> 
> In fact, the only place that uses something that could be 'table_size' is
> 'alloc_index()'. In this function, it is re-computed from 'min_index' and
> 'max_index'.

 drivers/infiniband/sw/rxe/rxe_pool.c | 28 ++++++++++++++++++----------
 drivers/infiniband/sw/rxe/rxe_pool.h | 14 --------------
 2 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
index 7b4cb46edfd9d..8ba2de0ca2faa 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.c
+++ b/drivers/infiniband/sw/rxe/rxe_pool.c
@@ -7,9 +7,17 @@
 #include "rxe.h"
 #include "rxe_loc.h"
 
-/* info about object pools
- */
-struct rxe_type_info rxe_type_info[RXE_NUM_TYPES] = {
+static const struct rxe_type_info {
+	const char		*name;
+	size_t			size;
+	size_t			elem_offset;
+	void			(*cleanup)(struct rxe_pool_entry *obj);
+	enum rxe_pool_flags	flags;
+	u32			min_index;
+	u32			max_index;
+	size_t			key_offset;
+	size_t			key_size;
+} rxe_type_info[RXE_NUM_TYPES] = {
 	[RXE_TYPE_UC] = {
 		.name		= "rxe-uc",
 		.size		= sizeof(struct rxe_ucontext),
@@ -60,8 +68,8 @@ struct rxe_type_info rxe_type_info[RXE_NUM_TYPES] = {
 		.elem_offset	= offsetof(struct rxe_mr, pelem),
 		.cleanup	= rxe_mr_cleanup,
 		.flags		= RXE_POOL_INDEX,
-		.max_index	= RXE_MAX_MR_INDEX,
 		.min_index	= RXE_MIN_MR_INDEX,
+		.max_index	= RXE_MAX_MR_INDEX,
 	},
 	[RXE_TYPE_MW] = {
 		.name		= "rxe-mw",
@@ -69,8 +77,8 @@ struct rxe_type_info rxe_type_info[RXE_NUM_TYPES] = {
 		.elem_offset	= offsetof(struct rxe_mw, pelem),
 		.cleanup	= rxe_mw_cleanup,
 		.flags		= RXE_POOL_INDEX | RXE_POOL_NO_ALLOC,
-		.max_index	= RXE_MAX_MW_INDEX,
 		.min_index	= RXE_MIN_MW_INDEX,
+		.max_index	= RXE_MAX_MW_INDEX,
 	},
 	[RXE_TYPE_MC_GRP] = {
 		.name		= "rxe-mc_grp",
@@ -329,7 +337,7 @@ void __rxe_drop_index(struct rxe_pool_entry *elem)
 
 void *rxe_alloc_locked(struct rxe_pool *pool)
 {
-	struct rxe_type_info *info = &rxe_type_info[pool->type];
+	const struct rxe_type_info *info = &rxe_type_info[pool->type];
 	struct rxe_pool_entry *elem;
 	u8 *obj;
 
@@ -354,7 +362,7 @@ void *rxe_alloc_locked(struct rxe_pool *pool)
 
 void *rxe_alloc(struct rxe_pool *pool)
 {
-	struct rxe_type_info *info = &rxe_type_info[pool->type];
+	const struct rxe_type_info *info = &rxe_type_info[pool->type];
 	struct rxe_pool_entry *elem;
 	u8 *obj;
 
@@ -397,7 +405,7 @@ void rxe_elem_release(struct kref *kref)
 	struct rxe_pool_entry *elem =
 		container_of(kref, struct rxe_pool_entry, ref_cnt);
 	struct rxe_pool *pool = elem->pool;
-	struct rxe_type_info *info = &rxe_type_info[pool->type];
+	const struct rxe_type_info *info = &rxe_type_info[pool->type];
 	u8 *obj;
 
 	if (pool->cleanup)
@@ -413,7 +421,7 @@ void rxe_elem_release(struct kref *kref)
 
 void *rxe_pool_get_index_locked(struct rxe_pool *pool, u32 index)
 {
-	struct rxe_type_info *info = &rxe_type_info[pool->type];
+	const struct rxe_type_info *info = &rxe_type_info[pool->type];
 	struct rb_node *node;
 	struct rxe_pool_entry *elem;
 	u8 *obj;
@@ -455,7 +463,7 @@ void *rxe_pool_get_index(struct rxe_pool *pool, u32 index)
 
 void *rxe_pool_get_key_locked(struct rxe_pool *pool, void *key)
 {
-	struct rxe_type_info *info = &rxe_type_info[pool->type];
+	const struct rxe_type_info *info = &rxe_type_info[pool->type];
 	struct rb_node *node;
 	struct rxe_pool_entry *elem;
 	u8 *obj;
diff --git a/drivers/infiniband/sw/rxe/rxe_pool.h b/drivers/infiniband/sw/rxe/rxe_pool.h
index 1feca1bffcedf..05c036eb52acb 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.h
+++ b/drivers/infiniband/sw/rxe/rxe_pool.h
@@ -32,20 +32,6 @@ enum rxe_elem_type {
 
 struct rxe_pool_entry;
 
-struct rxe_type_info {
-	const char		*name;
-	size_t			size;
-	size_t			elem_offset;
-	void			(*cleanup)(struct rxe_pool_entry *obj);
-	enum rxe_pool_flags	flags;
-	u32			max_index;
-	u32			min_index;
-	size_t			key_offset;
-	size_t			key_size;
-};
-
-extern struct rxe_type_info rxe_type_info[];
-
 struct rxe_pool_entry {
 	struct rxe_pool		*pool;
 	struct kref		ref_cnt;

Re: [PATCH] RDMA/rxe: Make rxe_type_info static const

[Jason Gunthorpe]

On Sun, Oct 24, 2021 at 10:13:14AM -0700, Joe Perches wrote:
> Make struct rxe_type_info static const and local to the only uses.
> Moves a bit of data to text.
> 
> $ size drivers/infiniband/sw/rxe/rxe_pool.o* (defconfig w/ infiniband swe)
>    text	   data	    bss	    dec	    hex	filename
>    4456	     12	      0	   4468	   1174	drivers/infiniband/sw/rxe/rxe_pool.o.new
>    3817	    652	      0	   4469	   1175	drivers/infiniband/sw/rxe/rxe_pool.o.old
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_pool.c | 28 ++++++++++++++++++----------
>  drivers/infiniband/sw/rxe/rxe_pool.h | 14 --------------
>  2 files changed, 18 insertions(+), 24 deletions(-)

Applied to for-next, thanks

Jason

Re: [PATCH 1/2] RDMA/rxe: Save a few bytes from struct rxe_pool

[Jason Gunthorpe]

On Sun, Oct 24, 2021 at 06:43:30PM +0200, Christophe JAILLET wrote:
> 'table_size' is never read, it can be removed.
> 
> In fact, the only place that uses something that could be 'table_size' is
> 'alloc_index()'. In this function, it is re-computed from 'min_index' and
> 'max_index'.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>  drivers/infiniband/sw/rxe/rxe_pool.c | 1 -
>  drivers/infiniband/sw/rxe/rxe_pool.h | 1 -
>  2 files changed, 2 deletions(-)

This and the next applied to for-next, thanks

Jason