[PATCH rdma-core 0/2] mlx5 misc

[PATCH rdma-core 0/2] mlx5 misc

[Yishai Hadas]

This patch set handles few mlx5 driver issues, details below.

The first patch from Majd consider the mmap command to issue to mlx5 kernel
driver based on the question whether Blueflame is used.

The second patch from Ariel fixes wrong size calculation of the bit map table
when the hugepage allocation mode is used.

PR was sent:
https://github.com/linux-rdma/rdma-core/pull/235 

Yishai

Ariel Levkovich (1):
  mlx5: Fix wrong bitmap table allocation size calculation

Majd Dibbiny (1):
  mlx5: Use non-cached mapping when Blueflame is not used

 providers/mlx5/buf.c    |  2 +-
 providers/mlx5/cq.c     |  2 +-
 providers/mlx5/mlx5.c   | 72 +++++++++++++++++++++++++++++++++++++------------
 providers/mlx5/mlx5.h   | 13 +++++++--
 providers/mlx5/mlx5dv.h |  5 ++++
 5 files changed, 73 insertions(+), 21 deletions(-)

-- 
1.8.3.1

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

[PATCH rdma-core 1/2] mlx5: Use non-cached mapping when Blueflame is not used

[Yishai Hadas]

From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The user can avoid using Blueflame by setting the MLX5_SHUT_UP_BF environment
variable. In this case, mapping the UAR to non-cached can save memory barrier
when writing the doorbell.

The knowledge whether a successful non-cached mapping was done can be achieved
by the DV API.

Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 providers/mlx5/cq.c     |  2 +-
 providers/mlx5/mlx5.c   | 72 +++++++++++++++++++++++++++++++++++++------------
 providers/mlx5/mlx5.h   | 13 +++++++--
 providers/mlx5/mlx5dv.h |  5 ++++
 4 files changed, 72 insertions(+), 20 deletions(-)

diff --git a/providers/mlx5/cq.c b/providers/mlx5/cq.c
index de261dd..5459488 100644
--- a/providers/mlx5/cq.c
+++ b/providers/mlx5/cq.c
@@ -1308,7 +1308,7 @@ int mlx5_arm_cq(struct ibv_cq *ibvcq, int solicited)
 	 */
 	mmio_wc_start();
 
-	mmio_write64_be(ctx->uar[0] + MLX5_CQ_DOORBELL, htobe64(doorbell));
+	mmio_write64_be(ctx->uar[0].reg + MLX5_CQ_DOORBELL, htobe64(doorbell));
 
 	mmio_flush_writes();
 
