Skip to contentSkip to conversation

Ammasso - ib_post_send fix

PATCH In review frank zago 4 participants 13 messages Nov 24, 2009 – Dec 2, 2009 6 messages not in the archive InfiniBand

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): 20091124043323.927819053-klaOcWyJdxkshyMvu7JE4pqQE7yCjDx5@public.gmane.org

frank zago

author
#2 lore raw

PATCH 1/8 Ammasso - ib_post_send fix

Always set ib_post_send()'s bad_wr.

Signed-off-by Frank Zago <[email protected]>
drivers/infiniband/hw/amso1100/c2_qp.c +5 -2
Index: linux-2.6.31/drivers/infiniband/hw/amso1100/c2_qp.c
===================================================================
@@ -798,8 +798,10 @@
 	u8 actual_sge_count;
 	u32 msg_size;
 
-	if (qp->state > IB_QPS_RTS)
-		return -EINVAL;
+	if (qp->state > IB_QPS_RTS) {
+		err = -EINVAL;
+		goto out;
+	}
 
 	while (ib_wr) {
 

@@ -930,6 +932,7 @@
 		ib_wr = ib_wr->next;
 	}
 
+out:
 	if (err)
 		*bad_wr = ib_wr;
 	return err;
Signature · 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 4B0C19A1.1050008-klaOcWyJdxkshyMvu7JE4pqQE7yCjDx5@public.gmane.org

#4 lore raw

These patches seem to be mangled and won't apply... the main problem seems to be that your MUA is setting Content-Type to format=flowed, which destroys the whitespace. Please fix up your mail setup and resend. (get-send-email will almost always work if nothing else will)

While you're fixing that up, please also fix the subject lines -- ef for this patch the header shouldn't be "Ammasso -" but rather "RDMA/amso1100:" as the other entries in the log for that driver use. And also after your signed-off-by line, put a line with "---" on it so git drops everything afterwards from the changelog, and include a diffstat there to make it clear what the patch touches.

(These comments apply to the whole series, but you can drop the nes patches since Chien picked those up)

Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#5 lore raw

PATCH 2/8 ehca - ib_post_send fixes Always set ib_post_send()s bad_wr. Do not report success if an error occurred.

