All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] qedr: Unlock on error in qedr_post_send()
@ 2016-11-15  9:51 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2016-11-15  9:51 UTC (permalink / raw)
  To: Doug Ledford, Ram Amrani
  Cc: Sean Hefty, Hal Rosenstock, Rajesh Borundia, linux-rdma, kernel-janitors

If "wr" is NULL then we return without unlocking.  I added an unlock
label and changed the direct returns to gotos.

Fixes: afa0e13be754 ("qedr: Add support for data path")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index a615142..01d05b0 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -2973,17 +2973,18 @@ int qedr_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
 
 	if ((qp->state == QED_ROCE_QP_STATE_RESET) ||
 	    (qp->state == QED_ROCE_QP_STATE_ERR)) {
-		spin_unlock_irqrestore(&qp->q_lock, flags);
 		*bad_wr = wr;
 		DP_DEBUG(dev, QEDR_MSG_CQ,
 			 "QP in wrong state! QP icid=0x%x state %d\n",
 			 qp->icid, qp->state);
-		return -EINVAL;
+		rc = -EINVAL;
+		goto unlock;
 	}
 
 	if (!wr) {
 		DP_ERR(dev, "Got an empty post send.\n");
-		return -EINVAL;
+		rc = -EINVAL;
+		goto unlock;
 	}
 
 	while (wr) {
@@ -3012,6 +3013,7 @@ int qedr_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
 	/* Make sure write sticks */
 	mmiowb();
 
+unlock:
 	spin_unlock_irqrestore(&qp->q_lock, flags);
 
 	return rc;

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [patch] qedr: Unlock on error in qedr_post_send()
@ 2016-11-15  9:51 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2016-11-15  9:51 UTC (permalink / raw)
  To: Doug Ledford, Ram Amrani
  Cc: Sean Hefty, Hal Rosenstock, Rajesh Borundia, linux-rdma, kernel-janitors

If "wr" is NULL then we return without unlocking.  I added an unlock
label and changed the direct returns to gotos.

Fixes: afa0e13be754 ("qedr: Add support for data path")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index a615142..01d05b0 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -2973,17 +2973,18 @@ int qedr_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
 
 	if ((qp->state = QED_ROCE_QP_STATE_RESET) ||
 	    (qp->state = QED_ROCE_QP_STATE_ERR)) {
-		spin_unlock_irqrestore(&qp->q_lock, flags);
 		*bad_wr = wr;
 		DP_DEBUG(dev, QEDR_MSG_CQ,
 			 "QP in wrong state! QP icid=0x%x state %d\n",
 			 qp->icid, qp->state);
-		return -EINVAL;
+		rc = -EINVAL;
+		goto unlock;
 	}
 
 	if (!wr) {
 		DP_ERR(dev, "Got an empty post send.\n");
-		return -EINVAL;
+		rc = -EINVAL;
+		goto unlock;
 	}
 
 	while (wr) {
@@ -3012,6 +3013,7 @@ int qedr_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
 	/* Make sure write sticks */
 	mmiowb();
 
+unlock:
 	spin_unlock_irqrestore(&qp->q_lock, flags);
 
 	return rc;

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* RE: [patch] qedr: Unlock on error in qedr_post_send()
  2016-11-15  9:51 ` Dan Carpenter
@ 2016-11-15 10:28   ` Amrani, Ram
  -1 siblings, 0 replies; 4+ messages in thread
From: Amrani, Ram @ 2016-11-15 10:28 UTC (permalink / raw)
  To: Dan Carpenter, Doug Ledford
  Cc: Sean Hefty, Hal Rosenstock, Borundia, Rajesh,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA, Wei Yongjun

> If "wr" is NULL then we return without unlocking.  I added an unlock
> label and changed the direct returns to gotos.
> 
> Fixes: afa0e13be754 ("qedr: Add support for data path")
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> 
> diff --git a/drivers/infiniband/hw/qedr/verbs.c
> b/drivers/infiniband/hw/qedr/verbs.c
> index a615142..01d05b0 100644
> --- a/drivers/infiniband/hw/qedr/verbs.c
> +++ b/drivers/infiniband/hw/qedr/verbs.c
> @@ -2973,17 +2973,18 @@ int qedr_post_send(struct ib_qp *ibqp, struct
> ib_send_wr *wr,
> 
>  	if ((qp->state == QED_ROCE_QP_STATE_RESET) ||
>  	    (qp->state == QED_ROCE_QP_STATE_ERR)) {
> -		spin_unlock_irqrestore(&qp->q_lock, flags);
>  		*bad_wr = wr;
>  		DP_DEBUG(dev, QEDR_MSG_CQ,
>  			 "QP in wrong state! QP icid=0x%x state %d\n",
>  			 qp->icid, qp->state);
> -		return -EINVAL;
> +		rc = -EINVAL;
> +		goto unlock;
>  	}
> 
>  	if (!wr) {
>  		DP_ERR(dev, "Got an empty post send.\n");
> -		return -EINVAL;
> +		rc = -EINVAL;
> +		goto unlock;
>  	}
> 
>  	while (wr) {
> @@ -3012,6 +3013,7 @@ int qedr_post_send(struct ib_qp *ibqp, struct
> ib_send_wr *wr,
>  	/* Make sure write sticks */
>  	mmiowb();
> 
> +unlock:
>  	spin_unlock_irqrestore(&qp->q_lock, flags);
> 
>  	return rc;

Thanks for catching this. However this was already taken care of by Wei Yongjun in a different manner.
See: https://patchwork.kernel.org/patch/9409053/ 

Thanks,
Ram

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [patch] qedr: Unlock on error in qedr_post_send()
@ 2016-11-15 10:28   ` Amrani, Ram
  0 siblings, 0 replies; 4+ messages in thread
From: Amrani, Ram @ 2016-11-15 10:28 UTC (permalink / raw)
  To: Dan Carpenter, Doug Ledford
  Cc: Sean Hefty, Hal Rosenstock, Borundia, Rajesh,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA, Wei Yongjun

> If "wr" is NULL then we return without unlocking.  I added an unlock
> label and changed the direct returns to gotos.
> 
> Fixes: afa0e13be754 ("qedr: Add support for data path")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/infiniband/hw/qedr/verbs.c
> b/drivers/infiniband/hw/qedr/verbs.c
> index a615142..01d05b0 100644
> --- a/drivers/infiniband/hw/qedr/verbs.c
> +++ b/drivers/infiniband/hw/qedr/verbs.c
> @@ -2973,17 +2973,18 @@ int qedr_post_send(struct ib_qp *ibqp, struct
> ib_send_wr *wr,
> 
>  	if ((qp->state == QED_ROCE_QP_STATE_RESET) ||
>  	    (qp->state == QED_ROCE_QP_STATE_ERR)) {
> -		spin_unlock_irqrestore(&qp->q_lock, flags);
>  		*bad_wr = wr;
>  		DP_DEBUG(dev, QEDR_MSG_CQ,
>  			 "QP in wrong state! QP icid=0x%x state %d\n",
>  			 qp->icid, qp->state);
> -		return -EINVAL;
> +		rc = -EINVAL;
> +		goto unlock;
>  	}
> 
>  	if (!wr) {
>  		DP_ERR(dev, "Got an empty post send.\n");
> -		return -EINVAL;
> +		rc = -EINVAL;
> +		goto unlock;
>  	}
> 
>  	while (wr) {
> @@ -3012,6 +3013,7 @@ int qedr_post_send(struct ib_qp *ibqp, struct
> ib_send_wr *wr,
>  	/* Make sure write sticks */
>  	mmiowb();
> 
> +unlock:
>  	spin_unlock_irqrestore(&qp->q_lock, flags);
> 
>  	return rc;

Thanks for catching this. However this was already taken care of by Wei Yongjun in a different manner.
See: https://patchwork.kernel.org/patch/9409053/ 

Thanks,
Ram


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-11-15 10:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-15  9:51 [patch] qedr: Unlock on error in qedr_post_send() Dan Carpenter
2016-11-15  9:51 ` Dan Carpenter
2016-11-15 10:28 ` Amrani, Ram
2016-11-15 10:28   ` Amrani, Ram

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.