[openib-general] [RFC] [PATCH] Remove redundant ib_qp_cap from 2 verb routines.

Krishna Kumar krkumar at us.ibm.com
Fri Oct 29 13:01:03 PDT 2004


Hi,

I know this changes the verbs interface a bit, but ...

I don't see a value in the qp_cap being passed to different routines,
when either ib_qp_attr or ib_qp_init_attr, both of which contain a
qp_cap, are being passed at the same time.

Above attached patch removes the qp_cap (saves stack space too) and I
have also included a patch for mthca just for completeness to show how
the mthca can avoid using the extra variable.

Does this sound right ?

Thanks,

- KK

[Inline patch for viewing]
diff -ruNp org/access/mad.c new/access/mad.c
--- org/access/mad.c	2004-10-29 12:38:35.000000000 -0700
+++ new/access/mad.c	2004-10-29 12:38:16.000000000 -0700
@@ -1509,7 +1509,6 @@ static inline int ib_mad_change_qp_state
 	int ret;
 	struct ib_qp_attr *attr;
 	int attr_mask;
-	struct ib_qp_cap qp_cap;

 	attr =  kmalloc(sizeof *attr, GFP_KERNEL);
 	if (!attr) {
@@ -1530,7 +1529,7 @@ static inline int ib_mad_change_qp_state
 		attr->qkey = IB_QP1_QKEY;
 	attr_mask = IB_QP_STATE | IB_QP_PKEY_INDEX | IB_QP_QKEY;

-	ret = ib_modify_qp(qp, attr, attr_mask, &qp_cap);
+	ret = ib_modify_qp(qp, attr, attr_mask);
 	kfree(attr);

 	if (ret)
@@ -1546,7 +1545,6 @@ static inline int ib_mad_change_qp_state
 	int ret;
 	struct ib_qp_attr *attr;
 	int attr_mask;
-	struct ib_qp_cap qp_cap;

         attr =  kmalloc(sizeof *attr, GFP_KERNEL);
 	if (!attr) {
@@ -1557,7 +1555,7 @@ static inline int ib_mad_change_qp_state
 	attr->qp_state = IB_QPS_RTR;
 	attr_mask = IB_QP_STATE;

-	ret = ib_modify_qp(qp, attr, attr_mask, &qp_cap);
+	ret = ib_modify_qp(qp, attr, attr_mask);
 	kfree(attr);

 	if (ret)
@@ -1573,7 +1571,6 @@ static inline int ib_mad_change_qp_state
 	int ret;
 	struct ib_qp_attr *attr;
 	int attr_mask;
-	struct ib_qp_cap qp_cap;

 	attr = kmalloc(sizeof *attr, GFP_KERNEL);
 	if (!attr) {
@@ -1585,7 +1582,7 @@ static inline int ib_mad_change_qp_state
 	attr->sq_psn = IB_MAD_SEND_Q_PSN;
 	attr_mask = IB_QP_STATE | IB_QP_SQ_PSN;

-	ret = ib_modify_qp(qp, attr, attr_mask, &qp_cap);
+	ret = ib_modify_qp(qp, attr, attr_mask);
 	kfree(attr);

 	if (ret)
@@ -1601,7 +1598,6 @@ static inline int ib_mad_change_qp_state
 	int ret;
 	struct ib_qp_attr *attr;
 	int attr_mask;
-	struct ib_qp_cap qp_cap;

 	attr = kmalloc(sizeof *attr, GFP_KERNEL);
  	if (!attr) {
@@ -1612,7 +1608,7 @@ static inline int ib_mad_change_qp_state
 	attr->qp_state = IB_QPS_RESET;
 	attr_mask = IB_QP_STATE;

-	ret = ib_modify_qp(qp, attr, attr_mask, &qp_cap);
+	ret = ib_modify_qp(qp, attr, attr_mask);
 	kfree(attr);

 	if (ret)
@@ -1761,7 +1757,6 @@ static int ib_mad_port_open(struct ib_de

 	for (i = 0; i < IB_MAD_QPS_CORE; i++) {
 		struct ib_qp_init_attr	qp_init_attr;
-		struct ib_qp_cap	qp_cap;

 		memset(&qp_init_attr, 0, sizeof qp_init_attr);
 		qp_init_attr.send_cq = port_priv->cq;
@@ -1774,8 +1769,7 @@ static int ib_mad_port_open(struct ib_de
 		qp_init_attr.cap.max_recv_sge = IB_MAD_RECV_REQ_MAX_SG;
 		qp_init_attr.qp_type = i;	/* Relies on ib_qp_type enum ordering of IB_QPT_SMI and IB_QPT_GSI */
 		qp_init_attr.port_num = port_priv->port_num;
-		port_priv->qp[i] = ib_create_qp(port_priv->pd, &qp_init_attr,
-						&qp_cap);
+		port_priv->qp[i] = ib_create_qp(port_priv->pd, &qp_init_attr);
 		if (IS_ERR(port_priv->qp[i])) {
 			printk(KERN_ERR PFX "Couldn't create ib_mad QP%d\n", i);
 			ret = PTR_ERR(port_priv->qp[i]);
diff -ruNp org/access/verbs.c new/access/verbs.c
--- org/access/verbs.c	2004-10-29 12:38:35.000000000 -0700
+++ new/access/verbs.c	2004-10-29 12:38:16.000000000 -0700
@@ -136,12 +136,11 @@ EXPORT_SYMBOL(ib_destroy_ah);
 /* Queue pair */

 struct ib_qp *ib_create_qp(struct ib_pd *pd,
-			   struct ib_qp_init_attr *qp_init_attr,
-			   struct ib_qp_cap *qp_cap)
+			   struct ib_qp_init_attr *qp_init_attr)
 {
 	struct ib_qp *qp;

-	qp = pd->device->create_qp(pd, qp_init_attr, qp_cap);
+	qp = pd->device->create_qp(pd, qp_init_attr);

 	if (!IS_ERR(qp)) {
 		qp->device	= pd->device;
diff -ruNp org/include/ib_verbs.h new/include/ib_verbs.h
--- org/include/ib_verbs.h	2004-10-29 12:38:39.000000000 -0700
+++ new/include/ib_verbs.h	2004-10-29 12:38:22.000000000 -0700
@@ -687,12 +687,10 @@ struct ib_device {
 				     struct ib_ah_attr *ah_attr);
 	int		 (*destroy_ah)(struct ib_ah *ah);
 	struct ib_qp *	 (*create_qp)(struct ib_pd *pd,
-				      struct ib_qp_init_attr *qp_init_attr,
-				      struct ib_qp_cap *qp_cap);
+				      struct ib_qp_init_attr *qp_init_attr);
 	int		 (*modify_qp)(struct ib_qp *qp,
 				      struct ib_qp_attr *qp_attr,
-				      int qp_attr_mask,
-				      struct ib_qp_cap *qp_cap);
+				      int qp_attr_mask);
 	int		 (*query_qp)(struct ib_qp *qp,
 				    struct ib_qp_attr *qp_attr,
 				    int qp_attr_mask,
@@ -844,15 +842,13 @@ int ib_query_ah(struct ib_ah *ah,
 int ib_destroy_ah(struct ib_ah *ah);

 struct ib_qp *ib_create_qp(struct ib_pd *pd,
-			   struct ib_qp_init_attr *qp_init_attr,
-			   struct ib_qp_cap *qp_cap);
+			   struct ib_qp_init_attr *qp_init_attr);

 static inline int ib_modify_qp(struct ib_qp *qp,
 			       struct ib_qp_attr *qp_attr,
-			       int qp_attr_mask,
-			       struct ib_qp_cap *qp_cap)
+			       int qp_attr_mask)
 {
-	return qp->device->modify_qp(qp, qp_attr, qp_attr_mask, qp_cap);
+	return qp->device->modify_qp(qp, qp_attr, qp_attr_mask);
 }

 int ib_query_qp(struct ib_qp *qp,




diff -ruNp org/hw/mthca/mthca_provider.c new/hw/mthca/mthca_provider.c
--- org/hw/mthca/mthca_provider.c	2004-10-29 12:42:38.000000000 -0700
+++ new/hw/mthca/mthca_provider.c	2004-10-29 12:46:41.000000000 -0700
@@ -289,8 +289,7 @@ static int mthca_ah_destroy(struct ib_ah
 }

 static struct ib_qp *mthca_create_qp(struct ib_pd *pd,
-				     struct ib_qp_init_attr *init_attr,
-				     struct ib_qp_cap *qp_cap)
+				     struct ib_qp_init_attr *init_attr)
 {
 	struct mthca_qp *qp;
 	int err;
@@ -349,8 +348,7 @@ static struct ib_qp *mthca_create_qp(str
 		return ERR_PTR(err);
 	}

-	*qp_cap = init_attr->cap;
-	qp_cap->max_inline_data = 0;
+        init_attr->cap.max_inline_data = 0;

 	return (struct ib_qp *) qp;
 }
-------------- next part --------------
diff -ruNp org/access/mad.c new/access/mad.c
--- org/access/mad.c	2004-10-29 12:38:35.000000000 -0700
+++ new/access/mad.c	2004-10-29 12:38:16.000000000 -0700
@@ -1509,7 +1509,6 @@ static inline int ib_mad_change_qp_state
 	int ret;
 	struct ib_qp_attr *attr;
 	int attr_mask;
-	struct ib_qp_cap qp_cap;
 
 	attr =  kmalloc(sizeof *attr, GFP_KERNEL);
 	if (!attr) {
@@ -1530,7 +1529,7 @@ static inline int ib_mad_change_qp_state
 		attr->qkey = IB_QP1_QKEY;
 	attr_mask = IB_QP_STATE | IB_QP_PKEY_INDEX | IB_QP_QKEY;
 
-	ret = ib_modify_qp(qp, attr, attr_mask, &qp_cap);
+	ret = ib_modify_qp(qp, attr, attr_mask);
 	kfree(attr);
 
 	if (ret)
@@ -1546,7 +1545,6 @@ static inline int ib_mad_change_qp_state
 	int ret;
 	struct ib_qp_attr *attr;
 	int attr_mask;
-	struct ib_qp_cap qp_cap;
 
         attr =  kmalloc(sizeof *attr, GFP_KERNEL);
 	if (!attr) {
@@ -1557,7 +1555,7 @@ static inline int ib_mad_change_qp_state
 	attr->qp_state = IB_QPS_RTR;
 	attr_mask = IB_QP_STATE;
 
-	ret = ib_modify_qp(qp, attr, attr_mask, &qp_cap);
+	ret = ib_modify_qp(qp, attr, attr_mask);
 	kfree(attr);
 
 	if (ret)
@@ -1573,7 +1571,6 @@ static inline int ib_mad_change_qp_state
 	int ret;
 	struct ib_qp_attr *attr;
 	int attr_mask;
-	struct ib_qp_cap qp_cap;
 
 	attr = kmalloc(sizeof *attr, GFP_KERNEL);
 	if (!attr) {
@@ -1585,7 +1582,7 @@ static inline int ib_mad_change_qp_state
 	attr->sq_psn = IB_MAD_SEND_Q_PSN;
 	attr_mask = IB_QP_STATE | IB_QP_SQ_PSN;
 
-	ret = ib_modify_qp(qp, attr, attr_mask, &qp_cap);
+	ret = ib_modify_qp(qp, attr, attr_mask);
 	kfree(attr);
 
 	if (ret)
@@ -1601,7 +1598,6 @@ static inline int ib_mad_change_qp_state
 	int ret;
 	struct ib_qp_attr *attr;
 	int attr_mask;
-	struct ib_qp_cap qp_cap;
 
 	attr = kmalloc(sizeof *attr, GFP_KERNEL);
  	if (!attr) {
@@ -1612,7 +1608,7 @@ static inline int ib_mad_change_qp_state
 	attr->qp_state = IB_QPS_RESET;
 	attr_mask = IB_QP_STATE;
 
-	ret = ib_modify_qp(qp, attr, attr_mask, &qp_cap);
+	ret = ib_modify_qp(qp, attr, attr_mask);
 	kfree(attr);
 
 	if (ret)
@@ -1761,7 +1757,6 @@ static int ib_mad_port_open(struct ib_de
 
 	for (i = 0; i < IB_MAD_QPS_CORE; i++) {
 		struct ib_qp_init_attr	qp_init_attr;
-		struct ib_qp_cap	qp_cap;
 
 		memset(&qp_init_attr, 0, sizeof qp_init_attr);
 		qp_init_attr.send_cq = port_priv->cq;
@@ -1774,8 +1769,7 @@ static int ib_mad_port_open(struct ib_de
 		qp_init_attr.cap.max_recv_sge = IB_MAD_RECV_REQ_MAX_SG;
 		qp_init_attr.qp_type = i;	/* Relies on ib_qp_type enum ordering of IB_QPT_SMI and IB_QPT_GSI */
 		qp_init_attr.port_num = port_priv->port_num;
-		port_priv->qp[i] = ib_create_qp(port_priv->pd, &qp_init_attr,
-						&qp_cap);
+		port_priv->qp[i] = ib_create_qp(port_priv->pd, &qp_init_attr);
 		if (IS_ERR(port_priv->qp[i])) {
 			printk(KERN_ERR PFX "Couldn't create ib_mad QP%d\n", i);
 			ret = PTR_ERR(port_priv->qp[i]);
diff -ruNp org/access/verbs.c new/access/verbs.c
--- org/access/verbs.c	2004-10-29 12:38:35.000000000 -0700
+++ new/access/verbs.c	2004-10-29 12:38:16.000000000 -0700
@@ -136,12 +136,11 @@ EXPORT_SYMBOL(ib_destroy_ah);
 /* Queue pair */
 
 struct ib_qp *ib_create_qp(struct ib_pd *pd,
-			   struct ib_qp_init_attr *qp_init_attr,
-			   struct ib_qp_cap *qp_cap)
+			   struct ib_qp_init_attr *qp_init_attr)
 {
 	struct ib_qp *qp;
 
-	qp = pd->device->create_qp(pd, qp_init_attr, qp_cap);
+	qp = pd->device->create_qp(pd, qp_init_attr);
 
 	if (!IS_ERR(qp)) {
 		qp->device	= pd->device;
diff -ruNp org/include/ib_verbs.h new/include/ib_verbs.h
--- org/include/ib_verbs.h	2004-10-29 12:38:39.000000000 -0700
+++ new/include/ib_verbs.h	2004-10-29 12:38:22.000000000 -0700
@@ -687,12 +687,10 @@ struct ib_device {
 				     struct ib_ah_attr *ah_attr);
 	int		 (*destroy_ah)(struct ib_ah *ah);
 	struct ib_qp *	 (*create_qp)(struct ib_pd *pd,
-				      struct ib_qp_init_attr *qp_init_attr,
-				      struct ib_qp_cap *qp_cap);
+				      struct ib_qp_init_attr *qp_init_attr);
 	int		 (*modify_qp)(struct ib_qp *qp,
 				      struct ib_qp_attr *qp_attr,
-				      int qp_attr_mask,
-				      struct ib_qp_cap *qp_cap);
+				      int qp_attr_mask);
 	int		 (*query_qp)(struct ib_qp *qp,
 				    struct ib_qp_attr *qp_attr,
 				    int qp_attr_mask,
@@ -844,15 +842,13 @@ int ib_query_ah(struct ib_ah *ah,
 int ib_destroy_ah(struct ib_ah *ah);
 
 struct ib_qp *ib_create_qp(struct ib_pd *pd,
-			   struct ib_qp_init_attr *qp_init_attr,
-			   struct ib_qp_cap *qp_cap);
+			   struct ib_qp_init_attr *qp_init_attr);
 
 static inline int ib_modify_qp(struct ib_qp *qp,
 			       struct ib_qp_attr *qp_attr,
-			       int qp_attr_mask,
-			       struct ib_qp_cap *qp_cap)
+			       int qp_attr_mask)
 {
-	return qp->device->modify_qp(qp, qp_attr, qp_attr_mask, qp_cap);
+	return qp->device->modify_qp(qp, qp_attr, qp_attr_mask);
 }
 
 int ib_query_qp(struct ib_qp *qp,




diff -ruNp org/hw/mthca/mthca_provider.c new/hw/mthca/mthca_provider.c
--- org/hw/mthca/mthca_provider.c	2004-10-29 12:42:38.000000000 -0700
+++ new/hw/mthca/mthca_provider.c	2004-10-29 12:46:41.000000000 -0700
@@ -289,8 +289,7 @@ static int mthca_ah_destroy(struct ib_ah
 }
 
 static struct ib_qp *mthca_create_qp(struct ib_pd *pd,
-				     struct ib_qp_init_attr *init_attr,
-				     struct ib_qp_cap *qp_cap)
+				     struct ib_qp_init_attr *init_attr)
 {
 	struct mthca_qp *qp;
 	int err;
@@ -349,8 +348,7 @@ static struct ib_qp *mthca_create_qp(str
 		return ERR_PTR(err);
 	}
 
-	*qp_cap = init_attr->cap;
-	qp_cap->max_inline_data = 0;
+        init_attr->cap.max_inline_data = 0;
 
 	return (struct ib_qp *) qp;
 }


More information about the general mailing list