diff --git a/providers/mlx5/mlx5.c b/providers/mlx5/mlx5.c
index 15f258d..d5ed4a0 100644
--- a/providers/mlx5/mlx5.c
+++ b/providers/mlx5/mlx5.c
@@ -684,7 +684,7 @@ static int mlx5dv_get_cq(struct ibv_cq *cq_in,
 	cq_out->cqe_size  = mcq->cqe_sz;
 	cq_out->buf       = mcq->active_buf->buf;
 	cq_out->dbrec     = mcq->dbrec;
-	cq_out->cq_uar	  = mctx->uar[0];
+	cq_out->cq_uar	  = mctx->uar[0].reg;
 
 	mcq->flags	 |= MLX5_CQ_FLAGS_DV_OWNED;
 
@@ -756,15 +756,52 @@ COMPAT_SYMVER_FUNC(mlx5dv_init_obj, 1_0, "MLX5_1.0",
 	return ret;
 }
 
-static off_t get_uar_mmap_offset(int idx, int page_size)
+static off_t get_uar_mmap_offset(int idx, int page_size, int command)
 {
 	off_t offset = 0;
 
-	set_command(MLX5_MMAP_GET_REGULAR_PAGES_CMD, &offset);
+	set_command(command, &offset);
 	set_index(idx, &offset);
+
 	return offset * page_size;
 }
 
+static off_t uar_type_to_cmd(int uar_type)
+{
+	return (uar_type == MLX5_UAR_TYPE_NC) ? MLX5_MMAP_GET_NC_PAGES_CMD :
+		MLX5_MMAP_GET_REGULAR_PAGES_CMD;
+}
+
+static void *mlx5_mmap(struct mlx5_uar_info *uar, int index,
+		       int cmd_fd, int page_size, int uar_type)
+{
+	off_t offset;
+
+	if (uar_type == MLX5_UAR_TYPE_NC) {
+		offset = get_uar_mmap_offset(index, page_size,
+					     MLX5_MMAP_GET_NC_PAGES_CMD);
+		uar->reg = mmap(NULL, page_size, PROT_WRITE, MAP_SHARED,
+				       cmd_fd, offset);
+		if (uar->reg != MAP_FAILED) {
+			uar->type = MLX5_UAR_TYPE_NC;
+			goto out;
+		}
+	}
+
+	/* Backward compatibility for legacy kernels that don't support
+	 * MLX5_MMAP_GET_NC_PAGES_CMD mmap command.
+	 */
+	offset = get_uar_mmap_offset(index, page_size,
+				     MLX5_MMAP_GET_REGULAR_PAGES_CMD);
+	uar->reg = mmap(NULL, page_size, PROT_WRITE, MAP_SHARED,
+			cmd_fd, offset);
+	if (uar->reg != MAP_FAILED)
+		uar->type = MLX5_UAR_TYPE_REGULAR;
+
+out:
+	return uar->reg;
+}
+
 int mlx5dv_set_context_attr(struct ibv_context *ibv_ctx,
 			enum mlx5dv_set_ctx_attr_type type, void *attr)
 {
@@ -808,7 +845,6 @@ static int mlx5_init_context(struct verbs_device *vdev,
 	int				low_lat_uuars;
 	int				gross_uuars;
 	int				j;
-	off_t				offset;
 	struct mlx5_device	       *mdev;
 	struct verbs_context	       *v_ctx;
 	struct ibv_port_attr		port_attr;
@@ -905,13 +941,15 @@ static int mlx5_init_context(struct verbs_device *vdev,
 
 	pthread_mutex_init(&context->db_list_mutex, NULL);
 
+	context->prefer_bf = get_always_bf();
+	context->shut_up_bf = get_shut_up_bf();
+
 	num_sys_page_map = context->tot_uuars / (context->num_uars_per_page * MLX5_NUM_NON_FP_BFREGS_PER_UAR);
 	for (i = 0; i < num_sys_page_map; ++i) {
-		offset = get_uar_mmap_offset(i, page_size);
-		context->uar[i] = mmap(NULL, page_size, PROT_WRITE, MAP_SHARED,
-				       cmd_fd, offset);
-		if (context->uar[i] == MAP_FAILED) {
-			context->uar[i] = NULL;
+		if (mlx5_mmap(&context->uar[i], i, cmd_fd, page_size,
+			      context->shut_up_bf ? MLX5_UAR_TYPE_NC :
+			      MLX5_UAR_TYPE_REGULAR) == MAP_FAILED) {
+			context->uar[i].reg = NULL;
 			goto err_free_bf;
 		}
 	}
@@ -920,7 +958,7 @@ static int mlx5_init_context(struct verbs_device *vdev,
 		for (j = 0; j < context->num_uars_per_page; j++) {
 			for (k = 0; k < NUM_BFREGS_PER_UAR; k++) {
 				bfi = (i * context->num_uars_per_page + j) * NUM_BFREGS_PER_UAR + k;
-				context->bfs[bfi].reg = context->uar[i] + MLX5_ADAPTER_PAGE_SIZE * j +
+				context->bfs[bfi].reg = context->uar[i].reg + MLX5_ADAPTER_PAGE_SIZE * j +
 							MLX5_BF_OFFSET + k * context->bf_reg_size;
 				context->bfs[bfi].need_lock = need_uuar_lock(context, bfi);
 				mlx5_spinlock_init(&context->bfs[bfi].lock);
@@ -928,7 +966,9 @@ static int mlx5_init_context(struct verbs_device *vdev,
 				if (bfi)
 					context->bfs[bfi].buf_size = context->bf_reg_size / 2;
 				context->bfs[bfi].uuarn = bfi;
-				context->bfs[bfi].uar_mmap_offset = get_uar_mmap_offset(i, page_size);
+				context->bfs[bfi].uar_mmap_offset = get_uar_mmap_offset(i,
+											page_size,
+											uar_type_to_cmd(context->uar[i].type));
 			}
 		}
 	}
@@ -941,8 +981,6 @@ static int mlx5_init_context(struct verbs_device *vdev,
 		mlx5_map_internal_clock(mdev, ctx);
 	}
 
-	context->prefer_bf = get_always_bf();
-	context->shut_up_bf = get_shut_up_bf();
 	mlx5_read_env(&vdev->device, context);
 
 	mlx5_spinlock_init(&context->hugetlb_lock);
@@ -988,8 +1026,8 @@ err_free_bf:
 
 err_free:
 	for (i = 0; i < MLX5_MAX_UARS; ++i) {
-		if (context->uar[i])
-			munmap(context->uar[i], page_size);
+		if (context->uar[i].reg)
+			munmap(context->uar[i].reg, page_size);
 	}
 	close_debug_file(context);
 	return errno;
@@ -1004,8 +1042,8 @@ static void mlx5_cleanup_context(struct verbs_device *device,
 
 	free(context->bfs);
 	for (i = 0; i < MLX5_MAX_UARS; ++i) {
-		if (context->uar[i])
-			munmap(context->uar[i], page_size);
+		if (context->uar[i].reg)
+			munmap(context->uar[i].reg, page_size);
 	}
 	if (context->hca_core_clock)
 		munmap(context->hca_core_clock - context->core_clock.offset,
diff --git a/providers/mlx5/mlx5.h b/providers/mlx5/mlx5.h
index 46fce28..66aae3a 100644
--- a/providers/mlx5/mlx5.h
+++ b/providers/mlx5/mlx5.h
@@ -57,7 +57,6 @@ enum {
 };
 
 enum {
-	MLX5_MMAP_GET_REGULAR_PAGES_CMD    = 0,
 	MLX5_MMAP_GET_CONTIGUOUS_PAGES_CMD = 1,
 	MLX5_MMAP_GET_CORE_CLOCK_CMD    = 5
 };
@@ -210,6 +209,16 @@ struct mlx5_spinlock {
 	int				in_use;
 };
 
+enum mlx5_uar_type {
+	MLX5_UAR_TYPE_REGULAR,
+	MLX5_UAR_TYPE_NC,
+};
+
+struct mlx5_uar_info {
+	void				*reg;
+	enum mlx5_uar_type		type;
+};
+
 struct mlx5_context {
 	struct ibv_context		ibv_ctx;
 	int				max_num_qps;
@@ -239,7 +248,7 @@ struct mlx5_context {
 	}				uidx_table[MLX5_UIDX_TABLE_SIZE];
 	pthread_mutex_t                 uidx_table_mutex;
 
-	void			       *uar[MLX5_MAX_UARS];
+	struct mlx5_uar_info		uar[MLX5_MAX_UARS];
 	struct mlx5_db_page	       *db_list;
 	pthread_mutex_t			db_list_mutex;
 	int				cache_line_size;
diff --git a/providers/mlx5/mlx5dv.h b/providers/mlx5/mlx5dv.h
index ffe2c55..9e20a25 100644
--- a/providers/mlx5/mlx5dv.h
+++ b/providers/mlx5/mlx5dv.h
@@ -632,6 +632,11 @@ enum mlx5dv_set_ctx_attr_type {
 	MLX5DV_CTX_ATTR_BUF_ALLOCATORS = 1,
 };
 
+enum {
+	MLX5_MMAP_GET_REGULAR_PAGES_CMD	= 0,
+	MLX5_MMAP_GET_NC_PAGES_CMD	= 3,
+};
+
 struct mlx5dv_ctx_allocators {
 	void *(*alloc)(size_t size, void *priv_data);
 	void (*free)(void *ptr, void *priv_data);
-- 
1.8.3.1

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

Re: [PATCH rdma-core 1/2] mlx5: Use non-cached mapping when Blueflame is not used

[Jason Gunthorpe]

On Sun, Oct 29, 2017 at 03:01:09PM +0200, Yishai Hadas wrote:

> diff --git a/providers/mlx5/mlx5dv.h b/providers/mlx5/mlx5dv.h
> index ffe2c55..9e20a25 100644
> +++ b/providers/mlx5/mlx5dv.h
> @@ -632,6 +632,11 @@ enum mlx5dv_set_ctx_attr_type {
>  	MLX5DV_CTX_ATTR_BUF_ALLOCATORS = 1,
>  };
>  
> +enum {
> +	MLX5_MMAP_GET_REGULAR_PAGES_CMD	= 0,
> +	MLX5_MMAP_GET_NC_PAGES_CMD	= 3,
> +};
> +

These are added to a public header, but I couldn't see where they
would get used by a DV user??

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

Re: [PATCH rdma-core 1/2] mlx5: Use non-cached mapping when Blueflame is not used

[Yishai Hadas]

On 10/29/2017 7:33 PM, Jason Gunthorpe wrote:
> On Sun, Oct 29, 2017 at 03:01:09PM +0200, Yishai Hadas wrote:
> 
>> diff --git a/providers/mlx5/mlx5dv.h b/providers/mlx5/mlx5dv.h
>> index ffe2c55..9e20a25 100644
>> +++ b/providers/mlx5/mlx5dv.h
>> @@ -632,6 +632,11 @@ enum mlx5dv_set_ctx_attr_type {
>>   	MLX5DV_CTX_ATTR_BUF_ALLOCATORS = 1,
>>   };
>>   
>> +enum {
>> +	MLX5_MMAP_GET_REGULAR_PAGES_CMD	= 0,
>> +	MLX5_MMAP_GET_NC_PAGES_CMD	= 3,
>> +};
>> +
> 
> These are added to a public header, but I couldn't see where they
> would get used by a DV user??

A DV user already had access to the mlx5 mmap command before this series 
by using the mlx5dv_get_qp() API:
qp_out->uar_mmap_offset = mqp->bf->uar_mmap_offset;

This offset value was used as part of the mmap() system call and reached 
the mlx5 kernel driver, it encoded the required command.

The enum that was added here, just exposed the optional commands which 
post this series may be issued including the new 
MLX5_MMAP_GET_NC_PAGES_CMD value.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-core 1/2] mlx5: Use non-cached mapping when Blueflame is not used

[Jason Gunthorpe]

On Sun, Oct 29, 2017 at 11:26:04PM +0200, Yishai Hadas wrote:
> >>@@ -632,6 +632,11 @@ enum mlx5dv_set_ctx_attr_type {
> >>  	MLX5DV_CTX_ATTR_BUF_ALLOCATORS = 1,
> >>  };
> >>+enum {
> >>+	MLX5_MMAP_GET_REGULAR_PAGES_CMD	= 0,
> >>+	MLX5_MMAP_GET_NC_PAGES_CMD	= 3,
> >>+};
> >>+
> >
> >These are added to a public header, but I couldn't see where they
> >would get used by a DV user??
> 
> A DV user already had access to the mlx5 mmap command before this series by
> using the mlx5dv_get_qp() API:
> qp_out->uar_mmap_offset = mqp->bf->uar_mmap_offset;

Okay.

As long as existing users can reasonably be expected to handle this
new value in the uar_mmap_offset output field...

Otherwise there is an abi problem..

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

Re: [PATCH rdma-core 1/2] mlx5: Use non-cached mapping when Blueflame is not used

[Yishai Hadas]

On 10/30/2017 4:57 PM, Jason Gunthorpe wrote:
> On Sun, Oct 29, 2017 at 11:26:04PM +0200, Yishai Hadas wrote:
>>>> @@ -632,6 +632,11 @@ enum mlx5dv_set_ctx_attr_type {
>>>>   	MLX5DV_CTX_ATTR_BUF_ALLOCATORS = 1,
>>>>   };
>>>> +enum {
>>>> +	MLX5_MMAP_GET_REGULAR_PAGES_CMD	= 0,
>>>> +	MLX5_MMAP_GET_NC_PAGES_CMD	= 3,
>>>> +};
>>>> +
>>>
>>> These are added to a public header, but I couldn't see where they
>>> would get used by a DV user??
>>
>> A DV user already had access to the mlx5 mmap command before this series by
>> using the mlx5dv_get_qp() API:
>> qp_out->uar_mmap_offset = mqp->bf->uar_mmap_offset;
> 
> Okay.
> 
> As long as existing users can reasonably be expected to handle this
> new value in the uar_mmap_offset output field...
> 

Yes, we expect existing users to work properly.

This output field was exposed by the DV API to enable re-mmap of the UAR 
address by a secondary process by using the same 'uar_mmap_offset' 
regardless its internal command value.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-core 1/2] mlx5: Use non-cached mapping when Blueflame is not used

[Jason Gunthorpe]

On Mon, Oct 30, 2017 at 06:31:12PM +0200, Yishai Hadas wrote:
> On 10/30/2017 4:57 PM, Jason Gunthorpe wrote:
> >On Sun, Oct 29, 2017 at 11:26:04PM +0200, Yishai Hadas wrote:
> >>>>@@ -632,6 +632,11 @@ enum mlx5dv_set_ctx_attr_type {
> >>>>  	MLX5DV_CTX_ATTR_BUF_ALLOCATORS = 1,
> >>>>  };
> >>>>+enum {
> >>>>+	MLX5_MMAP_GET_REGULAR_PAGES_CMD	= 0,
> >>>>+	MLX5_MMAP_GET_NC_PAGES_CMD	= 3,
> >>>>+};
> >>>>+
> >>>
> >>>These are added to a public header, but I couldn't see where they
> >>>would get used by a DV user??
> >>
> >>A DV user already had access to the mlx5 mmap command before this series by
> >>using the mlx5dv_get_qp() API:
> >>qp_out->uar_mmap_offset = mqp->bf->uar_mmap_offset;
> >
> >Okay.
> >
> >As long as existing users can reasonably be expected to handle this
> >new value in the uar_mmap_offset output field...
> >
> 
> Yes, we expect existing users to work properly.
> 
> This output field was exposed by the DV API to enable re-mmap of the UAR
> address by a secondary process by using the same 'uar_mmap_offset'
> regardless its internal command value.

Ah, I see.. Seems OK then.

The values kernel side in drivers/infiniband/hw/mlx5/mlx5_ib.h related
to this need to be moved to a uapi header :|

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

Re: [PATCH rdma-core 1/2] mlx5: Use non-cached mapping when Blueflame is not used

[Yishai Hadas]

On 10/30/2017 6:38 PM, Jason Gunthorpe wrote:
> On Mon, Oct 30, 2017 at 06:31:12PM +0200, Yishai Hadas wrote:
>> On 10/30/2017 4:57 PM, Jason Gunthorpe wrote:
>>> On Sun, Oct 29, 2017 at 11:26:04PM +0200, Yishai Hadas wrote:
>>>>>> @@ -632,6 +632,11 @@ enum mlx5dv_set_ctx_attr_type {
>>>>>>   	MLX5DV_CTX_ATTR_BUF_ALLOCATORS = 1,
>>>>>>   };
>>>>>> +enum {
>>>>>> +	MLX5_MMAP_GET_REGULAR_PAGES_CMD	= 0,
>>>>>> +	MLX5_MMAP_GET_NC_PAGES_CMD	= 3,
>>>>>> +};
>>>>>> +
>>>>>
>>>>> These are added to a public header, but I couldn't see where they
>>>>> would get used by a DV user??
>>>>
>>>> A DV user already had access to the mlx5 mmap command before this series by
>>>> using the mlx5dv_get_qp() API:
>>>> qp_out->uar_mmap_offset = mqp->bf->uar_mmap_offset;
>>>
>>> Okay.
>>>
>>> As long as existing users can reasonably be expected to handle this
>>> new value in the uar_mmap_offset output field...
>>>
>>
>> Yes, we expect existing users to work properly.
>>
>> This output field was exposed by the DV API to enable re-mmap of the UAR
>> address by a secondary process by using the same 'uar_mmap_offset'
>> regardless its internal command value.
> 
> Ah, I see.. Seems OK then.

Thanks, series was merged.

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

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

[Yishai Hadas]

From: Ariel Levkovich <lariel-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 providers/mlx5/buf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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;
 
-- 
1.8.3.1

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

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

[Jason Gunthorpe]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Yishai Hadas]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Yishai Hadas]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html