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

Leon Romanovsky
2017-07-13T07:23:43Z

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