In reply to a message not in the archive
Signed-off-by Frank Zago <[email protected]>
drivers/infiniband/hw/ehca/ehca_reqs.c +8 -11
Index: linux-2.6.31/drivers/infiniband/hw/ehca/ehca_reqs.c
===================================================================
@@ -400,7 +400,6 @@
 
 static inline int post_one_send(struct ehca_qp *my_qp,
 			 struct ib_send_wr *cur_send_wr,
-			 struct ib_send_wr **bad_send_wr,
 			 int hidden)
 {
 	struct ehca_wqe *wqe_p;

@@ -412,8 +411,6 @@
 	wqe_p = ipz_qeit_get_inc(&my_qp->ipz_squeue);
 	if (unlikely(!wqe_p)) {
 		/* too many posted work requests: queue overflow */
-		if (bad_send_wr)
-			*bad_send_wr = cur_send_wr;
 		ehca_err(my_qp->ib_qp.device, "Too many posted WQEs "
 			 "qp_num=%x", my_qp->ib_qp.qp_num);
 		return -ENOMEM;

@@ -433,8 +430,6 @@
 	 */
 	if (unlikely(ret)) {
 		my_qp->ipz_squeue.current_q_offset = start_offset;
-		if (bad_send_wr)
-			*bad_send_wr = cur_send_wr;
 		ehca_err(my_qp->ib_qp.device, "Could not write WQE "
 			 "qp_num=%x", my_qp->ib_qp.qp_num);
 		return -EINVAL;

@@ -457,7 +452,8 @@
 	if (unlikely(my_qp->state < IB_QPS_RTS)) {
 		ehca_err(qp->device, "Invalid QP state  qp_state=%d qpn=%x",
 			 my_qp->state, qp->qp_num);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	/* LOCK the QUEUE */

@@ -476,7 +472,7 @@
 		struct ib_send_wr circ_wr;
 		memset(&circ_wr, 0, sizeof(circ_wr));
 		circ_wr.opcode = IB_WR_RDMA_READ;
-		post_one_send(my_qp, &circ_wr, NULL, 1); /* ignore retcode */
+		post_one_send(my_qp, &circ_wr, 1); /* ignore retcode */
 		wqe_cnt++;
 		ehca_dbg(qp->device, "posted circ wr  qp_num=%x", qp->qp_num);
 		my_qp->message_count = my_qp->packet_count = 0;

@@ -485,11 +481,8 @@
 	/* loop processes list of send reqs */
 	for (cur_send_wr = send_wr; cur_send_wr != NULL;
 	     cur_send_wr = cur_send_wr->next) {
-		ret = post_one_send(my_qp, cur_send_wr, bad_send_wr, 0);
+		ret = post_one_send(my_qp, cur_send_wr, 0);
 		if (unlikely(ret)) {
-			/* if one or more WQEs were successful, don't fail */
-			if (wqe_cnt)
-				ret = 0;
 			goto post_send_exit0;
 		}
 		wqe_cnt++;

@@ -503,6 +496,10 @@
 			 my_qp, qp->qp_num, wqe_cnt, ret);
 	my_qp->message_count += wqe_cnt;
 	spin_unlock_irqrestore(&my_qp->spinlock_s, flags);
+
+out:
+	if (ret)
+		*bad_send_wr = ib_wr;
 	return ret;
 }
Signature · 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#6 lore raw

PATCH 3/8 cxgb3 - ib_post_send fixes

In reply to a message not in the archive

Always set ib_post_send()'s bad_wr.

Signed-off-by Frank Zago <[email protected]>
drivers/infiniband/hw/cxgb3/iwch_qp.c +9 -6
Index: linux-2.6.31/drivers/infiniband/hw/cxgb3/iwch_qp.c
===================================================================
@@ -365,18 +365,19 @@
 	spin_lock_irqsave(&qhp->lock, flag);
 	if (qhp->attr.state > IWCH_QP_STATE_RTS) {
 		spin_unlock_irqrestore(&qhp->lock, flag);
-		return -EINVAL;
+		err = -EINVAL;
+		goto out;
 	}
 	num_wrs = Q_FREECNT(qhp->wq.sq_rptr, qhp->wq.sq_wptr,
 		  qhp->wq.sq_size_log2);
 	if (num_wrs <= 0) {
 		spin_unlock_irqrestore(&qhp->lock, flag);
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto out;
 	}
 	while (wr) {
 		if (num_wrs == 0) {
 			err = -ENOMEM;
-			*bad_wr = wr;
 			break;
 		}
 		idx = Q_PTR2IDX(qhp->wq.wptr, qhp->wq.size_log2);

@@ -428,10 +429,8 @@
 			     wr->opcode);
 			err = -EINVAL;
 		}
-		if (err) {
-			*bad_wr = wr;
+		if (err)
 			break;
-		}
 		wqe->send.wrid.id0.hi = qhp->wq.sq_wptr;
 		sqp->wr_id = wr->wr_id;
 		sqp->opcode = wr2opcode(t3_wr_opcode);

@@ -454,6 +453,10 @@
 	}
 	spin_unlock_irqrestore(&qhp->lock, flag);
 	ring_doorbell(qhp->wq.doorbell, qhp->wq.qpid);
+
+out:
+	if (err)
+		*bad_wr = wr;
 	return err;
 }
Signature · 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#7 lore raw

PATCH 4/8 cxgb3 - ib_post_recv fixes

In reply to a message not in the archive

Always set ib_post_recv()'s bad_wr. And like the other IB drivers, return ENOMEM when the queue is full.

