[PATCH rdma-core 0/8] Sparse updates for Mellanox and vmw_pvrdma providers

[PATCH rdma-core 0/8] Sparse updates for Mellanox and vmw_pvrdma providers

[Jason Gunthorpe]

This makes 4 more providers sparse clean

There are two more small changes to the user facing API, around the
invalidate_rkey/imm_data duality, these matches the changes already
made..

Jason Gunthorpe (8):
  verbs: Annoate ibv_wc helpers with endian
  verbs: Annoate ibv_send_wr with endian
  mlx4: Add sparse annotations
  mlx5: Avoid sparse complaints about !!
  mlx5: Add sparse annotations
  mthca: Add sparse annotations
  vmw_pvrdma: Update kernel header
  vmw_pvrdma: Add sparse annotations

 CMakeLists.txt                               | 12 ++--
 buildlib/fixup-include/rdma-vmw_pvrdma-abi.h |  4 +-
 libibverbs/man/ibv_create_cq_ex.3            |  5 +-
 libibverbs/man/ibv_post_send.3               |  7 ++-
 libibverbs/verbs.h                           | 22 +++++++-
 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 +---
 providers/mlx5/cq.c                          | 31 +++++-----
 providers/mlx5/dbrec.c                       |  6 +-
 providers/mlx5/mlx5.h                        | 14 ++---
 providers/mlx5/mlx5dv.h                      | 68 +++++++++++-----------
 providers/mlx5/qp.c                          | 10 ++--
 providers/mlx5/verbs.c                       |  4 +-
 providers/mlx5/wqe.h                         | 34 +++++------
 providers/mthca/cq.c                         | 32 +++++------
 providers/mthca/memfree.c                    |  4 +-
 providers/mthca/mthca.h                      | 24 ++++----
 providers/mthca/qp.c                         |  4 +-
 providers/mthca/wqe.h                        | 60 ++++++++++----------
 providers/vmw_pvrdma/pvrdma.h                |  6 +-
 providers/vmw_pvrdma/verbs.c                 |  7 +--
 25 files changed, 250 insertions(+), 233 deletions(-)

-- 
2.7.4

--
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/8] verbs: Annoate ibv_wc helpers with endian

[Jason Gunthorpe]

This follows the scheme of used in the wc by introducing a
ibv_wc_read_invalidated_rkey to access the host endian
invalidated_rkey value with proper annotations.

This is just an inline wrapper to allow sparse to work sensibly,
not really a good reason to add another driver entry point.

Fixes: 32186550 ("verbs: Add be annotations to public headers")
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libibverbs/man/ibv_create_cq_ex.3 |  5 ++++-
 libibverbs/verbs.h                | 13 +++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/libibverbs/man/ibv_create_cq_ex.3 b/libibverbs/man/ibv_create_cq_ex.3
index 7dfbef28d2413b..e943e0e266c582 100644
--- a/libibverbs/man/ibv_create_cq_ex.3
+++ b/libibverbs/man/ibv_create_cq_ex.3
@@ -104,9 +104,12 @@ Below members and functions are used in order to poll the current completion. Th
 .BI "uint32_t ibv_wc_read_byte_len(struct ibv_cq_ex " "*cq"); \c
  Get the vendor error from the current completion.
 
-.BI "uint32_t ibv_wc_read_imm_data(struct ibv_cq_ex " "*cq"); \c
+.BI "__be32 ibv_wc_read_imm_data(struct ibv_cq_ex " "*cq"); \c
  Get the immediate data field from the current completion.
 
+.BI "uint32_t ibv_wc_read_invalidated_rkey(struct ibv_cq_ex " "*cq"); \c
+ Get the rkey invalided by the SEND_INVAL from the current completion.
+
 .BI "uint32_t ibv_wc_read_qp_num(struct ibv_cq_ex " "*cq"); \c
  Get the QP number field from the current completion.
 
diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h
index 4f0765e0476db8..997ef248b26b62 100644
--- a/libibverbs/verbs.h
+++ b/libibverbs/verbs.h
@@ -1093,7 +1093,7 @@ struct ibv_cq_ex {
 	enum ibv_wc_opcode (*read_opcode)(struct ibv_cq_ex *current);
 	uint32_t (*read_vendor_err)(struct ibv_cq_ex *current);
 	uint32_t (*read_byte_len)(struct ibv_cq_ex *current);
-	uint32_t (*read_imm_data)(struct ibv_cq_ex *current);
+	__be32 (*read_imm_data)(struct ibv_cq_ex *current);
 	uint32_t (*read_qp_num)(struct ibv_cq_ex *current);
 	uint32_t (*read_src_qp)(struct ibv_cq_ex *current);
 	int (*read_wc_flags)(struct ibv_cq_ex *current);
@@ -1141,11 +1141,20 @@ static inline uint32_t ibv_wc_read_byte_len(struct ibv_cq_ex *cq)
 	return cq->read_byte_len(cq);
 }
 
-static inline uint32_t ibv_wc_read_imm_data(struct ibv_cq_ex *cq)
+static inline __be32 ibv_wc_read_imm_data(struct ibv_cq_ex *cq)
 {
 	return cq->read_imm_data(cq);
 }
 
+static inline uint32_t ibv_wc_read_invalidated_rkey(struct ibv_cq_ex *cq)
+{
+#ifdef __CHECKER__
+	return (__attribute__((force)) uint32_t)cq->read_imm_data(cq);
+#else
+	return cq->read_imm_data(cq);
+#endif
+}
+
 static inline uint32_t ibv_wc_read_qp_num(struct ibv_cq_ex *cq)
 {
 	return cq->read_qp_num(cq);
-- 
2.7.4

--
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/8] verbs: Annoate ibv_send_wr with endian

[Jason Gunthorpe]

This follows the scheme of used in the wc by introducing a
transparent union in ibv_send_wr to indicate the invalidate_rkey
is in host endian.

Fixes: 32186550 ("verbs: Add be annotations to public headers")
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 libibverbs/man/ibv_post_send.3 | 7 ++++++-
 libibverbs/verbs.h             | 9 ++++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/libibverbs/man/ibv_post_send.3 b/libibverbs/man/ibv_post_send.3
index ebbbb806882eca..d0ee275329d3ce 100644
--- a/libibverbs/man/ibv_post_send.3
+++ b/libibverbs/man/ibv_post_send.3
@@ -35,7 +35,12 @@ struct ibv_sge         *sg_list;                /* Pointer to the s/g array */
 int                     num_sge;                /* Size of the s/g array */
 enum ibv_wr_opcode      opcode;                 /* Operation type */
 int                     send_flags;             /* Flags of the WR properties */
