diff mbox series

[for-next,v12,01/11] RDMA/rxe: Replace #define by enum

Message ID 20220318015514.231621-2-rpearsonhpe@gmail.com (mailing list archive)
State Superseded
Delegated to: Jason Gunthorpe
Headers show
Series Fix race conditions in rxe_pool | expand

Commit Message

Bob Pearson March 18, 2022, 1:55 a.m. UTC
Currently the #define IB_SRQ_INIT_MASK is used to distinguish
the rxe_create_srq verb from the rxe_modify_srq verb so that some code
can be shared between these two subroutines.

This commit replaces the #define with an enum which extends
enum ib_srq_attr_mask and makes related changes to prototypes
to clean up type warnings. The parameter is given a rxe specific
name.

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_loc.h   |  8 ++------
 drivers/infiniband/sw/rxe/rxe_srq.c   | 10 +++++-----
 drivers/infiniband/sw/rxe/rxe_verbs.c |  7 ++++---
 drivers/infiniband/sw/rxe/rxe_verbs.h |  6 ++++++
 4 files changed, 17 insertions(+), 14 deletions(-)

Comments

Jason Gunthorpe March 18, 2022, 6 p.m. UTC | #1
On Thu, Mar 17, 2022 at 08:55:04PM -0500, Bob Pearson wrote:
> Currently the #define IB_SRQ_INIT_MASK is used to distinguish
> the rxe_create_srq verb from the rxe_modify_srq verb so that some code
> can be shared between these two subroutines.
> 
> This commit replaces the #define with an enum which extends
> enum ib_srq_attr_mask and makes related changes to prototypes
> to clean up type warnings. The parameter is given a rxe specific
> name.
> 
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
>  drivers/infiniband/sw/rxe/rxe_loc.h   |  8 ++------
>  drivers/infiniband/sw/rxe/rxe_srq.c   | 10 +++++-----
>  drivers/infiniband/sw/rxe/rxe_verbs.c |  7 ++++---
>  drivers/infiniband/sw/rxe/rxe_verbs.h |  6 ++++++
>  4 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index 2ffbe3390668..9067d3b6f1ee 100644
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -159,17 +159,13 @@ void retransmit_timer(struct timer_list *t);
>  void rnr_nak_timer(struct timer_list *t);
>  
>  /* rxe_srq.c */
> -#define IB_SRQ_INIT_MASK (~IB_SRQ_LIMIT)
> -
>  int rxe_srq_chk_attr(struct rxe_dev *rxe, struct rxe_srq *srq,
> -		     struct ib_srq_attr *attr, enum ib_srq_attr_mask mask);
> -
> +		     struct ib_srq_attr *attr, enum rxe_srq_attr_mask
> mask);

It is very old code that is passing around a enum to indicate a
bitfield, this is not the correct way.

Please just change these to unsigned int.

> +enum rxe_srq_attr_mask {
> +	RXE_SRQ_MAX_WR		= IB_SRQ_MAX_WR,
> +	RXE_SRQ_LIMIT		= IB_SRQ_LIMIT,
> +	RXE_SRQ_INIT		= BIT(2),
> +};

But this seems pretty dodgy?

Why not add another parameter, or maybe split the function or
something?

Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 2ffbe3390668..9067d3b6f1ee 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -159,17 +159,13 @@  void retransmit_timer(struct timer_list *t);
 void rnr_nak_timer(struct timer_list *t);
 
 /* rxe_srq.c */
-#define IB_SRQ_INIT_MASK (~IB_SRQ_LIMIT)
-
 int rxe_srq_chk_attr(struct rxe_dev *rxe, struct rxe_srq *srq,
-		     struct ib_srq_attr *attr, enum ib_srq_attr_mask mask);
-
+		     struct ib_srq_attr *attr, enum rxe_srq_attr_mask mask);
 int rxe_srq_from_init(struct rxe_dev *rxe, struct rxe_srq *srq,
 		      struct ib_srq_init_attr *init, struct ib_udata *udata,
 		      struct rxe_create_srq_resp __user *uresp);
-
 int rxe_srq_from_attr(struct rxe_dev *rxe, struct rxe_srq *srq,
-		      struct ib_srq_attr *attr, enum ib_srq_attr_mask mask,
+		      struct ib_srq_attr *attr, enum rxe_srq_attr_mask mask,
 		      struct rxe_modify_srq_cmd *ucmd, struct ib_udata *udata);
 
 void rxe_dealloc(struct ib_device *ib_dev);
