Re: [PATCH rdma-core 3/8] mlx4: Add sparse annotations

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3362 bytes --]

On Wed, Jul 12, 2017 at 03:17:37PM -0600, Jason Gunthorpe wrote:
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  CMakeLists.txt          |  4 +--
>  providers/mlx4/cq.c     |  8 +++--
>  providers/mlx4/dbrec.c  |  6 ++--
>  providers/mlx4/mlx4.h   | 16 +++++-----
>  providers/mlx4/mlx4dv.h | 84 ++++++++++++++++++++++++-------------------------
>  providers/mlx4/qp.c     |  4 +--
>  providers/mlx4/verbs.c  | 11 ++-----
>  7 files changed, 64 insertions(+), 69 deletions(-)
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 16196205035f61..1f319390f2e05c 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -414,8 +414,8 @@ add_subdirectory(providers/cxgb3) # NO SPARSE
>  add_subdirectory(providers/cxgb4) # NO SPARSE
>  add_subdirectory(providers/hns) # NO SPARSE
>  add_subdirectory(providers/i40iw) # NO SPARSE
> -add_subdirectory(providers/mlx4) # NO SPARSE
> -add_subdirectory(providers/mlx4/man) # NO SPARSE
> +add_subdirectory(providers/mlx4)
> +add_subdirectory(providers/mlx4/man)
>  add_subdirectory(providers/mlx5) # NO SPARSE
>  add_subdirectory(providers/mlx5/man) # NO SPARSE
>  add_subdirectory(providers/mthca) # NO SPARSE
> diff --git a/providers/mlx4/cq.c b/providers/mlx4/cq.c
> index 50adebb82237f2..afc0e3b8c8eeb1 100644
> --- a/providers/mlx4/cq.c
> +++ b/providers/mlx4/cq.c
> @@ -287,7 +287,7 @@ static inline int mlx4_parse_cqe(struct mlx4_cq *cq,
>  		case MLX4_RECV_OPCODE_SEND_INVAL:
>  			wc->opcode   = IBV_WC_RECV;
>  			wc->wc_flags |= IBV_WC_WITH_INV;
> -			wc->imm_data = be32toh(cqe->immed_rss_invalid);
> +			wc->invalidated_rkey = be32toh(cqe->immed_rss_invalid);
>  			break;
>  		case MLX4_RECV_OPCODE_SEND:
>  			wc->opcode   = IBV_WC_RECV;
> @@ -550,13 +550,15 @@ static uint32_t mlx4_cq_read_wc_vendor_err(struct ibv_cq_ex *ibcq)
>  	return ecqe->vendor_err;
>  }
>
> -static uint32_t mlx4_cq_read_wc_imm_data(struct ibv_cq_ex *ibcq)
> +static __be32 mlx4_cq_read_wc_imm_data(struct ibv_cq_ex *ibcq)
>  {
>  	struct mlx4_cq *cq = to_mcq(ibv_cq_ex_to_cq(ibcq));
>
>  	switch (mlx4dv_get_cqe_opcode(cq->cqe)) {
>  	case MLX4_RECV_OPCODE_SEND_INVAL:
> -		return be32toh(cq->cqe->immed_rss_invalid);
> +		/* This is returning invalidate_rkey which is in host order, see
> +		 * ibv_wc_read_invalidated_rkey */
> +		return (__force __be32)be32toh(cq->cqe->immed_rss_invalid);

Jason,
It is insane construction, convert to host-> force to be32 -> use as uint32_t.

>  	default:
>  		return cq->cqe->immed_rss_invalid;
>  	}
> diff --git a/providers/mlx4/dbrec.c b/providers/mlx4/dbrec.c
> index 3e875738fa61d8..84b01770dcb2c5 100644
> --- a/providers/mlx4/dbrec.c
> +++ b/providers/mlx4/dbrec.c
> @@ -84,7 +84,7 @@ static struct mlx4_db_page *__add_page(struct mlx4_context *context,
>  	return page;
>  }
>
> -uint32_t *mlx4_alloc_db(struct mlx4_context *context, enum mlx4_db_type type)
> +__be32 *mlx4_alloc_db(struct mlx4_context *context, enum mlx4_db_type type)
>  {
>  	struct mlx4_db_page *page;
>  	uint32_t *db = NULL;
> @@ -113,10 +113,10 @@ found:
>  out:
>  	pthread_mutex_unlock(&context->db_list_mutex);
>
> -	return db;
> +	return (__force __be32 *)db;
>  }

I see that librdmacm/rsocket.c full of these __force annotations.
I would be very happy to see it fixed rather suppressed, but don't know
if it is realistic goal or not.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]