-uint32_t                imm_data;               /* Immediate data (in network byte order) */
+union {
+.in +8
+__be32                  imm_data;               /* Immediate data (in network byte order) */
+uint32_t                invalidate_rkey;        /* Remote rkey to invalidate */
+.in -8
+};
 union {
 .in +8
 struct {
diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h
index 997ef248b26b62..ed251fd28c0729 100644
--- a/libibverbs/verbs.h
+++ b/libibverbs/verbs.h
@@ -941,7 +941,14 @@ struct ibv_send_wr {
 	int			num_sge;
 	enum ibv_wr_opcode	opcode;
 	int			send_flags;
-	__be32			imm_data;
+	/* When opcode is *_WITH_IMM: Immediate data in network byte order.
+	 * When opcode is *_INV: Stores the rkey to invalidate
+	 */
+	union
+	{
+		__be32			imm_data;
+		uint32_t		invalidate_rkey;
+	};
 	union {
 		struct {
 			uint64_t	remote_addr;
-- 
2.7.4

--
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 3/8] mlx4: Add sparse annotations

[Jason Gunthorpe]

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);
 	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;
 }
 
-void mlx4_free_db(struct mlx4_context *context, enum mlx4_db_type type, uint32_t *db)
+void mlx4_free_db(struct mlx4_context *context, enum mlx4_db_type type, __be32 *db)
 {
 	struct mlx4_db_page *page;
 	uintptr_t ps = to_mdev(context->ibv_ctx.device)->page_size;
diff --git a/providers/mlx4/mlx4.h b/providers/mlx4/mlx4.h
index f38feedddf4705..f83215933e9dce 100644
--- a/providers/mlx4/mlx4.h
+++ b/providers/mlx4/mlx4.h
@@ -159,8 +159,8 @@ struct mlx4_cq {
 	pthread_spinlock_t		lock;
 	uint32_t			cqn;
 	uint32_t			cons_index;
-	uint32_t		       *set_ci_db;
-	uint32_t		       *arm_db;
+	__be32			       *set_ci_db;
+	__be32			       *arm_db;
 	int				arm_sn;
 	int				cqe_size;
 	struct mlx4_qp			*cur_qp;
@@ -179,7 +179,7 @@ struct mlx4_srq {
 	int				wqe_shift;
 	int				head;
 	int				tail;
-	uint32_t		       *db;
+	__be32			       *db;
 	uint16_t			counter;
 	uint8_t				ext_srq;
 };
@@ -202,12 +202,12 @@ struct mlx4_qp {
 	int				max_inline_data;
 	int				buf_size;
 
-	uint32_t			doorbell_qpn;
-	uint32_t			sq_signal_bits;
+	__be32				doorbell_qpn;
+	__be32				sq_signal_bits;
 	int				sq_spare_wqes;
 	struct mlx4_wq			sq;
 
-	uint32_t		       *db;
+	__be32			       *db;
 	struct mlx4_wq			rq;
 
 	uint8_t				link_layer;
@@ -292,8 +292,8 @@ static inline int cleanup_on_fatal(int ret)
 int mlx4_alloc_buf(struct mlx4_buf *buf, size_t size, int page_size);
 void mlx4_free_buf(struct mlx4_buf *buf);
 
-uint32_t *mlx4_alloc_db(struct mlx4_context *context, enum mlx4_db_type type);
-void mlx4_free_db(struct mlx4_context *context, enum mlx4_db_type type, uint32_t *db);
+__be32 *mlx4_alloc_db(struct mlx4_context *context, enum mlx4_db_type type);
+void mlx4_free_db(struct mlx4_context *context, enum mlx4_db_type type, __be32 *db);
 
 int mlx4_query_device(struct ibv_context *context,
 		       struct ibv_device_attr *attr);
diff --git a/providers/mlx4/mlx4dv.h b/providers/mlx4/mlx4dv.h
index ea16ce8c85dcd4..16045ef223e746 100644
--- a/providers/mlx4/mlx4dv.h
+++ b/providers/mlx4/mlx4dv.h
@@ -125,20 +125,20 @@ enum mlx4_cqe_status {
 };
 
 struct mlx4_cqe {
-	uint32_t	vlan_my_qpn;
-	uint32_t	immed_rss_invalid;
-	uint32_t	g_mlpath_rqpn;
+	__be32		vlan_my_qpn;
+	__be32		immed_rss_invalid;
+	__be32		g_mlpath_rqpn;
 	union {
 		struct {
-			uint16_t	sl_vid;
-			uint16_t	rlid;
+			__be16	sl_vid;
+			__be16	rlid;
 		};
-		uint32_t ts_47_16;
+		__be32	ts_47_16;
 	};
-	uint32_t	status;
-	uint32_t	byte_cnt;
-	uint16_t	wqe_index;
-	uint16_t	checksum;
+	__be32		status;
+	__be32		byte_cnt;
+	__be16		wqe_index;
+	__be16		checksum;
 	uint8_t		reserved3;
 	uint8_t		ts_15_8;
 	uint8_t		ts_7_0;
@@ -146,9 +146,9 @@ struct mlx4_cqe {
 };
 
 struct mlx4dv_qp {
-	uint32_t		*rdb;
+	__be32		       *rdb;
 	uint32_t		*sdb;
-	uint32_t		doorbell_qpn;
+	__be32			doorbell_qpn;
 	struct {
 		uint32_t	wqe_cnt;
 		int		wqe_shift;
@@ -173,8 +173,8 @@ struct mlx4dv_cq {
 	} buf;
 	uint32_t			cqe_cnt;
 	uint32_t			cqn;
-	uint32_t		       *set_ci_db;
-	uint32_t		       *arm_db;
+	__be32			       *set_ci_db;
+	__be32			       *arm_db;
 	int				arm_sn;
 	int				cqe_size;
 	uint64_t			comp_mask;
@@ -187,7 +187,7 @@ struct mlx4dv_srq {
 	int				wqe_shift;
 	int				head;
 	int				tail;
-	uint32_t		       *db;
+	__be32			       *db;
 	uint64_t			comp_mask;
 };
 
@@ -284,28 +284,28 @@ enum {
 
 struct mlx4_wqe_local_inval_seg {
 	uint64_t		reserved1;
-	uint32_t		mem_key;
+	__be32			mem_key;
 	uint32_t		reserved2;
 	uint64_t		reserved3[2];
 };
 
 struct mlx4_wqe_bind_seg {
-	uint32_t		flags1;
-	uint32_t		flags2;
-	uint32_t		new_rkey;
-	uint32_t		lkey;
-	uint64_t		addr;
-	uint64_t		length;
+	__be32			flags1;
+	__be32			flags2;
+	__be32			new_rkey;
+	__be32			lkey;
+	__be64			addr;
+	__be64			length;
 };
 
 struct mlx4_wqe_ctrl_seg {
-	uint32_t		owner_opcode;
+	__be32			owner_opcode;
 	union {
 		struct {
 			uint8_t			reserved[3];
 			uint8_t			fence_size;
 		};
-		uint32_t	bf_qpn;
+		__be32		bf_qpn;
 	};
 	/*
 	 * High 24 bits are SRC remote buffer; low 8 bits are flags:
@@ -316,61 +316,61 @@ struct mlx4_wqe_ctrl_seg {
 	 * [1]   SE (solicited event)
 	 * [0]   FL (force loopback)
 	 */
-	uint32_t		srcrb_flags;
+	__be32			srcrb_flags;
 	/*
 	 * imm is immediate data for send/RDMA write w/ immediate;
 	 * also invalidation key for send with invalidate; input
 	 * modifier for WQEs on CCQs.
 	 */
-	uint32_t		imm;
+	__be32			imm;
 };
 
 struct mlx4_av {
-	uint32_t			port_pd;
+	__be32				port_pd;
 	uint8_t				reserved1;
 	uint8_t				g_slid;
-	uint16_t			dlid;
+	__be16				dlid;
 	uint8_t				reserved2;
 	uint8_t				gid_index;
 	uint8_t				stat_rate;
 	uint8_t				hop_limit;
-	uint32_t			sl_tclass_flowlabel;
+	__be32				sl_tclass_flowlabel;
 	uint8_t				dgid[16];
 };
 
 struct mlx4_wqe_datagram_seg {
 	struct mlx4_av		av;
-	uint32_t		dqpn;
-	uint32_t		qkey;
-	uint16_t		vlan;
+	__be32			dqpn;
+	__be32			qkey;
+	__be16			vlan;
 	uint8_t			mac[6];
 };
 
 struct mlx4_wqe_data_seg {
-	uint32_t		byte_count;
-	uint32_t		lkey;
-	uint64_t		addr;
+	__be32			byte_count;
+	__be32			lkey;
+	__be64			addr;
 };
 
 struct mlx4_wqe_inline_seg {
-	uint32_t		byte_count;
+	__be32			byte_count;
 };
 
 struct mlx4_wqe_srq_next_seg {
 	uint16_t		reserved1;
-	uint16_t		next_wqe_index;
+	__be16			next_wqe_index;
 	uint32_t		reserved2[3];
 };
 
 struct mlx4_wqe_raddr_seg {
-	uint64_t		raddr;
-	uint32_t		rkey;
-	uint32_t		reserved;
+	__be64			raddr;
+	__be32			rkey;
+	__be32			reserved;
 };
 
 struct mlx4_wqe_atomic_seg {
-	uint64_t		swap_add;
-	uint64_t		compare;
+	__be64			swap_add;
+	__be64			compare;
 };
 
 /*
diff --git a/providers/mlx4/qp.c b/providers/mlx4/qp.c
index 1be100b9c2abf7..6d5986ec1c2958 100644
--- a/providers/mlx4/qp.c
+++ b/providers/mlx4/qp.c
@@ -304,7 +304,7 @@ int mlx4_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
 			case IBV_WR_LOCAL_INV:
 				ctrl->srcrb_flags |=
 					htobe32(MLX4_WQE_CTRL_STRONG_ORDER);
-				set_local_inv_seg(wqe, wr->imm_data);
+				set_local_inv_seg(wqe, wr->invalidate_rkey);
 				wqe  += sizeof
 					(struct mlx4_wqe_local_inval_seg);
 				size += sizeof
@@ -320,7 +320,7 @@ int mlx4_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
 					(struct mlx4_wqe_bind_seg) / 16;
 				break;
 			case IBV_WR_SEND_WITH_INV:
-				ctrl->imm = htobe32(wr->imm_data);
+				ctrl->imm = htobe32(wr->invalidate_rkey);
 				break;
 
 			default:
diff --git a/providers/mlx4/verbs.c b/providers/mlx4/verbs.c
index 548ac2ac2864f0..6a240c5146092f 100644
--- a/providers/mlx4/verbs.c
+++ b/providers/mlx4/verbs.c
@@ -104,7 +104,7 @@ int mlx4_query_device_ex(struct ibv_context *context,
 
 static int mlx4_read_clock(struct ibv_context *context, uint64_t *cycles)
 {
-	unsigned int clockhi, clocklo, clockhi1;
+	uint32_t clockhi, clocklo, clockhi1;
 	int i;
 	struct mlx4_context *ctx = to_mctx(context);
 
@@ -1123,14 +1123,7 @@ int mlx4_destroy_qp(struct ibv_qp *ibqp)
 
 static int link_local_gid(const union ibv_gid *gid)
 {
-	uint32_t *tmp = (uint32_t *)gid->raw;
-	uint32_t hi = tmp[0];
-	uint32_t lo = tmp[1];
-
-	if (hi == htobe32(0xfe800000) && lo == 0)
-		return 1;
-
-	return 0;
+	return gid->global.subnet_prefix == htobe64(0xfe80000000000000ULL);
 }
 
 static int is_multicast_gid(const union ibv_gid *gid)
-- 
2.7.4

--
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 4/8] mlx5: Avoid sparse complaints about !!

[Jason Gunthorpe]

Sparse says:
 warning: dubious: !x & y

Unclear why sparse thinks this is bad, but casting to C99 bool is the
same as !! and much easier to read.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 providers/mlx5/cq.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/providers/mlx5/cq.c b/providers/mlx5/cq.c
index 500116133c1f9e..9a8d958a9ced68 100644
--- a/providers/mlx5/cq.c
+++ b/providers/mlx5/cq.c
@@ -206,11 +206,12 @@ static inline int handle_responder(struct ibv_wc *wc, struct mlx5_cqe64 *cqe,
 		if (likely(cur_rsc->type == MLX5_RSC_TYPE_QP)) {
 			wq = &qp->rq;
 			if (qp->qp_cap_cache & MLX5_RX_CSUM_VALID)
-				wc->wc_flags |= (!!(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
-						 !!(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
-						(get_cqe_l3_hdr_type(cqe) ==
-						MLX5_CQE_L3_HDR_TYPE_IPV4)) <<
-						IBV_WC_IP_CSUM_OK_SHIFT;
+				wc->wc_flags |=
+				    ((bool)(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
+				     (bool)(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
+				     (get_cqe_l3_hdr_type(cqe) ==
+				      MLX5_CQE_L3_HDR_TYPE_IPV4))
+				    << IBV_WC_IP_CSUM_OK_SHIFT;
 		} else {
 			wq = &(rsc_to_mrwq(cur_rsc)->rq);
 		}
@@ -1105,11 +1106,11 @@ static inline int mlx5_cq_read_wc_flags(struct ibv_cq_ex *ibcq)
 	int wc_flags = 0;
 
 	if (cq->flags & MLX5_CQ_FLAGS_RX_CSUM_VALID)
-		wc_flags = (!!(cq->cqe64->hds_ip_ext & MLX5_CQE_L4_OK) &
-				 !!(cq->cqe64->hds_ip_ext & MLX5_CQE_L3_OK) &
-				 (get_cqe_l3_hdr_type(cq->cqe64) ==
-				  MLX5_CQE_L3_HDR_TYPE_IPV4)) <<
-				IBV_WC_IP_CSUM_OK_SHIFT;
+		wc_flags = ((bool)(cq->cqe64->hds_ip_ext & MLX5_CQE_L4_OK) &
+			    (bool)(cq->cqe64->hds_ip_ext & MLX5_CQE_L3_OK) &
+			    (get_cqe_l3_hdr_type(cq->cqe64) ==
+			     MLX5_CQE_L3_HDR_TYPE_IPV4))
+			   << IBV_WC_IP_CSUM_OK_SHIFT;
 
 	switch (mlx5dv_get_cqe_opcode(cq->cqe64)) {
 	case MLX5_CQE_RESP_WR_IMM:
-- 
2.7.4

--
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 5/8] mlx5: Add sparse annotations

[Jason Gunthorpe]

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 CMakeLists.txt          |  4 +--
 providers/mlx5/cq.c     | 10 +++++---
 providers/mlx5/dbrec.c  |  6 ++---
 providers/mlx5/mlx5.h   | 14 +++++-----
 providers/mlx5/mlx5dv.h | 68 ++++++++++++++++++++++++-------------------------
 providers/mlx5/qp.c     | 10 ++++----
 providers/mlx5/verbs.c  |  4 +--
 providers/mlx5/wqe.h    | 34 ++++++++++++-------------
 8 files changed, 76 insertions(+), 74 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1f319390f2e05c..e57285ae07c9d1 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -416,8 +416,8 @@ add_subdirectory(providers/hns) # NO SPARSE
 add_subdirectory(providers/i40iw) # 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/mlx5)
+add_subdirectory(providers/mlx5/man)
 add_subdirectory(providers/mthca) # NO SPARSE
 add_subdirectory(providers/nes) # NO SPARSE
 add_subdirectory(providers/ocrdma)
diff --git a/providers/mlx5/cq.c b/providers/mlx5/cq.c
index 9a8d958a9ced68..b845127de937d0 100644
--- a/providers/mlx5/cq.c
+++ b/providers/mlx5/cq.c
@@ -246,7 +246,7 @@ static inline int handle_responder(struct ibv_wc *wc, struct mlx5_cqe64 *cqe,
 	case MLX5_CQE_RESP_SEND_INV:
 		wc->opcode = IBV_WC_RECV;
 		wc->wc_flags |= IBV_WC_WITH_INV;
-		wc->imm_data = be32toh(cqe->imm_inval_pkey);
+		wc->invalidated_rkey = be32toh(cqe->imm_inval_pkey);
 		break;
 	}
 	wc->slid	   = be16toh(cqe->slid);
@@ -262,7 +262,7 @@ static inline int handle_responder(struct ibv_wc *wc, struct mlx5_cqe64 *cqe,
 
 static void dump_cqe(FILE *fp, void *buf)
 {
-	uint32_t *p = buf;
+	__be32 *p = buf;
 	int i;
 
 	for (i = 0; i < 16; i += 4)
@@ -1141,13 +1141,15 @@ static inline uint32_t mlx5_cq_read_wc_vendor_err(struct ibv_cq_ex *ibcq)
 	return ecqe->vendor_err_synd;
 }
 
-static inline uint32_t mlx5_cq_read_wc_imm_data(struct ibv_cq_ex *ibcq)
+static inline __be32 mlx5_cq_read_wc_imm_data(struct ibv_cq_ex *ibcq)
 {
 	struct mlx5_cq *cq = to_mcq(ibv_cq_ex_to_cq(ibcq));
 
 	switch (mlx5dv_get_cqe_opcode(cq->cqe64)) {
 	case MLX5_CQE_RESP_SEND_INV:
-		return be32toh(cq->cqe64->imm_inval_pkey);
+		/* This is returning invalidate_rkey which is in host order, see
+		 * ibv_wc_read_invalidated_rkey */
+		return (__force __be32)be32toh(cq->cqe64->imm_inval_pkey);
 	default:
 		return cq->cqe64->imm_inval_pkey;
 	}
diff --git a/providers/mlx5/dbrec.c b/providers/mlx5/dbrec.c
index f346939a8edf2b..4e51857f31bdd6 100644
--- a/providers/mlx5/dbrec.c
+++ b/providers/mlx5/dbrec.c
@@ -80,7 +80,7 @@ static struct mlx5_db_page *__add_page(struct mlx5_context *context)
 	return page;
 }
 
-uint32_t *mlx5_alloc_dbrec(struct mlx5_context *context)
+__be32 *mlx5_alloc_dbrec(struct mlx5_context *context)
 {
 	struct mlx5_db_page *page;
 	uint32_t *db = NULL;
@@ -110,10 +110,10 @@ found:
 out:
 	pthread_mutex_unlock(&context->db_list_mutex);
 
-	return db;
+	return (__force __be32 *)db;
 }
 
-void mlx5_free_db(struct mlx5_context *context, uint32_t *db)
+void mlx5_free_db(struct mlx5_context *context, __be32 *db)
 {
 	struct mlx5_db_page *page;
 	uintptr_t ps = to_mdev(context->ibv_ctx.device)->page_size;
diff --git a/providers/mlx5/mlx5.h b/providers/mlx5/mlx5.h
index 615dea38e4fedd..4c494dda2727f2 100644
--- a/providers/mlx5/mlx5.h
+++ b/providers/mlx5/mlx5.h
@@ -324,7 +324,7 @@ struct mlx5_cq {
 	struct mlx5_spinlock		lock;
 	uint32_t			cqn;
 	uint32_t			cons_index;
-	uint32_t		       *dbrec;
+	__be32			       *dbrec;
 	int				arm_sn;
 	int				cqe_sz;
 	int				resize_cqe_sz;
@@ -352,7 +352,7 @@ struct mlx5_srq {
 	int				wqe_shift;
 	int				head;
 	int				tail;
-	uint32_t		       *db;
+	__be32			       *db;
 	uint16_t			counter;
 	int				wq_sig;
 };
@@ -410,7 +410,7 @@ struct mlx5_qp {
 	uint8_t	                        sq_signal_bits;
 	struct mlx5_wq                  sq;
 
-	uint32_t                       *db;
+	__be32                         *db;
 	struct mlx5_wq                  rq;
 	int                             wq_sig;
 	uint32_t			qp_cap_cache;
@@ -432,9 +432,9 @@ struct mlx5_rwq {
 	struct mlx5_buf buf;
 	int buf_size;
 	struct mlx5_wq rq;
-	uint32_t *db;
+	__be32  *db;
 	void	*pbuff;
-	uint32_t	*recv_db;
+	__be32	*recv_db;
 	int wq_sig;
 };
 
@@ -561,8 +561,8 @@ void mlx5_get_alloc_type(const char *component,
 			 enum mlx5_alloc_type default_alloc_type);
 int mlx5_use_huge(const char *key);
 
-uint32_t *mlx5_alloc_dbrec(struct mlx5_context *context);
-void mlx5_free_db(struct mlx5_context *context, uint32_t *db);
+__be32 *mlx5_alloc_dbrec(struct mlx5_context *context);
+void mlx5_free_db(struct mlx5_context *context, __be32 *db);
 
 int mlx5_query_device(struct ibv_context *context,
 		       struct ibv_device_attr *attr);
diff --git a/providers/mlx5/mlx5dv.h b/providers/mlx5/mlx5dv.h
index a03b1d73ccb544..cff3a10457300f 100644
--- a/providers/mlx5/mlx5dv.h
+++ b/providers/mlx5/mlx5dv.h
@@ -106,7 +106,7 @@ int mlx5dv_query_device(struct ibv_context *ctx_in,
 			struct mlx5dv_context *attrs_out);
 
 struct mlx5dv_qp {
-	uint32_t		*dbrec;
+	__be32			*dbrec;
 	struct {
 		void		*buf;
 		uint32_t	wqe_cnt;
@@ -126,7 +126,7 @@ struct mlx5dv_qp {
 
 struct mlx5dv_cq {
 	void			*buf;
-	uint32_t		*dbrec;
+	__be32			*dbrec;
 	uint32_t		cqe_cnt;
 	uint32_t		cqe_size;
 	void			*uar;
@@ -136,7 +136,7 @@ struct mlx5dv_cq {
 
 struct mlx5dv_srq {
 	void			*buf;
-	uint32_t		*dbrec;
+	__be32			*dbrec;
 	uint32_t		stride;
 	uint32_t		head;
 	uint32_t		tail;
@@ -145,7 +145,7 @@ struct mlx5dv_srq {
 
 struct mlx5dv_rwq {
 	void		*buf;
-	uint32_t	*dbrec;
+	__be32		*dbrec;
 	uint32_t	wqe_cnt;
 	uint32_t	stride;
 	uint64_t	comp_mask;
@@ -285,18 +285,18 @@ struct mlx5_cqe64 {
 	uint8_t		rsvd0[17];
 	uint8_t		ml_path;
 	uint8_t		rsvd20[4];
-	uint16_t	slid;
-	uint32_t	flags_rqpn;
+	__be16		slid;
+	__be32		flags_rqpn;
 	uint8_t		hds_ip_ext;
 	uint8_t		l4_hdr_type_etc;
-	uint16_t	vlan_info;
-	uint32_t	srqn_uidx;
-	uint32_t	imm_inval_pkey;
+	__be16		vlan_info;
+	__be32		srqn_uidx;
+	__be32		imm_inval_pkey;
 	uint8_t		rsvd40[4];
-	uint32_t	byte_cnt;
+	__be32		byte_cnt;
 	__be64		timestamp;
-	uint32_t	sop_drop_qpn;
-	uint16_t	wqe_counter;
+	__be32		sop_drop_qpn;
+	__be16		wqe_counter;
 	uint8_t		signature;
 	uint8_t		op_own;
 };
@@ -372,43 +372,43 @@ enum {
 
 struct mlx5_wqe_srq_next_seg {
 	uint8_t			rsvd0[2];
-	uint16_t		next_wqe_index;
+	__be16			next_wqe_index;
 	uint8_t			signature;
 	uint8_t			rsvd1[11];
 };
 
 struct mlx5_wqe_data_seg {
-	uint32_t		byte_count;
-	uint32_t		lkey;
-	uint64_t		addr;
+	__be32			byte_count;
+	__be32			lkey;
+	__be64			addr;
 };
 
 struct mlx5_wqe_ctrl_seg {
-	uint32_t	opmod_idx_opcode;
-	uint32_t	qpn_ds;
+	__be32		opmod_idx_opcode;
+	__be32		qpn_ds;
 	uint8_t		signature;
 	uint8_t		rsvd[2];
 	uint8_t		fm_ce_se;
-	uint32_t	imm;
+	__be32		imm;
 };
 
 struct mlx5_wqe_av {
 	union {
 		struct {
-			uint32_t	qkey;
-			uint32_t	reserved;
+			__be32		qkey;
+			__be32		reserved;
 		} qkey;
-		uint64_t	dc_key;
+		__be64		dc_key;
 	} key;
-	uint32_t	dqp_dct;
+	__be32		dqp_dct;
 	uint8_t		stat_rate_sl;
 	uint8_t		fl_mlid;
-	uint16_t	rlid;
+	__be16		rlid;
 	uint8_t		reserved0[4];
 	uint8_t		rmac[6];
 	uint8_t		tclass;
 	uint8_t		hop_limit;
-	uint32_t	grh_gid_fl;
+	__be32		grh_gid_fl;
 	uint8_t		rgid[16];
 };
 
@@ -417,14 +417,14 @@ struct mlx5_wqe_datagram_seg {
 };
 
 struct mlx5_wqe_raddr_seg {
-	uint64_t	raddr;
-	uint32_t	rkey;
-	uint32_t	reserved;
+	__be64		raddr;
+	__be32		rkey;
+	__be32		reserved;
 };
 
 struct mlx5_wqe_atomic_seg {
-	uint64_t	swap_add;
-	uint64_t	compare;
+	__be64		swap_add;
+	__be64		compare;
 };
 
 struct mlx5_wqe_inl_data_seg {
@@ -432,12 +432,12 @@ struct mlx5_wqe_inl_data_seg {
 };
 
 struct mlx5_wqe_eth_seg {
-	uint32_t	rsvd0;
+	__be32		rsvd0;
 	uint8_t		cs_flags;
 	uint8_t		rsvd1;
-	uint16_t	mss;
-	uint32_t	rsvd2;
-	uint16_t	inline_hdr_sz;
+	__be16		mss;
+	__be32		rsvd2;
+	__be16		inline_hdr_sz;
 	uint8_t		inline_hdr_start[2];
 	uint8_t		inline_hdr[16];
 };
diff --git a/providers/mlx5/qp.c b/providers/mlx5/qp.c
index e71284e191de00..52da8c15ab28db 100644
--- a/providers/mlx5/qp.c
+++ b/providers/mlx5/qp.c
@@ -249,14 +249,14 @@ static void mlx5_bf_copy(uint64_t *dst, const uint64_t *src, unsigned bytecnt,
 	} while (bytecnt > 0);
 }
 
-static uint32_t send_ieth(struct ibv_send_wr *wr)
+static __be32 send_ieth(struct ibv_send_wr *wr)
 {
 	switch (wr->opcode) {
 	case IBV_WR_SEND_WITH_IMM:
 	case IBV_WR_RDMA_WRITE_WITH_IMM:
 		return wr->imm_data;
 	case IBV_WR_SEND_WITH_INV:
-		return htobe32(wr->imm_data);
+		return htobe32(wr->invalidate_rkey);
 	default:
 		return 0;
 	}
@@ -408,7 +408,7 @@ static inline int copy_eth_inline_headers(struct ibv_qp *ibqp,
 
 #define ALIGN(x, log_a) ((((x) + (1 << (log_a)) - 1)) & ~((1 << (log_a)) - 1))
 
-static inline uint16_t get_klm_octo(int nentries)
+static inline __be16 get_klm_octo(int nentries)
 {
 	return htobe16(ALIGN(nentries, 3) / 2);
 }
@@ -737,7 +737,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
 				struct ibv_mw_bind_info	bind_info = {};
 
 				next_fence = MLX5_WQE_CTRL_INITIATOR_SMALL_FENCE;
-				ctrl->imm = htobe32(wr->imm_data);
+				ctrl->imm = htobe32(wr->invalidate_rkey);
 				err = set_bind_wr(qp, IBV_MW_TYPE_2, 0,
 						  &bind_info, ibqp->qp_num,
 						  &seg, &size);
@@ -782,7 +782,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
 				struct ibv_mw_bind_info	bind_info = {};
 
 				next_fence = MLX5_WQE_CTRL_INITIATOR_SMALL_FENCE;
-				ctrl->imm = htobe32(wr->imm_data);
+				ctrl->imm = htobe32(wr->invalidate_rkey);
 				err = set_bind_wr(qp, IBV_MW_TYPE_2, 0,
 						  &bind_info, ibqp->qp_num,
 						  &seg, &size);
diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c
index c95a4d49f29614..c18adf91ac7563 100644
--- a/providers/mlx5/verbs.c
+++ b/providers/mlx5/verbs.c
@@ -1555,7 +1555,7 @@ int mlx5_modify_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
 	struct mlx5_qp *mqp = to_mqp(qp);
 	struct mlx5_context *context = to_mctx(qp->context);
 	int ret;
-	uint32_t *db;
+	__be32 *db;
 
 	if (mqp->rss_qp)
 		return ENOSYS;
@@ -1638,7 +1638,7 @@ struct ibv_ah *mlx5_create_ah(struct ibv_pd *pd, struct ibv_ah_attr *attr)
 	struct ibv_port_attr port_attr;
 	struct mlx5_ah *ah;
 	uint32_t gid_type;
-	uint32_t tmp;
+	__be32 tmp;
 	uint8_t grh;
 	int is_eth;
 
diff --git a/providers/mlx5/wqe.h b/providers/mlx5/wqe.h
index cbe05d3d673867..063dc9ab9a03f4 100644
--- a/providers/mlx5/wqe.h
+++ b/providers/mlx5/wqe.h
@@ -51,7 +51,7 @@ struct mlx5_eqe_qp_srq {
 };
 
 struct mlx5_wqe_xrc_seg {
-	uint32_t	xrc_srqn;
+	__be32		xrc_srqn;
 	uint8_t		rsvd[12];
 };
 
@@ -89,17 +89,17 @@ enum {
 struct mlx5_wqe_umr_ctrl_seg {
 	uint8_t		flags;
 	uint8_t		rsvd0[3];
-	uint16_t	klm_octowords;
-	uint16_t	translation_offset;
-	uint64_t	mkey_mask;
+	__be16		klm_octowords;
+	__be16		translation_offset;
+	__be64		mkey_mask;
 	uint8_t		rsvd1[32];
 };
 
 struct mlx5_wqe_umr_klm_seg {
 	/* up to 2GB */
-	uint32_t	byte_count;
-	uint32_t	mkey;
-	uint64_t	address;
+	__be32		byte_count;
+	__be32		mkey;
+	__be64		address;
 };
 
 union mlx5_wqe_umr_inline_seg {
@@ -123,17 +123,17 @@ struct mlx5_wqe_mkey_context_seg {
 	uint8_t		reserved1;
 	uint8_t		access_flags;
 	uint8_t		sf;
-	uint32_t	qpn_mkey;
-	uint32_t	reserved2;
-	uint32_t	flags_pd;
-	uint64_t	start_addr;
-	uint64_t	len;
-	uint32_t	bsf_octword_size;
-	uint32_t	reserved3[4];
-	uint32_t	translations_octword_size;
+	__be32		qpn_mkey;
+	__be32		reserved2;
+	__be32		flags_pd;
+	__be64		start_addr;
+	__be64		len;
+	__be32		bsf_octword_size;
+	__be32		reserved3[4];
+	__be32		translations_octword_size;
 	uint8_t		reserved4[3];
 	uint8_t		log_page_size;
-	uint32_t	reserved;
+	__be32		reserved;
 	union mlx5_wqe_umr_inline_seg inseg[0];
 };
 
@@ -183,7 +183,7 @@ struct mlx5_wqe_signature_seg {
 };
 
 struct mlx5_wqe_inline_seg {
-	uint32_t	byte_count;
+	__be32		byte_count;
 };
 
 
-- 
2.7.4

--
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 6/8] mthca: Add sparse annotations

[Jason Gunthorpe]

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 CMakeLists.txt            |  2 +-
 providers/mthca/cq.c      | 32 ++++++++++++-------------
 providers/mthca/memfree.c |  4 ++--
 providers/mthca/mthca.h   | 24 +++++++++----------
 providers/mthca/qp.c      |  4 ++--
 providers/mthca/wqe.h     | 60 +++++++++++++++++++++++------------------------
 6 files changed, 63 insertions(+), 63 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e57285ae07c9d1..c2d6618ebd4601 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -418,7 +418,7 @@ add_subdirectory(providers/mlx4)
 add_subdirectory(providers/mlx4/man)
 add_subdirectory(providers/mlx5)
 add_subdirectory(providers/mlx5/man)
-add_subdirectory(providers/mthca) # NO SPARSE
+add_subdirectory(providers/mthca)
 add_subdirectory(providers/nes) # NO SPARSE
 add_subdirectory(providers/ocrdma)
 add_subdirectory(providers/qedr)
diff --git a/providers/mthca/cq.c b/providers/mthca/cq.c
index 68550410f349af..dd8baca198c3a3 100644
--- a/providers/mthca/cq.c
+++ b/providers/mthca/cq.c
@@ -93,14 +93,14 @@ enum {
 };
 
 struct mthca_cqe {
-	uint32_t	my_qpn;
-	uint32_t	my_ee;
-	uint32_t	rqpn;
-	uint16_t	sl_g_mlpath;
-	uint16_t	rlid;
-	uint32_t	imm_etype_pkey_eec;
-	uint32_t	byte_cnt;
-	uint32_t	wqe;
+	__be32		my_qpn;
+	__be32		my_ee;
+	__be32		rqpn;
+	__be16		sl_g_mlpath;
+	__be16		rlid;
+	__be32		imm_etype_pkey_eec;
+	__be32		byte_cnt;
+	__be32		wqe;
 	uint8_t		opcode;
 	uint8_t		is_send;
 	uint8_t		reserved;
@@ -108,13 +108,13 @@ struct mthca_cqe {
 };
 
 struct mthca_err_cqe {
-	uint32_t	my_qpn;
-	uint32_t	reserved1[3];
+	__be32		my_qpn;
+	__be32		reserved1[3];
 	uint8_t		syndrome;
 	uint8_t		vendor_err;
-	uint16_t	db_cnt;
-	uint32_t	reserved2;
-	uint32_t	wqe;
+	__be16		db_cnt;
+	__be32		reserved2;
+	__be32		wqe;
 	uint8_t		opcode;
 	uint8_t		reserved3[2];
 	uint8_t		owner;
@@ -163,11 +163,11 @@ static inline void update_cons_index(struct mthca_cq *cq, int incr)
 
 static void dump_cqe(void *cqe_ptr)
 {
-	uint32_t *cqe = cqe_ptr;
+	__be32 *cqe = cqe_ptr;
 	int i;
 
 	for (i = 0; i < 8; ++i)
-		printf("  [%2x] %08x\n", i * 4, be32toh(((uint32_t *) cqe)[i]));
+		printf("  [%2x] %08x\n", i * 4, be32toh(cqe[i]));
 }
 
 static int handle_error_cqe(struct mthca_cq *cq,
@@ -177,7 +177,7 @@ static int handle_error_cqe(struct mthca_cq *cq,
 {
 	int err;
 	int dbd;
-	uint32_t new_wqe;
+	__be32 new_wqe;
 
 	if (cqe->syndrome == SYNDROME_LOCAL_QP_OP_ERR) {
 		printf("local QP operation err "
diff --git a/providers/mthca/memfree.c b/providers/mthca/memfree.c
index 25855429b302f0..14edb00afefd3a 100644
--- a/providers/mthca/memfree.c
+++ b/providers/mthca/memfree.c
@@ -56,7 +56,7 @@ struct mthca_db_table {
 };
 
 int mthca_alloc_db(struct mthca_db_table *db_tab, enum mthca_db_type type,
-		   uint32_t **db)
+		   __be32 **db)
 {
 	int i, j, k;
 	int group, start, end, dir;
@@ -140,7 +140,7 @@ out:
 	return ret;
 }
 
-void mthca_set_db_qn(uint32_t *db, enum mthca_db_type type, uint32_t qn)
+void mthca_set_db_qn(__be32 *db, enum mthca_db_type type, uint32_t qn)
 {
 	db[1] = htobe32((qn << 8) | (type << 5));
 }
diff --git a/providers/mthca/mthca.h b/providers/mthca/mthca.h
index d456e6a50c889f..648ed4a25f1ade 100644
--- a/providers/mthca/mthca.h
+++ b/providers/mthca/mthca.h
@@ -134,9 +134,9 @@ struct mthca_cq {
 
 	/* Next fields are mem-free only */
 	int                set_ci_db_index;
-	uint32_t          *set_ci_db;
+	__be32            *set_ci_db;
 	int                arm_db_index;
-	uint32_t          *arm_db;
+	__be32            *arm_db;
 	int                arm_sn;
 };
 
@@ -157,7 +157,7 @@ struct mthca_srq {
 
 	/* Next fields are mem-free only */
 	int           	   db_index;
-	uint32_t      	  *db;
+	__be32      	  *db;
 	uint16_t      	   counter;
 };
 
@@ -174,7 +174,7 @@ struct mthca_wq {
 
 	/* Next fields are mem-free only */
 	int                db_index;
-	uint32_t          *db;
+	__be32		  *db;
 };
 
 struct mthca_qp {
@@ -191,16 +191,16 @@ struct mthca_qp {
 };
 
 struct mthca_av {
-	uint32_t port_pd;
+	__be32   port_pd;
 	uint8_t  reserved1;
 	uint8_t  g_slid;
-	uint16_t dlid;
+	__be16   dlid;
 	uint8_t  reserved2;
 	uint8_t  gid_index;
 	uint8_t  msg_sr;
 	uint8_t  hop_limit;
-	uint32_t sl_tclass_flowlabel;
-	uint32_t dgid[4];
+	__be32   sl_tclass_flowlabel;
+	__be32   dgid[4];
 };
 
 struct mthca_ah {
@@ -215,7 +215,7 @@ static inline unsigned long align(unsigned long val, unsigned long align)
 	return (val + align - 1) & ~(align - 1);
 }
 
-static inline uintptr_t db_align(uint32_t *db)
+static inline uintptr_t db_align(__be32 *db)
 {
 	return (uintptr_t) db & ~((uintptr_t) MTHCA_DB_REC_PAGE_SIZE - 1);
 }
@@ -268,8 +268,8 @@ int mthca_alloc_buf(struct mthca_buf *buf, size_t size, int page_size);
 void mthca_free_buf(struct mthca_buf *buf);
 
 int mthca_alloc_db(struct mthca_db_table *db_tab, enum mthca_db_type type,
-		   uint32_t **db);
-void mthca_set_db_qn(uint32_t *db, enum mthca_db_type type, uint32_t qn);
+		   __be32 **db);
+void mthca_set_db_qn(__be32 *db, enum mthca_db_type type, uint32_t qn);
 void mthca_free_db(struct mthca_db_table *db_tab, enum mthca_db_type type, int db_index);
 struct mthca_db_table *mthca_alloc_db_tab(int uarc_size);
 void mthca_free_db_tab(struct mthca_db_table *db_tab);
@@ -340,7 +340,7 @@ struct mthca_qp *mthca_find_qp(struct mthca_context *ctx, uint32_t qpn);
 int mthca_store_qp(struct mthca_context *ctx, uint32_t qpn, struct mthca_qp *qp);
 void mthca_clear_qp(struct mthca_context *ctx, uint32_t qpn);
 int mthca_free_err_wqe(struct mthca_qp *qp, int is_send,
-		       int index, int *dbd, uint32_t *new_wqe);
+		       int index, int *dbd, __be32 *new_wqe);
 struct ibv_ah *mthca_create_ah(struct ibv_pd *pd, struct ibv_ah_attr *attr);
 int mthca_destroy_ah(struct ibv_ah *ah);
 int mthca_alloc_av(struct mthca_pd *pd, struct ibv_ah_attr *attr,
diff --git a/providers/mthca/qp.c b/providers/mthca/qp.c
index 1907f2b82d6987..f4289032a5d2a9 100644
--- a/providers/mthca/qp.c
+++ b/providers/mthca/qp.c
@@ -852,7 +852,7 @@ int mthca_alloc_qp_buf(struct ibv_pd *pd, struct ibv_qp_cap *cap,
 
 	if (mthca_is_memfree(pd->context)) {
 		struct mthca_data_seg *scatter;
-		uint32_t sz;
+		__be32 sz;
 
 		sz = htobe32((sizeof (struct mthca_next_seg) +
 			    qp->rq.max_gs * sizeof (struct mthca_data_seg)) / 16);
@@ -926,7 +926,7 @@ void mthca_clear_qp(struct mthca_context *ctx, uint32_t qpn)
 }
 
 int mthca_free_err_wqe(struct mthca_qp *qp, int is_send,
-		       int index, int *dbd, uint32_t *new_wqe)
+		       int index, int *dbd, __be32 *new_wqe)
 {
 	struct mthca_next_seg *next;
 
diff --git a/providers/mthca/wqe.h b/providers/mthca/wqe.h
index dfd600a26d6022..e56ed9c26cfb6b 100644
--- a/providers/mthca/wqe.h
+++ b/providers/mthca/wqe.h
@@ -60,57 +60,57 @@ enum {
 };
 
 struct mthca_next_seg {
-	uint32_t	nda_op;	/* [31:6] next WQE [4:0] next opcode */
-	uint32_t	ee_nds;	/* [31:8] next EE  [7] DBD [6] F [5:0] next WQE size */
-	uint32_t	flags;	/* [3] CQ [2] Event [1] Solicit */
-	uint32_t	imm;	/* immediate data */
+	__be32		nda_op;	/* [31:6] next WQE [4:0] next opcode */
+	__be32		ee_nds;	/* [31:8] next EE  [7] DBD [6] F [5:0] next WQE size */
+	__be32		flags;	/* [3] CQ [2] Event [1] Solicit */
+	__be32		imm;	/* immediate data */
 };
 
 struct mthca_tavor_ud_seg {
-	uint32_t	reserved1;
-	uint32_t	lkey;
-	uint64_t	av_addr;
-	uint32_t	reserved2[4];
-	uint32_t	dqpn;
-	uint32_t	qkey;
-	uint32_t	reserved3[2];
+	__be32		reserved1;
+	__be32		lkey;
+	__be64		av_addr;
+	__be32		reserved2[4];
+	__be32		dqpn;
+	__be32		qkey;
+	__be32		reserved3[2];
 };
 
 struct mthca_arbel_ud_seg {
-	uint32_t	av[8];
-	uint32_t	dqpn;
-	uint32_t	qkey;
-	uint32_t	reserved[2];
+	__be32		av[8];
+	__be32		dqpn;
+	__be32		qkey;
+	__be32		reserved[2];
 };
 
 struct mthca_bind_seg {
-	uint32_t	flags;	/* [31] Atomic [30] rem write [29] rem read */
-	uint32_t	reserved;
-	uint32_t	new_rkey;
-	uint32_t	lkey;
-	uint64_t	addr;
-	uint64_t	length;
+	__be32		flags;	/* [31] Atomic [30] rem write [29] rem read */
+	__be32		reserved;
+	__be32		new_rkey;
+	__be32		lkey;
+	__be64		addr;
+	__be64		length;
 };
 
 struct mthca_raddr_seg {
-	uint64_t	raddr;
-	uint32_t	rkey;
-	uint32_t	reserved;
+	__be64		raddr;
+	__be32		rkey;
+	__be32		reserved;
 };
 
 struct mthca_atomic_seg {
-	uint64_t	swap_add;
-	uint64_t	compare;
+	__be64		swap_add;
+	__be64		compare;
 };
 
 struct mthca_data_seg {
-	uint32_t	byte_count;
-	uint32_t	lkey;
-	uint64_t	addr;
+	__be32		byte_count;
+	__be32		lkey;
+	__be64		addr;
 };
 
 struct mthca_inline_seg {
-	uint32_t	byte_count;
+	__be32		byte_count;
 };
 
 #endif /* WQE_H */
-- 
2.7.4

--
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 7/8] vmw_pvrdma: Update kernel header

[Jason Gunthorpe]

For sparse to be clean we need the updated endian annotations.

>From kernel commit cc47dd684ee0 ("IB/vmw_pvrdma: Spare annotate imm_data")

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 buildlib/fixup-include/rdma-vmw_pvrdma-abi.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/buildlib/fixup-include/rdma-vmw_pvrdma-abi.h b/buildlib/fixup-include/rdma-vmw_pvrdma-abi.h
index 5016abc9ee9712..c8c1d2d6df4d52 100644
--- a/buildlib/fixup-include/rdma-vmw_pvrdma-abi.h
+++ b/buildlib/fixup-include/rdma-vmw_pvrdma-abi.h
@@ -222,7 +222,7 @@ struct pvrdma_sq_wqe_hdr {
 	__u32 opcode;		/* operation type */
 	__u32 send_flags;	/* wr flags */
 	union {
-		__u32 imm_data;
+		__be32 imm_data;
 		__u32 invalidate_rkey;
 	} ex;
 	__u32 reserved;
@@ -273,7 +273,7 @@ struct pvrdma_cqe {
 	__u32 opcode;
 	__u32 status;
 	__u32 byte_len;
-	__u32 imm_data;
+	__be32 imm_data;
 	__u32 src_qp;
 	__u32 wc_flags;
 	__u32 vendor_err;
-- 
2.7.4

--
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 8/8] vmw_pvrdma: Add sparse annotations

[Jason Gunthorpe]

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 CMakeLists.txt                | 2 +-
 providers/vmw_pvrdma/pvrdma.h | 6 +++---
 providers/vmw_pvrdma/verbs.c  | 7 +------
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c2d6618ebd4601..1e9b1a363b7436 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -422,7 +422,7 @@ add_subdirectory(providers/mthca)
 add_subdirectory(providers/nes) # NO SPARSE
 add_subdirectory(providers/ocrdma)
 add_subdirectory(providers/qedr)
-add_subdirectory(providers/vmw_pvrdma) # NO SPARSE
+add_subdirectory(providers/vmw_pvrdma)
 endif()
 
 add_subdirectory(providers/hfi1verbs)
diff --git a/providers/vmw_pvrdma/pvrdma.h b/providers/vmw_pvrdma/pvrdma.h
index d4acefcedde154..7840e11ee7266c 100644
--- a/providers/vmw_pvrdma/pvrdma.h
+++ b/providers/vmw_pvrdma/pvrdma.h
@@ -152,7 +152,7 @@ struct pvrdma_qp {
 	struct pvrdma_buf		sbuf;
 	int				max_inline_data;
 	int				buf_size;
-	uint32_t			sq_signal_bits;
+	__be32				sq_signal_bits;
 	int				sq_spare_wqes;
 	struct pvrdma_wq		sq;
 	struct pvrdma_wq		rq;
@@ -210,12 +210,12 @@ static inline struct pvrdma_ah *to_vah(struct ibv_ah *ibah)
 
 static inline void pvrdma_write_uar_qp(void *uar, unsigned value)
 {
-	*(uint32_t *)(uar + PVRDMA_UAR_QP_OFFSET) = htole32(value);
+	*(__le32 *)(uar + PVRDMA_UAR_QP_OFFSET) = htole32(value);
 }
 
 static inline void pvrdma_write_uar_cq(void *uar, unsigned value)
 {
-	*(uint32_t *)(uar + PVRDMA_UAR_CQ_OFFSET) = htole32(value);
+	*(__le32 *)(uar + PVRDMA_UAR_CQ_OFFSET) = htole32(value);
 }
 
 static inline int ibv_send_flags_to_pvrdma(int flags)
diff --git a/providers/vmw_pvrdma/verbs.c b/providers/vmw_pvrdma/verbs.c
index 0a07748da513aa..1bb1634863a6e9 100644
--- a/providers/vmw_pvrdma/verbs.c
+++ b/providers/vmw_pvrdma/verbs.c
@@ -154,12 +154,7 @@ static int is_multicast_gid(const union ibv_gid *gid)
 
 static int is_link_local_gid(const union ibv_gid *gid)
 {
-	uint32_t *hi = (uint32_t *)(gid->raw);
-	uint32_t *lo = (uint32_t *)(gid->raw + 4);
-	if (hi[0] == htobe32(0xfe800000) && lo[0] == 0)
-		return 1;
-
-	return 0;
+	return gid->global.subnet_prefix == htobe64(0xfe80000000000000ULL);
 }
 
 static int is_ipv6_addr_v4mapped(const struct in6_addr *a)
-- 
2.7.4

--
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/8] verbs: Annoate ibv_wc helpers with endian

[Leon Romanovsky]

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

On Wed, Jul 12, 2017 at 03:17:35PM -0600, Jason Gunthorpe wrote:
> This follows the scheme of used in the wc by introducing a
> ibv_wc_read_invalidated_rkey to access the host endian
> invalidated_rkey value with proper annotations.
>
> This is just an inline wrapper to allow sparse to work sensibly,
> not really a good reason to add another driver entry point.
>
> Fixes: 32186550 ("verbs: Add be annotations to public headers")
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  libibverbs/man/ibv_create_cq_ex.3 |  5 ++++-
>  libibverbs/verbs.h                | 13 +++++++++++--
>  2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/libibverbs/man/ibv_create_cq_ex.3 b/libibverbs/man/ibv_create_cq_ex.3
> index 7dfbef28d2413b..e943e0e266c582 100644
> --- a/libibverbs/man/ibv_create_cq_ex.3
> +++ b/libibverbs/man/ibv_create_cq_ex.3
> @@ -104,9 +104,12 @@ Below members and functions are used in order to poll the current completion. Th
>  .BI "uint32_t ibv_wc_read_byte_len(struct ibv_cq_ex " "*cq"); \c
>   Get the vendor error from the current completion.
>
> -.BI "uint32_t ibv_wc_read_imm_data(struct ibv_cq_ex " "*cq"); \c
> +.BI "__be32 ibv_wc_read_imm_data(struct ibv_cq_ex " "*cq"); \c
>   Get the immediate data field from the current completion.
>
> +.BI "uint32_t ibv_wc_read_invalidated_rkey(struct ibv_cq_ex " "*cq"); \c
> + Get the rkey invalided by the SEND_INVAL from the current completion.
> +
>  .BI "uint32_t ibv_wc_read_qp_num(struct ibv_cq_ex " "*cq"); \c
>   Get the QP number field from the current completion.
>
> diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h
> index 4f0765e0476db8..997ef248b26b62 100644
> --- a/libibverbs/verbs.h
> +++ b/libibverbs/verbs.h
> @@ -1093,7 +1093,7 @@ struct ibv_cq_ex {
>  	enum ibv_wc_opcode (*read_opcode)(struct ibv_cq_ex *current);
>  	uint32_t (*read_vendor_err)(struct ibv_cq_ex *current);
>  	uint32_t (*read_byte_len)(struct ibv_cq_ex *current);
> -	uint32_t (*read_imm_data)(struct ibv_cq_ex *current);
> +	__be32 (*read_imm_data)(struct ibv_cq_ex *current);

The functions which use this function call are still uint32_t:
➜  rdma-core git:(master) ✗ grep mlx4_cq_read_wc_imm_data * -rI
providers/mlx4/cq.c:static uint32_t mlx4_cq_read_wc_imm_data(struct ibv_cq_ex *ibcq)
providers/mlx4/cq.c:		cq->ibv_cq.read_imm_data = mlx4_cq_read_wc_imm_data;
➜  rdma-core git:(master) ✗ grep mlx5_cq_read_wc_imm_data * -rI
providers/mlx5/cq.c:static inline uint32_t mlx5_cq_read_wc_imm_data(struct ibv_cq_ex *ibcq)
providers/mlx5/cq.c:		cq->ibv_cq.read_imm_data = mlx5_cq_read_wc_imm_data;

>  	uint32_t (*read_qp_num)(struct ibv_cq_ex *current);
>  	uint32_t (*read_src_qp)(struct ibv_cq_ex *current);
>  	int (*read_wc_flags)(struct ibv_cq_ex *current);
> @@ -1141,11 +1141,20 @@ static inline uint32_t ibv_wc_read_byte_len(struct ibv_cq_ex *cq)
>  	return cq->read_byte_len(cq);
>  }
>
> -static inline uint32_t ibv_wc_read_imm_data(struct ibv_cq_ex *cq)
> +static inline __be32 ibv_wc_read_imm_data(struct ibv_cq_ex *cq)

Doesn't this change break user applications?

>  {
>  	return cq->read_imm_data(cq);
>  }
>
> +static inline uint32_t ibv_wc_read_invalidated_rkey(struct ibv_cq_ex *cq)
> +{
> +#ifdef __CHECKER__
> +	return (__attribute__((force)) uint32_t)cq->read_imm_data(cq);
> +#else
> +	return cq->read_imm_data(cq);
> +#endif
> +}

I don't think that those __CHECKER__ ifdefs should be part of the code.
They are part of infrastructure to support development of library, but
are not required for the user of that library.

> +
>  static inline uint32_t ibv_wc_read_qp_num(struct ibv_cq_ex *cq)
>  {
>  	return cq->read_qp_num(cq);
> --
> 2.7.4
>
> --
> 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

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

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

[Leon Romanovsky]

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

Re: [PATCH rdma-core 4/8] mlx5: Avoid sparse complaints about !!

[Leon Romanovsky]

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

On Wed, Jul 12, 2017 at 03:17:38PM -0600, Jason Gunthorpe wrote:
> Sparse says:
>  warning: dubious: !x & y
>
> Unclear why sparse thinks this is bad, but casting to C99 bool is the
> same as !! and much easier to read.

IMHO, it is because of usage "&" to compare boolean result.

>
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  providers/mlx5/cq.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/providers/mlx5/cq.c b/providers/mlx5/cq.c
> index 500116133c1f9e..9a8d958a9ced68 100644
> --- a/providers/mlx5/cq.c
> +++ b/providers/mlx5/cq.c
> @@ -206,11 +206,12 @@ static inline int handle_responder(struct ibv_wc *wc, struct mlx5_cqe64 *cqe,
>  		if (likely(cur_rsc->type == MLX5_RSC_TYPE_QP)) {
>  			wq = &qp->rq;
>  			if (qp->qp_cap_cache & MLX5_RX_CSUM_VALID)
> -				wc->wc_flags |= (!!(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> -						 !!(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> -						(get_cqe_l3_hdr_type(cqe) ==
> -						MLX5_CQE_L3_HDR_TYPE_IPV4)) <<
> -						IBV_WC_IP_CSUM_OK_SHIFT;
> +				wc->wc_flags |=
> +				    ((bool)(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> +				     (bool)(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &

Those "&" should be &&.

> +				     (get_cqe_l3_hdr_type(cqe) ==
> +				      MLX5_CQE_L3_HDR_TYPE_IPV4))
> +				    << IBV_WC_IP_CSUM_OK_SHIFT;
>  		} else {
>  			wq = &(rsc_to_mrwq(cur_rsc)->rq);
>  		}
> @@ -1105,11 +1106,11 @@ static inline int mlx5_cq_read_wc_flags(struct ibv_cq_ex *ibcq)
>  	int wc_flags = 0;
>
>  	if (cq->flags & MLX5_CQ_FLAGS_RX_CSUM_VALID)
> -		wc_flags = (!!(cq->cqe64->hds_ip_ext & MLX5_CQE_L4_OK) &
> -				 !!(cq->cqe64->hds_ip_ext & MLX5_CQE_L3_OK) &
> -				 (get_cqe_l3_hdr_type(cq->cqe64) ==
> -				  MLX5_CQE_L3_HDR_TYPE_IPV4)) <<
> -				IBV_WC_IP_CSUM_OK_SHIFT;
> +		wc_flags = ((bool)(cq->cqe64->hds_ip_ext & MLX5_CQE_L4_OK) &
> +			    (bool)(cq->cqe64->hds_ip_ext & MLX5_CQE_L3_OK) &
> +			    (get_cqe_l3_hdr_type(cq->cqe64) ==
> +			     MLX5_CQE_L3_HDR_TYPE_IPV4))
> +			   << IBV_WC_IP_CSUM_OK_SHIFT;
>
>  	switch (mlx5dv_get_cqe_opcode(cq->cqe64)) {
>  	case MLX5_CQE_RESP_WR_IMM:
> --
> 2.7.4
>
> --
> 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

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

Re: [PATCH rdma-core 4/8] mlx5: Avoid sparse complaints about !!

[Christoph Hellwig]

On Wed, Jul 12, 2017 at 03:17:38PM -0600, Jason Gunthorpe wrote:
> Sparse says:
>  warning: dubious: !x & y
> 
> Unclear why sparse thinks this is bad, but casting to C99 bool is the
> same as !! and much easier to read.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  providers/mlx5/cq.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/providers/mlx5/cq.c b/providers/mlx5/cq.c
> index 500116133c1f9e..9a8d958a9ced68 100644
> --- a/providers/mlx5/cq.c
> +++ b/providers/mlx5/cq.c
> @@ -206,11 +206,12 @@ static inline int handle_responder(struct ibv_wc *wc, struct mlx5_cqe64 *cqe,
>  		if (likely(cur_rsc->type == MLX5_RSC_TYPE_QP)) {
>  			wq = &qp->rq;
>  			if (qp->qp_cap_cache & MLX5_RX_CSUM_VALID)
> -				wc->wc_flags |= (!!(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> -						 !!(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> -						(get_cqe_l3_hdr_type(cqe) ==
> -						MLX5_CQE_L3_HDR_TYPE_IPV4)) <<
> -						IBV_WC_IP_CSUM_OK_SHIFT;
> +				wc->wc_flags |=
> +				    ((bool)(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> +				     (bool)(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> +				     (get_cqe_l3_hdr_type(cqe) ==
> +				      MLX5_CQE_L3_HDR_TYPE_IPV4))
> +				    << IBV_WC_IP_CSUM_OK_SHIFT;

Meh.  This code is complete crap.  Please factor it out into a little
helper that mere humans can read first.  And then replace the odd ^ used
as && with proper if constructs and all should make much more sense.

Something like:

static bool mlx4_ipv4_csum_ok(sruct mlx5_cqe64 *cqe)
{
	if (get_cqe_l3_hdr_type(cqe) != MLX5_CQE_L3_HDR_TYPE_IPV4)
		return false;
	if (!(cqe->hds_ip_ext & MLX5_CQE_L4_OK))
		return false;
	if (!(cqe->hds_ip_ext & MLX5_CQE_L3_OK))
		return false;
	return true;
}

...

		if (mlx4_ipv4_csum_ok(cqe))
			wc->wc_flags |= 1 << IBV_WC_IP_CSUM_OK_SHIFT;
--
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 3/8] mlx4: Add sparse annotations

[Christoph Hellwig]

> >  	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.

Yes, this doesn't make sense to me - we are swapping it but pretending
it's native?  There's certainly something very odd going on here.

It seems like the ->read_imm_data needs to be split and/or have a
prototype change.  The sad part is that it is an exported ABI, although
one that is almost undocumented.

> >  }
> >
> > -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.

The db local variable in mlx4_alloc_db should be easily changed to a
__be32 pointer - it is derived from a void pointer using pointer
arithmetics.

In general a __foce for endian annotations is a GIANT WARNING sign.
Don't ever add one without a long discussion first, and even then only
in a well documented helper and not randomly all over code.
--
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 3/8] mlx4: Add sparse annotations

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 01:07:52AM -0700, Christoph Hellwig wrote:
> > >  	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.
> 
> Yes, this doesn't make sense to me - we are swapping it but pretending
> it's native?  There's certainly something very odd going on here.

Yes, it is odd.

mlx4_cq_read_wc_imm_data is an ABI entry point, we cannot change it.

The ABI defines it to return a be32 when the wc has immediate data and
a u32 when the wc has an remote invalidated rkey - hence the madness.

My approach is to retain this ABI, and put two user facing wrappers
around the function call that return the correct types depending on
the expected data accessed. Since the function is called '*_imm_data'
I choose to return a be32 to match the choice of name.

> It seems like the ->read_imm_data needs to be split and/or have a
> prototype change.  The sad part is that it is an exported ABI, although
> one that is almost undocumented.

Even if we split it, the existing version must retain the same
behavior and the only way to get such insane behaviar and be sparse
clean is with force right here.

Since we are stuck with that I can't really justify adding another
entry point just for invalidated rkey.. I can't really get rid of the
ifdef CHECKER in the public header with this approach.

> > 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.
> 
> The db local variable in mlx4_alloc_db should be easily changed to a
> __be32 pointer - it is derived from a void pointer using pointer
> arithmetics.

Yeah, OK, I thought I tried that, but yep, it is OK. I will update
both places.

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 4/8] mlx5: Avoid sparse complaints about !!

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 12:51:16AM -0700, Christoph Hellwig wrote:
> >  			if (qp->qp_cap_cache & MLX5_RX_CSUM_VALID)
> > -				wc->wc_flags |= (!!(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> > -						 !!(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> > -						(get_cqe_l3_hdr_type(cqe) ==
> > -						MLX5_CQE_L3_HDR_TYPE_IPV4)) <<
> > -						IBV_WC_IP_CSUM_OK_SHIFT;
> > +				wc->wc_flags |=
> > +				    ((bool)(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> > +				     (bool)(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> > +				     (get_cqe_l3_hdr_type(cqe) ==
> > +				      MLX5_CQE_L3_HDR_TYPE_IPV4))
> > +				    << IBV_WC_IP_CSUM_OK_SHIFT;
> 
> Meh.  This code is complete crap.  Please factor it out into a little
> helper that mere humans can read first.  And then replace the odd ^ used
> as && with proper if constructs and all should make much more sense.

Leon/Christoph:

As far as I could make out, this ugly thing is designed like this for
performance.

The choice of & vs && avoids branching (since && is short circuiting)
and the above boils down to some bitwise maths with no branches.

Other easier to read alternatives have branching and many more
instructions.

I don't want to argue with Mellanox on performance just to fix a
sparse warning, and they use the same construct in their kernel
driver, IIRC.

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/8] verbs: Annoate ibv_wc helpers with endian

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 09:50:57AM +0300, Leon Romanovsky wrote:
 
> The functions which use this function call are still uint32_t:
> ➜  rdma-core git:(master) ✗ grep mlx4_cq_read_wc_imm_data * -rI
> providers/mlx4/cq.c:static uint32_t mlx4_cq_read_wc_imm_data(struct ibv_cq_ex *ibcq)
> providers/mlx4/cq.c:		cq->ibv_cq.read_imm_data = mlx4_cq_read_wc_imm_data;
> ➜  rdma-core git:(master) ✗ grep mlx5_cq_read_wc_imm_data * -rI
> providers/mlx5/cq.c:static inline uint32_t mlx5_cq_read_wc_imm_data(struct ibv_cq_ex *ibcq)
> providers/mlx5/cq.c:		cq->ibv_cq.read_imm_data = mlx5_cq_read_wc_imm_data;

Yes, at this point in the series that is fine, they are converted with
each driver's patch. Without sparse be32 and utin32_t are identical.

> > -static inline uint32_t ibv_wc_read_imm_data(struct ibv_cq_ex *cq)
> > +static inline __be32 ibv_wc_read_imm_data(struct ibv_cq_ex *cq)
> 
> Doesn't this change break user applications?

No, they are identical types.

> > +static inline uint32_t ibv_wc_read_invalidated_rkey(struct ibv_cq_ex *cq)
> > +{
> > +#ifdef __CHECKER__
> > +	return (__attribute__((force)) uint32_t)cq->read_imm_data(cq);
> > +#else
> > +	return cq->read_imm_data(cq);
> > +#endif
> > +}
> 
> I don't think that those __CHECKER__ ifdefs should be part of the code.
> They are part of infrastructure to support development of library, but
> are not required for the user of that library.

I do not see a viable alternative.

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 4/8] mlx5: Avoid sparse complaints about !!

[Bart Van Assche]

On Thu, 2017-07-13 at 11:20 -0600, Jason Gunthorpe wrote:
> On Thu, Jul 13, 2017 at 12:51:16AM -0700, Christoph Hellwig wrote:
> > >  			if (qp->qp_cap_cache & MLX5_RX_CSUM_VALID)
> > > -				wc->wc_flags |= (!!(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> > > -						 !!(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> > > -						(get_cqe_l3_hdr_type(cqe) ==
> > > -						MLX5_CQE_L3_HDR_TYPE_IPV4)) <<
> > > -						IBV_WC_IP_CSUM_OK_SHIFT;
> > > +				wc->wc_flags |=
> > > +				    ((bool)(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> > > +				     (bool)(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> > > +				     (get_cqe_l3_hdr_type(cqe) ==
> > > +				      MLX5_CQE_L3_HDR_TYPE_IPV4))
> > > +				    << IBV_WC_IP_CSUM_OK_SHIFT;
> > 
> > Meh.  This code is complete crap.  Please factor it out into a little
> > helper that mere humans can read first.  And then replace the odd ^ used
> > as && with proper if constructs and all should make much more sense.
> 
> As far as I could make out, this ugly thing is designed like this for
> performance.

Hello Jason,

How about using an expression like the below to avoid that branches get inserted
for testing the MLX5_CQE_L4_OK and MLX5_CQE_L3_OK flags?

(cqe->hds_ip_ext & (MLX5_CQE_L4_OK | MLX5_CQE_L3_OK)) == MLX5_CQE_L4_OK | MLX5_CQE_L3_OK

Bart.--
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 4/8] mlx5: Avoid sparse complaints about !!

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 05:52:06PM +0000, Bart Van Assche wrote:
> On Thu, 2017-07-13 at 11:20 -0600, Jason Gunthorpe wrote:
> > On Thu, Jul 13, 2017 at 12:51:16AM -0700, Christoph Hellwig wrote:
> > > >  			if (qp->qp_cap_cache & MLX5_RX_CSUM_VALID)
> > > > -				wc->wc_flags |= (!!(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> > > > -						 !!(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> > > > -						(get_cqe_l3_hdr_type(cqe) ==
> > > > -						MLX5_CQE_L3_HDR_TYPE_IPV4)) <<
> > > > -						IBV_WC_IP_CSUM_OK_SHIFT;
> > > > +				wc->wc_flags |=
> > > > +				    ((bool)(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> > > > +				     (bool)(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> > > > +				     (get_cqe_l3_hdr_type(cqe) ==
> > > > +				      MLX5_CQE_L3_HDR_TYPE_IPV4))
> > > > +				    << IBV_WC_IP_CSUM_OK_SHIFT;
> > > 
> > > Meh.  This code is complete crap.  Please factor it out into a little
> > > helper that mere humans can read first.  And then replace the odd ^ used
> > > as && with proper if constructs and all should make much more sense.
> > 
> > As far as I could make out, this ugly thing is designed like this for
> > performance.
> 
> Hello Jason,
> 
> How about using an expression like the below to avoid that branches get inserted
> for testing the MLX5_CQE_L4_OK and MLX5_CQE_L3_OK flags?
> 
> (cqe->hds_ip_ext & (MLX5_CQE_L4_OK | MLX5_CQE_L3_OK)) == MLX5_CQE_L4_OK | MLX5_CQE_L3_OK

Yes, thanks, that seems a reasonable middle ground:

diff --git a/providers/mlx5/cq.c b/providers/mlx5/cq.c
index b845127de937d0..0a6274e6878d3c 100644
--- a/providers/mlx5/cq.c
+++ b/providers/mlx5/cq.c
@@ -182,6 +182,15 @@ static inline int handle_responder_lazy(struct mlx5_cq *cq, struct mlx5_cqe64 *c
 	return err;
 }
 
+/* Returns IBV_WC_IP_CSUM_OK or 0 */
+static inline int get_csum_ok(struct mlx5_cqe64 *cqe)
+{
+	return (((cqe->hds_ip_ext & (MLX5_CQE_L4_OK | MLX5_CQE_L3_OK)) ==
+		 (MLX5_CQE_L4_OK | MLX5_CQE_L3_OK)) &
+		(get_cqe_l3_hdr_type(cqe) == MLX5_CQE_L3_HDR_TYPE_IPV4))
+	       << IBV_WC_IP_CSUM_OK_SHIFT;
+}
+
 static inline int handle_responder(struct ibv_wc *wc, struct mlx5_cqe64 *cqe,
 				   struct mlx5_resource *cur_rsc, struct mlx5_srq *srq)
 {
@@ -206,12 +215,7 @@ static inline int handle_responder(struct ibv_wc *wc, struct mlx5_cqe64 *cqe,
 		if (likely(cur_rsc->type == MLX5_RSC_TYPE_QP)) {
 			wq = &qp->rq;
 			if (qp->qp_cap_cache & MLX5_RX_CSUM_VALID)
-				wc->wc_flags |=
-				    ((bool)(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
-				     (bool)(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
-				     (get_cqe_l3_hdr_type(cqe) ==
-				      MLX5_CQE_L3_HDR_TYPE_IPV4))
-				    << IBV_WC_IP_CSUM_OK_SHIFT;
+				wc->wc_flags |= get_csum_ok(cqe);
 		} else {
 			wq = &(rsc_to_mrwq(cur_rsc)->rq);
 		}
@@ -1106,11 +1110,7 @@ static inline int mlx5_cq_read_wc_flags(struct ibv_cq_ex *ibcq)
 	int wc_flags = 0;
 
 	if (cq->flags & MLX5_CQ_FLAGS_RX_CSUM_VALID)
-		wc_flags = ((bool)(cq->cqe64->hds_ip_ext & MLX5_CQE_L4_OK) &
-			    (bool)(cq->cqe64->hds_ip_ext & MLX5_CQE_L3_OK) &
-			    (get_cqe_l3_hdr_type(cq->cqe64) ==
-			     MLX5_CQE_L3_HDR_TYPE_IPV4))
-			   << IBV_WC_IP_CSUM_OK_SHIFT;
+		wc_flags = get_csum_ok(cq->cqe64);
 
 	switch (mlx5dv_get_cqe_opcode(cq->cqe64)) {
 	case MLX5_CQE_RESP_WR_IMM:
--
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 7/8] vmw_pvrdma: Update kernel header

[Adit Ranadive]

On 7/12/17, 2:17 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote:
> For sparse to be clean we need the updated endian annotations.
>
> From kernel commit cc47dd684ee0 ("IB/vmw_pvrdma: Spare annotate imm_data")
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
> buildlib/fixup-include/rdma-vmw_pvrdma-abi.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Thanks!

Acked-by: Adit Ranadive <aditr@vmware.com>

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

[Adit Ranadive]

On 7/12/17, 2:17 PM, "Jason Gunthorpe" <jgunthorpe@obsidianresearch.com> wrote:
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
> CMakeLists.txt                | 2 +-
> providers/vmw_pvrdma/pvrdma.h | 6 +++---
> providers/vmw_pvrdma/verbs.c  | 7 +------
> 3 files changed, 5 insertions(+), 10 deletions(-)

Thanks for adding the sparse support!

Tested-by: Adit Ranadive <aditr@vmware.com>
Acked-by: Adit Ranadive <aditr@vmware.com>

Re: [PATCH rdma-core 4/8] mlx5: Avoid sparse complaints about !!

[Leon Romanovsky]

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

On Thu, Jul 13, 2017 at 12:37:37PM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 13, 2017 at 05:52:06PM +0000, Bart Van Assche wrote:
> > On Thu, 2017-07-13 at 11:20 -0600, Jason Gunthorpe wrote:
> > > On Thu, Jul 13, 2017 at 12:51:16AM -0700, Christoph Hellwig wrote:
> > > > >  			if (qp->qp_cap_cache & MLX5_RX_CSUM_VALID)
> > > > > -				wc->wc_flags |= (!!(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> > > > > -						 !!(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> > > > > -						(get_cqe_l3_hdr_type(cqe) ==
> > > > > -						MLX5_CQE_L3_HDR_TYPE_IPV4)) <<
> > > > > -						IBV_WC_IP_CSUM_OK_SHIFT;
> > > > > +				wc->wc_flags |=
> > > > > +				    ((bool)(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> > > > > +				     (bool)(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> > > > > +				     (get_cqe_l3_hdr_type(cqe) ==
> > > > > +				      MLX5_CQE_L3_HDR_TYPE_IPV4))
> > > > > +				    << IBV_WC_IP_CSUM_OK_SHIFT;
> > > >
> > > > Meh.  This code is complete crap.  Please factor it out into a little
> > > > helper that mere humans can read first.  And then replace the odd ^ used
> > > > as && with proper if constructs and all should make much more sense.
> > >
> > > As far as I could make out, this ugly thing is designed like this for
> > > performance.
> >
> > Hello Jason,
> >
> > How about using an expression like the below to avoid that branches get inserted
> > for testing the MLX5_CQE_L4_OK and MLX5_CQE_L3_OK flags?
> >
> > (cqe->hds_ip_ext & (MLX5_CQE_L4_OK | MLX5_CQE_L3_OK)) == MLX5_CQE_L4_OK | MLX5_CQE_L3_OK
>
> Yes, thanks, that seems a reasonable middle ground:

Thanks Jason and Bart.

>
> diff --git a/providers/mlx5/cq.c b/providers/mlx5/cq.c
> index b845127de937d0..0a6274e6878d3c 100644
> --- a/providers/mlx5/cq.c
> +++ b/providers/mlx5/cq.c
> @@ -182,6 +182,15 @@ static inline int handle_responder_lazy(struct mlx5_cq *cq, struct mlx5_cqe64 *c
>  	return err;
>  }
>
> +/* Returns IBV_WC_IP_CSUM_OK or 0 */
> +static inline int get_csum_ok(struct mlx5_cqe64 *cqe)
> +{
> +	return (((cqe->hds_ip_ext & (MLX5_CQE_L4_OK | MLX5_CQE_L3_OK)) ==
> +		 (MLX5_CQE_L4_OK | MLX5_CQE_L3_OK)) &
> +		(get_cqe_l3_hdr_type(cqe) == MLX5_CQE_L3_HDR_TYPE_IPV4))
> +	       << IBV_WC_IP_CSUM_OK_SHIFT;
> +}
> +
>  static inline int handle_responder(struct ibv_wc *wc, struct mlx5_cqe64 *cqe,
>  				   struct mlx5_resource *cur_rsc, struct mlx5_srq *srq)
>  {
> @@ -206,12 +215,7 @@ static inline int handle_responder(struct ibv_wc *wc, struct mlx5_cqe64 *cqe,
>  		if (likely(cur_rsc->type == MLX5_RSC_TYPE_QP)) {
>  			wq = &qp->rq;
>  			if (qp->qp_cap_cache & MLX5_RX_CSUM_VALID)
> -				wc->wc_flags |=
> -				    ((bool)(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> -				     (bool)(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> -				     (get_cqe_l3_hdr_type(cqe) ==
> -				      MLX5_CQE_L3_HDR_TYPE_IPV4))
> -				    << IBV_WC_IP_CSUM_OK_SHIFT;
> +				wc->wc_flags |= get_csum_ok(cqe);
>  		} else {
>  			wq = &(rsc_to_mrwq(cur_rsc)->rq);
>  		}
> @@ -1106,11 +1110,7 @@ static inline int mlx5_cq_read_wc_flags(struct ibv_cq_ex *ibcq)
>  	int wc_flags = 0;
>
>  	if (cq->flags & MLX5_CQ_FLAGS_RX_CSUM_VALID)
> -		wc_flags = ((bool)(cq->cqe64->hds_ip_ext & MLX5_CQE_L4_OK) &
> -			    (bool)(cq->cqe64->hds_ip_ext & MLX5_CQE_L3_OK) &
> -			    (get_cqe_l3_hdr_type(cq->cqe64) ==
> -			     MLX5_CQE_L3_HDR_TYPE_IPV4))
> -			   << IBV_WC_IP_CSUM_OK_SHIFT;
> +		wc_flags = get_csum_ok(cq->cqe64);
>
>  	switch (mlx5dv_get_cqe_opcode(cq->cqe64)) {
>  	case MLX5_CQE_RESP_WR_IMM:
> --
> 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

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

Re: [PATCH rdma-core 1/8] verbs: Annoate ibv_wc helpers with endian

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 09:50:57AM +0300, Leon Romanovsky wrote:

> > +static inline uint32_t ibv_wc_read_invalidated_rkey(struct ibv_cq_ex *cq)
> > +{
> > +#ifdef __CHECKER__
> > +	return (__attribute__((force)) uint32_t)cq->read_imm_data(cq);
> > +#else
> > +	return cq->read_imm_data(cq);
> > +#endif
> > +}
> 
> I don't think that those __CHECKER__ ifdefs should be part of the code.
> They are part of infrastructure to support development of library, but
> are not required for the user of that library.

I don't see a way to get rid of it here and keep the ABI the same.

__CHECKER__ is safe to use because we already rely on it being defined
sensibly when we include linux/types.h and use __be32 in the public
header.

If we try and drop it post-install or something then we are really
just blocking downstream projects from also using sparse, I don't see
the point in doing that...

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/8] verbs: Annoate ibv_wc helpers with endian

[Leon Romanovsky]

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

On Wed, Jul 19, 2017 at 10:53:51AM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 13, 2017 at 09:50:57AM +0300, Leon Romanovsky wrote:
>
> > > +static inline uint32_t ibv_wc_read_invalidated_rkey(struct ibv_cq_ex *cq)
> > > +{
> > > +#ifdef __CHECKER__
> > > +	return (__attribute__((force)) uint32_t)cq->read_imm_data(cq);
> > > +#else
> > > +	return cq->read_imm_data(cq);
> > > +#endif
> > > +}
> >
> > I don't think that those __CHECKER__ ifdefs should be part of the code.
> > They are part of infrastructure to support development of library, but
> > are not required for the user of that library.
>
> I don't see a way to get rid of it here and keep the ABI the same.
>
> __CHECKER__ is safe to use because we already rely on it being defined
> sensibly when we include linux/types.h and use __be32 in the public
> header.
>
> If we try and drop it post-install or something then we are really
> just blocking downstream projects from also using sparse, I don't see
> the point in doing that...

I'm curious, how many of them know and care about sparse :)

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

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