Signed-off-by Frank Zago <[email protected]>
drivers/infiniband/hw/cxgb3/iwch_qp.c +11 -6
Index: linux-2.6.31/drivers/infiniband/hw/cxgb3/iwch_qp.c
===================================================================
@@ -474,18 +474,19 @@
 	spin_lock_irqsave(&qhp->lock, flag);
 	if (qhp->attr.state > IWCH_QP_STATE_RTS) {
 		spin_unlock_irqrestore(&qhp->lock, flag);
-		return -EINVAL;
+		err = -EINVAL;
+		goto out;
 	}
 	num_wrs = Q_FREECNT(qhp->wq.rq_rptr, qhp->wq.rq_wptr,
 			    qhp->wq.rq_size_log2) - 1;
 	if (!wr) {
 		spin_unlock_irqrestore(&qhp->lock, flag);
-		return -EINVAL;
+		err = -ENOMEM;
+		goto out;
 	}
 	while (wr) {
 		if (wr->num_sge > T3_MAX_SGE) {
 			err = -EINVAL;
-			*bad_wr = wr;
 			break;
 		}
 		idx = Q_PTR2IDX(qhp->wq.wptr, qhp->wq.size_log2);

@@ -497,10 +498,10 @@
 				err = build_zero_stag_recv(qhp, wqe, wr);
 		else
 			err = -ENOMEM;
-		if (err) {
-			*bad_wr = wr;
+
+		if (err)
 			break;
-		}
+
 		build_fw_riwrh((void *) wqe, T3_WR_RCV, T3_COMPLETION_FLAG,
 			       Q_GENBIT(qhp->wq.wptr, qhp->wq.size_log2),
 			       0, sizeof(struct t3_receive_wr) >> 3, T3_SOPEOP);

@@ -514,6 +515,10 @@
 	}
 	spin_unlock_irqrestore(&qhp->lock, flag);
 	ring_doorbell(qhp->wq.doorbell, qhp->wq.qpid);
+
+out:
+	if (err)
+		*bad_wr = wr;
 	return err;
 }
Signature · 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#8 lore raw

PATCH 5/8 ehca - ib_post_recv fixes Always set ib_post_recv()s bad_wr.

In reply to a message not in the archive
Signed-off-by Frank Zago <[email protected]>
drivers/infiniband/hw/ehca/ehca_reqs.c +12 -13
Index: linux-2.6.31/drivers/infiniband/hw/ehca/ehca_reqs.c
===================================================================
@@ -519,7 +519,7 @@
 	if (unlikely(!HAS_RQ(my_qp))) {
 		ehca_err(dev, "QP has no RQ  ehca_qp=%p qp_num=%x ext_type=%d",
 			 my_qp, my_qp->real_qp_num, my_qp->ext_type);
-		return -ENODEV;
+		ret = -ENODEV;
 	}
 
 	/* LOCK the QUEUE */

@@ -533,13 +533,9 @@
 		wqe_p = ipz_qeit_get_inc(&my_qp->ipz_rqueue);
 		if (unlikely(!wqe_p)) {
 			/* too many posted work requests: queue overflow */
-			if (bad_recv_wr)
-				*bad_recv_wr = cur_recv_wr;
-			if (wqe_cnt == 0) {
-				ret = -ENOMEM;
-				ehca_err(dev, "Too many posted WQEs "
-					 "qp_num=%x", my_qp->real_qp_num);
-			}
+			ret = -ENOMEM;
+			ehca_err(dev, "Too many posted WQEs "
+				"qp_num=%x", my_qp->real_qp_num);
 			goto post_recv_exit0;
 		}
 		/*

@@ -558,11 +554,9 @@
 		if (unlikely(ret)) {
 			my_qp->ipz_rqueue.current_q_offset = start_offset;
 			*bad_recv_wr = cur_recv_wr;
-			if (wqe_cnt == 0) {
-				ret = -EINVAL;
-				ehca_err(dev, "Could not write WQE "
-					 "qp_num=%x", my_qp->real_qp_num);
-			}
+			ret = -EINVAL;
+			ehca_err(dev, "Could not write WQE "
+				"qp_num=%x", my_qp->real_qp_num);
 			goto post_recv_exit0;
 		}
 

@@ -581,6 +575,11 @@
 	    ehca_dbg(dev, "ehca_qp=%p qp_num=%x wqe_cnt=%d ret=%i",
 		     my_qp, my_qp->real_qp_num, wqe_cnt, ret);
 	spin_unlock_irqrestore(&my_qp->spinlock_r, flags);
+
+out:
+	if (ret)
+		*bad_recv_wr = cur_recv_wr;
+
 	return ret;
 }
Signature · 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 4B0C19AE.6030006-klaOcWyJdxkshyMvu7JE4pqQE7yCjDx5@public.gmane.org

#10 lore raw

Re: [patch 5/8] ehca - ib_post_recv fixes Always set ib_post_recv()s bad_wr.

On Tue, 24 Nov 2009 11:36:46 -0600
frank zago <[email protected]> wrote:

Hi Frank,

drivers/infiniband/hw/ehca/ehca_reqs.c line 522

Quoted 13 lines
Signed-off-by Frank Zago <[email protected]>

Index: linux-2.6.31/drivers/infiniband/hw/ehca/ehca_reqs.c
===================================================================
--- linux-2.6.31.orig/drivers/infiniband/hw/ehca/ehca_reqs.c 2009-11-23
21:23:08.000000000 -0600
+++ linux-2.6.31/drivers/infiniband/hw/ehca/ehca_reqs.c 2009-11-23
21:25:48.000000000 -0600

drivers/infiniband/hw/ehca/ehca_reqs.c +1 -1
@@ -519,7 +519,7 @@
  	if (unlikely(!HAS_RQ(my_qp))) {
  		ehca_err(dev, "QP has no RQ  ehca_qp=%p qp_num=%x ext_type=%d",
  			 my_qp, my_qp->real_qp_num, my_qp->ext_type);
-		return -ENODEV;
+		ret = -ENODEV;

aren't you missing a goto statement here?

Quoted 38 lines
drivers/infiniband/hw/ehca/ehca_reqs.c +6 -12
  	}
 
  	/* LOCK the QUEUE */

