[PATCH v2 2/3] nvme-rdma: don't complete requests before a send work request has completed

Sagi Grimberg sagi at grimberg.me
Mon Nov 20 03:12:41 PST 2017


> -	if ((wc->wc_flags & IB_WC_WITH_INVALIDATE) &&
> -	    wc->ex.invalidate_rkey == req->mr->rkey) {
> -		atomic_dec(&req->ref);
> -	} else if (req->mr->need_inval) {
> +	if (req->mr->need_inval &&
> +	    (!(wc->wc_flags & IB_WC_WITH_INVALIDATE) ||
> +	     wc->ex.invalidate_rkey != req->mr->rkey)) {

I've been meaning to change this path because getting a remote
invalidation on a bogus MR is a protocol error and we want to
detect it regardless of req->mr->need_inval.

However, I do agree that we can save a atomic if we bail out
after we queue a local invalidate:

How about this (should be split into two patches):
--
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index a3802de0f248..d0a5a397ad66 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1148,8 +1148,6 @@ static int nvme_rdma_map_sg_fr(struct 
nvme_rdma_queue *queue,
         sg->type = (NVME_KEY_SGL_FMT_DATA_DESC << 4) |
                         NVME_SGL_FMT_INVALIDATE;

-       refcount_inc(&req->ref);
-
         return 0;
  }

@@ -1335,10 +1333,14 @@ static int nvme_rdma_process_nvme_rsp(struct 
nvme_rdma_queue *queue,
         req->cqe.status = cqe->status;
         req->cqe.result = cqe->result;

-       if ((wc->wc_flags & IB_WC_WITH_INVALIDATE) &&
-           wc->ex.invalidate_rkey == req->mr->rkey) {
+       if (wc->wc_flags & IB_WC_WITH_INVALIDATE) {
+               if (unlikely(wc->ex.invalidate_rkey == req->mr->rkey)) {
+                       dev_err(queue->ctrl->ctrl.device,
+                               "Bogus remote invalidation for rkey %#x\n",
+                               req->mr->rkey);
+                       nvme_rdma_error_recovery(queue->ctrl);
+               }
                 req->mr->need_inval = false;
-               refcount_dec(&req->ref);
         } else if (req->mr->need_inval) {
                 ret = nvme_rdma_inv_rkey(queue, req);
                 if (unlikely(ret < 0)) {
@@ -1347,6 +1349,8 @@ static int nvme_rdma_process_nvme_rsp(struct 
nvme_rdma_queue *queue,
                                 req->mr->rkey, ret);
                         nvme_rdma_error_recovery(queue->ctrl);
                 }
+               /* return here as we can't really end the request */
+               return 0;
         }

         if (refcount_dec_and_test(&req->ref)) {
--



More information about the Linux-nvme mailing list