RDMA/rxe: Fix errant WARN_ONCE in rxe_completer()
authorBob Pearson <rpearsonhpe@gmail.com>
Thu, 4 Mar 2021 19:20:49 +0000 (13:20 -0600)
committerJason Gunthorpe <jgg@nvidia.com>
Fri, 5 Mar 2021 18:15:22 +0000 (14:15 -0400)
In rxe_comp.c in rxe_completer() the function free_pkt() did not clear skb
which triggered a warning at 'done:' and could possibly at 'exit:'. The
WARN_ONCE() calls are not actually needed.  The call to free_pkt() is
moved to the end to clearly show that all skbs are freed.

Fixes: 899aba891cab ("RDMA/rxe: Fix FIXME in rxe_udp_encap_recv()")
Link: https://lore.kernel.org/r/20210304192048.2958-1-rpearson@hpe.com
Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
drivers/infiniband/sw/rxe/rxe_comp.c

index a8ac791..17a361b 100644 (file)
@@ -547,6 +547,7 @@ int rxe_completer(void *arg)
        struct sk_buff *skb = NULL;
        struct rxe_pkt_info *pkt = NULL;
        enum comp_state state;
+       int ret = 0;
 
        rxe_add_ref(qp);
 
@@ -554,7 +555,8 @@ int rxe_completer(void *arg)
            qp->req.state == QP_STATE_RESET) {
                rxe_drain_resp_pkts(qp, qp->valid &&
                                    qp->req.state == QP_STATE_ERROR);
-               goto exit;
+               ret = -EAGAIN;
+               goto done;
        }
 
        if (qp->comp.timeout) {
@@ -564,8 +566,10 @@ int rxe_completer(void *arg)
                qp->comp.timeout_retry = 0;
        }
 
-       if (qp->req.need_retry)
-               goto exit;
+       if (qp->req.need_retry) {
+               ret = -EAGAIN;
+               goto done;
+       }
 
        state = COMPST_GET_ACK;
 
@@ -636,8 +640,6 @@ int rxe_completer(void *arg)
                        break;
 
                case COMPST_DONE:
-                       if (pkt)
-                               free_pkt(pkt);
                        goto done;
 
                case COMPST_EXIT:
@@ -660,7 +662,8 @@ int rxe_completer(void *arg)
                            qp->qp_timeout_jiffies)
                                mod_timer(&qp->retrans_timer,
                                          jiffies + qp->qp_timeout_jiffies);
-                       goto exit;
+                       ret = -EAGAIN;
+                       goto done;
 
                case COMPST_ERROR_RETRY:
                        /* we come here if the retry timer fired and we did
@@ -672,18 +675,18 @@ int rxe_completer(void *arg)
                         */
 
                        /* there is nothing to retry in this case */
-                       if (!wqe || (wqe->state == wqe_state_posted))
-                               goto exit;
+                       if (!wqe || (wqe->state == wqe_state_posted)) {
+                               pr_warn("Retry attempted without a valid wqe\n");
+                               ret = -EAGAIN;
+                               goto done;
+                       }
 
                        /* if we've started a retry, don't start another
                         * retry sequence, unless this is a timeout.
                         */
                        if (qp->comp.started_retry &&
-                           !qp->comp.timeout_retry) {
-                               if (pkt)
-                                       free_pkt(pkt);
+                           !qp->comp.timeout_retry)
                                goto done;
-                       }
 
                        if (qp->comp.retry_cnt > 0) {
                                if (qp->comp.retry_cnt != 7)
@@ -704,8 +707,6 @@ int rxe_completer(void *arg)
                                        qp->comp.started_retry = 1;
                                        rxe_run_task(&qp->req.task, 0);
                                }
-                               if (pkt)
-                                       free_pkt(pkt);
                                goto done;
 
                        } else {
@@ -726,8 +727,8 @@ int rxe_completer(void *arg)
                                mod_timer(&qp->rnr_nak_timer,
                                          jiffies + rnrnak_jiffies(aeth_syn(pkt)
                                                & ~AETH_TYPE_MASK));
-                               free_pkt(pkt);
-                               goto exit;
+                               ret = -EAGAIN;
+                               goto done;
                        } else {
                                rxe_counter_inc(rxe,
                                                RXE_CNT_RNR_RETRY_EXCEEDED);
@@ -740,25 +741,15 @@ int rxe_completer(void *arg)
                        WARN_ON_ONCE(wqe->status == IB_WC_SUCCESS);
                        do_complete(qp, wqe);
                        rxe_qp_error(qp);
-                       if (pkt)
-                               free_pkt(pkt);
-                       goto exit;
+                       ret = -EAGAIN;
+                       goto done;
                }
        }
 
-exit:
-       /* we come here if we are done with processing and want the task to
-        * exit from the loop calling us
-        */
-       WARN_ON_ONCE(skb);
-       rxe_drop_ref(qp);
-       return -EAGAIN;
-
 done:
-       /* we come here if we have processed a packet we want the task to call
-        * us again to see if there is anything else to do
-        */
-       WARN_ON_ONCE(skb);
+       if (pkt)
+               free_pkt(pkt);
        rxe_drop_ref(qp);
-       return 0;
+
+       return ret;
 }