@@ -533,13 +533,9 @@
  		wqe_p = ipz_qeit_get_inc(&my_qp->ipz_rqueue);
  		if (unlikely(!wqe_p)) {
  			/* too many posted work requests: queue overflow */
-			if (bad_recv_wr)
-				*bad_recv_wr = cur_recv_wr;
-			if (wqe_cnt == 0) {
-				ret = -ENOMEM;
-				ehca_err(dev, "Too many posted WQEs "
-					 "qp_num=%x", my_qp->real_qp_num);
-			}
+			ret = -ENOMEM;
+			ehca_err(dev, "Too many posted WQEs "
+				"qp_num=%x", my_qp->real_qp_num);
  			goto post_recv_exit0;
  		}
  		/*

@@ -558,11 +554,9 @@
  		if (unlikely(ret)) {
  			my_qp->ipz_rqueue.current_q_offset = start_offset;
  			*bad_recv_wr = cur_recv_wr;
-			if (wqe_cnt == 0) {
-				ret = -EINVAL;
-				ehca_err(dev, "Could not write WQE "
-					 "qp_num=%x", my_qp->real_qp_num);
-			}
+			ret = -EINVAL;
+			ehca_err(dev, "Could not write WQE "
+				"qp_num=%x", my_qp->real_qp_num);
  			goto post_recv_exit0;
  		}
 

@@ -581,6 +575,11 @@
  	    ehca_dbg(dev, "ehca_qp=%p qp_num=%x wqe_cnt=%d ret=%i",

drivers/infiniband/hw/ehca/ehca_reqs.c line 585

Quoted 10 lines
drivers/infiniband/hw/ehca/ehca_reqs.c +5 -0
  		     my_qp, my_qp->real_qp_num, wqe_cnt, ret);
  	spin_unlock_irqrestore(&my_qp->spinlock_r, flags);
+
+out:
+	if (ret)
+		*bad_recv_wr = cur_recv_wr;
+
  	return ret;
  }

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#11 lore raw

Re: [patch 5/8] ehca - ib_post_recv fixes Always set ib_post_recv()s bad_wr.

Quoted 6 lines · Alexander Schmidt

On Tue, 24 Nov 2009 11:36:46 -0600
frank zago <[email protected]> wrote:

Hi Frank,

Quoted 17 lines
Quoted 8 lines
Signed-off-by Frank Zago <[email protected]>

Index: linux-2.6.31/drivers/infiniband/hw/ehca/ehca_reqs.c
===================================================================
--- linux-2.6.31.orig/drivers/infiniband/hw/ehca/ehca_reqs.c 2009-11-23
21:23:08.000000000 -0600
+++ linux-2.6.31/drivers/infiniband/hw/ehca/ehca_reqs.c 2009-11-23
21:25:48.000000000 -0600

drivers/infiniband/hw/ehca/ehca_reqs.c line 523

Quoted 6 lines
drivers/infiniband/hw/ehca/ehca_reqs.c +1 -1
@@ -519,7 +519,7 @@
  	if (unlikely(!HAS_RQ(my_qp))) {
  		ehca_err(dev, "QP has no RQ  ehca_qp=%p qp_num=%x ext_type=%d",
  			 my_qp, my_qp->real_qp_num, my_qp->ext_type);
-		return -ENODEV;
+		ret = -ENODEV;

aren't you missing a goto statement here?

Indeed. I'll resend a patch.

Frank.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#12 lore raw

PATCH 6/8 ammasso - ib_post_recv fix

In reply to a message not in the archive

Always set ib_post_recv()'s bad_wr.

Signed-off-by Frank Zago <[email protected]>
drivers/infiniband/hw/amso1100/c2_qp.c +5 -2
Index: linux-2.6.31/drivers/infiniband/hw/amso1100/c2_qp.c
===================================================================
@@ -947,8 +947,10 @@
 	unsigned long lock_flags;
 	int err = 0;
 
-	if (qp->state > IB_QPS_RTS)
-		return -EINVAL;
+	if (qp->state > IB_QPS_RTS) {
+		err = -EINVAL;
+		goto out;
+	}
 
 	/*
 	 * Try and post each work request

@@ -1001,6 +1003,7 @@
 		ib_wr = ib_wr->next;
 	}
 
+out:
 	if (err)
 		*bad_wr = ib_wr;
 	return err;
Signature · 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#13 lore raw

PATCH 7/8 NES - ib_post_send fixes

In reply to a message not in the archive

Always set ib_post_send()'s bad_wr. Stop processing the wor request queue when an error is detected.

Signed-off-by Frank Zago <[email protected]>
drivers/infiniband/hw/nes/nes_verbs.c +8 -2
Index: linux-2.6.31/drivers/infiniband/hw/nes/nes_verbs.c
===================================================================
@@ -3386,8 +3386,10 @@
 	wqe_count = 0;
 	total_payload_length = 0;
 
-	if (nesqp->ibqp_state > IB_QPS_RTS)
-		return -EINVAL;
+	if (nesqp->ibqp_state > IB_QPS_RTS) {
+		err = -EINVAL;
+		goto out;
+	}
 
 	spin_lock_irqsave(&nesqp->lock, flags);
 

@@ -3498,6 +3500,9 @@
 					break;
 			}
 
+		if (err)
+			break;
+
 		if (ib_wr->send_flags & IB_SEND_SIGNALED) {
 			wqe_misc |= NES_IWARP_SQ_WQE_SIGNALED_COMPL;
 		}

@@ -3522,6 +3527,7 @@
 
 	spin_unlock_irqrestore(&nesqp->lock, flags);
 
+out:
 	if (err)
 		*bad_wr = ib_wr;
 	return err;
Signature · 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 4B0C19B1.8030304-klaOcWyJdxkshyMvu7JE4pqQE7yCjDx5@public.gmane.org

Acked-by
#15 lore raw

RE: [patch 7/8] NES - ib_post_send fixes

Quoted 2 lines

Always set ib_post_send()'s bad_wr. Stop processing the wor request queue when an error is detected.

Thanks for the patch.

RDMA/nes: In nes_post_send() always set bad_wr on error

Acked-by Chien Tung <[email protected]>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#16 lore raw

PATCH 8/8 NES - ib_post_recv fixes

In reply to a message not in the archive

Always set ib_post_recv()'s bad_wr.

Signed-off-by Frank Zago <[email protected]>
drivers/infiniband/hw/nes/nes_verbs.c +5 -2
Index: linux-2.6.31/drivers/infiniband/hw/nes/nes_verbs.c
===================================================================
@@ -3554,8 +3554,10 @@
 	u32 counter;
 	u32 total_payload_length;
 
-	if (nesqp->ibqp_state > IB_QPS_RTS)
-		return -EINVAL;
+	if (nesqp->ibqp_state > IB_QPS_RTS) {
+		err = -EINVAL;
+		goto out;
+	}
 
 	spin_lock_irqsave(&nesqp->lock, flags);
 

@@ -3618,6 +3620,7 @@
 
 	spin_unlock_irqrestore(&nesqp->lock, flags);
 
+out:
 	if (err)
 		*bad_wr = ib_wr;
 	return err;
Signature · 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 4B0C19B8.8010807-klaOcWyJdxkshyMvu7JE4pqQE7yCjDx5@public.gmane.org