diff --git a/drivers/infiniband/sw/rxe/rxe_srq.c b/drivers/infiniband/sw/rxe/rxe_srq.c
index 0c0721f04357..862aa749c93a 100644
--- a/drivers/infiniband/sw/rxe/rxe_srq.c
+++ b/drivers/infiniband/sw/rxe/rxe_srq.c
@@ -10,14 +10,14 @@ 
 #include "rxe_queue.h"
 
 int rxe_srq_chk_attr(struct rxe_dev *rxe, struct rxe_srq *srq,
-		     struct ib_srq_attr *attr, enum ib_srq_attr_mask mask)
+		     struct ib_srq_attr *attr, enum rxe_srq_attr_mask mask)
 {
 	if (srq && srq->error) {
 		pr_warn("srq in error state\n");
 		goto err1;
 	}
 
-	if (mask & IB_SRQ_MAX_WR) {
+	if (mask & RXE_SRQ_MAX_WR) {
 		if (attr->max_wr > rxe->attr.max_srq_wr) {
 			pr_warn("max_wr(%d) > max_srq_wr(%d)\n",
 				attr->max_wr, rxe->attr.max_srq_wr);
@@ -39,7 +39,7 @@  int rxe_srq_chk_attr(struct rxe_dev *rxe, struct rxe_srq *srq,
 			attr->max_wr = RXE_MIN_SRQ_WR;
 	}
 
-	if (mask & IB_SRQ_LIMIT) {
+	if (mask & RXE_SRQ_LIMIT) {
 		if (attr->srq_limit > rxe->attr.max_srq_wr) {
 			pr_warn("srq_limit(%d) > max_srq_wr(%d)\n",
 				attr->srq_limit, rxe->attr.max_srq_wr);
@@ -54,7 +54,7 @@  int rxe_srq_chk_attr(struct rxe_dev *rxe, struct rxe_srq *srq,
 		}
 	}
 
-	if (mask == IB_SRQ_INIT_MASK) {
+	if (mask == RXE_SRQ_INIT) {
 		if (attr->max_sge > rxe->attr.max_srq_sge) {
 			pr_warn("max_sge(%d) > max_srq_sge(%d)\n",
 				attr->max_sge, rxe->attr.max_srq_sge);
@@ -122,7 +122,7 @@  int rxe_srq_from_init(struct rxe_dev *rxe, struct rxe_srq *srq,
 }
 
 int rxe_srq_from_attr(struct rxe_dev *rxe, struct rxe_srq *srq,
-		      struct ib_srq_attr *attr, enum ib_srq_attr_mask mask,
+		      struct ib_srq_attr *attr, enum rxe_srq_attr_mask mask,
 		      struct rxe_modify_srq_cmd *ucmd, struct ib_udata *udata)
 {
 	int err;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index 67184b0281a0..5609956d2bc3 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -7,8 +7,8 @@ 
 #include <linux/dma-mapping.h>
 #include <net/addrconf.h>
 #include <rdma/uverbs_ioctl.h>
+
 #include "rxe.h"
-#include "rxe_loc.h"
 #include "rxe_queue.h"
 #include "rxe_hw_counters.h"
 
@@ -295,7 +295,7 @@  static int rxe_create_srq(struct ib_srq *ibsrq, struct ib_srq_init_attr *init,
 		uresp = udata->outbuf;
 	}
 
-	err = rxe_srq_chk_attr(rxe, NULL, &init->attr, IB_SRQ_INIT_MASK);
+	err = rxe_srq_chk_attr(rxe, NULL, &init->attr, RXE_SRQ_INIT);
 	if (err)
 		goto err1;
 
@@ -320,13 +320,14 @@  static int rxe_create_srq(struct ib_srq *ibsrq, struct ib_srq_init_attr *init,
 }
 
 static int rxe_modify_srq(struct ib_srq *ibsrq, struct ib_srq_attr *attr,
-			  enum ib_srq_attr_mask mask,
+			  enum ib_srq_attr_mask ibmask,
 			  struct ib_udata *udata)
 {
 	int err;
 	struct rxe_srq *srq = to_rsrq(ibsrq);
 	struct rxe_dev *rxe = to_rdev(ibsrq->device);
 	struct rxe_modify_srq_cmd ucmd = {};
+	enum rxe_srq_attr_mask mask = (enum rxe_srq_attr_mask)ibmask;
 
 	if (udata) {
 		if (udata->inlen < sizeof(ucmd))
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index e7eff1ca75e9..34aa013c7801 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -93,6 +93,12 @@  struct rxe_rq {
 	struct rxe_queue	*queue;
 };
 
+enum rxe_srq_attr_mask {
+	RXE_SRQ_MAX_WR		= IB_SRQ_MAX_WR,
+	RXE_SRQ_LIMIT		= IB_SRQ_LIMIT,
+	RXE_SRQ_INIT		= BIT(2),
+};
+
 struct rxe_srq {
 	struct ib_srq		ibsrq;
 	struct rxe_pool_elem	elem;