diff mbox

[rdma-core,2/2] mlx5: Fix wrong bitmap table allocation size calculation

Message ID 1509282070-1391-3-git-send-email-yishaih@mellanox.com (mailing list archive)
State Accepted
Headers show

Commit Message

Yishai Hadas Oct. 29, 2017, 1:01 p.m. UTC 
From: Ariel Levkovich <lariel@mellanox.com>

This fixes wrong element size calculation during allocation
of bitmap table. We divide the number of required bits in the bitmap
into groups (elements) of sizeof(long) which can be 4 or more bytes
(architecture depended) but allocating num_elements * 4 bytes of
memory for the table.

Signed-off-by: Ariel Levkovich <lariel@mellanox.com>
Reviewed-by: Yishai Hadas <yishaih@mellanox.com>
---
 providers/mlx5/buf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jason Gunthorpe Oct. 29, 2017, 5:37 p.m. UTC | #1 
On Sun, Oct 29, 2017 at 03:01:10PM +0200, Yishai Hadas wrote:

> diff --git a/providers/mlx5/buf.c b/providers/mlx5/buf.c
> index 8196db6..0ba8188 100644
> +++ b/providers/mlx5/buf.c
> @@ -51,7 +51,7 @@ static int mlx5_bitmap_init(struct mlx5_bitmap *bitmap, uint32_t num,
>  	bitmap->avail = num;
>  	bitmap->mask = mask;
>  	bitmap->avail = bitmap->max;
> -	bitmap->table = calloc(BITS_TO_LONGS(bitmap->max), sizeof(uint32_t));
> +	bitmap->table = calloc(BITS_TO_LONGS(bitmap->max), sizeof(long));

bitmap->table is sensibly set to a 'unsigned long' but here, and in
several other related macros, 'long' is used instead.. Looks weird to
mix and match the sigend and unsigned versions.

Generally, I prefer:
	bitmap->table = calloc(BITS_TO_LONGS(bitmap->max),
	                       sizeof(*bitmap->table));

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yishai Hadas Oct. 29, 2017, 9:31 p.m. UTC | #2 
On 10/29/2017 7:37 PM, Jason Gunthorpe wrote:
> On Sun, Oct 29, 2017 at 03:01:10PM +0200, Yishai Hadas wrote:
> 
>> diff --git a/providers/mlx5/buf.c b/providers/mlx5/buf.c
>> index 8196db6..0ba8188 100644
>> +++ b/providers/mlx5/buf.c
>> @@ -51,7 +51,7 @@ static int mlx5_bitmap_init(struct mlx5_bitmap *bitmap, uint32_t num,
>>   	bitmap->avail = num;
>>   	bitmap->mask = mask;
>>   	bitmap->avail = bitmap->max;
>> -	bitmap->table = calloc(BITS_TO_LONGS(bitmap->max), sizeof(uint32_t));
>> +	bitmap->table = calloc(BITS_TO_LONGS(bitmap->max), sizeof(long));
> 
> bitmap->table is sensibly set to a 'unsigned long' but here, and in
> several other related macros, 'long' is used instead.. Looks weird to
> mix and match the sigend and unsigned versions.
> 
> Generally, I prefer:
> 	bitmap->table = calloc(BITS_TO_LONGS(bitmap->max),
> 	                       sizeof(*bitmap->table));
> 

I'm fine with your suggestion, will use as part of V1.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yishai Hadas Oct. 30, 2017, 12:45 p.m. UTC | #3 
On 10/29/2017 7:37 PM, Jason Gunthorpe wrote:
> On Sun, Oct 29, 2017 at 03:01:10PM +0200, Yishai Hadas wrote:
> 
>> diff --git a/providers/mlx5/buf.c b/providers/mlx5/buf.c
>> index 8196db6..0ba8188 100644
>> +++ b/providers/mlx5/buf.c
>> @@ -51,7 +51,7 @@ static int mlx5_bitmap_init(struct mlx5_bitmap *bitmap, uint32_t num,
>>   	bitmap->avail = num;
>>   	bitmap->mask = mask;
>>   	bitmap->avail = bitmap->max;
>> -	bitmap->table = calloc(BITS_TO_LONGS(bitmap->max), sizeof(uint32_t));
>> +	bitmap->table = calloc(BITS_TO_LONGS(bitmap->max), sizeof(long));
> 
> bitmap->table is sensibly set to a 'unsigned long' but here, and in
> several other related macros, 'long' is used instead.. Looks weird to
> mix and match the sigend and unsigned versions.
> 
> Generally, I prefer:
> 	bitmap->table = calloc(BITS_TO_LONGS(bitmap->max),
> 	                       sizeof(*bitmap->table));

PR was updated as of that suggestion:
https://github.com/linux-rdma/rdma-core/pull/235


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/providers/mlx5/buf.c b/providers/mlx5/buf.c
index 8196db6..0ba8188 100644
--- a/providers/mlx5/buf.c
+++ b/providers/mlx5/buf.c
@@ -51,7 +51,7 @@  static int mlx5_bitmap_init(struct mlx5_bitmap *bitmap, uint32_t num,
 	bitmap->avail = num;
 	bitmap->mask = mask;
 	bitmap->avail = bitmap->max;
-	bitmap->table = calloc(BITS_TO_LONGS(bitmap->max), sizeof(uint32_t));
+	bitmap->table = calloc(BITS_TO_LONGS(bitmap->max), sizeof(long));
 	if (!bitmap->table)
 		return -ENOMEM;