nvme-rdma: don't suppress send completions
Subsystems (2)
Sagi Grimberg
maintainerPATCH 1/1 nvme-rdma: don't suppress send completions
The entire completions suppress mechanism is currently broken because the HCA might retry a send operation (due to dropped ack) after the nvme transaction has completed.
In order to handle this, we signal all send completions and introduce a separate done handler for async events as they will be handled differently (as they don't include in-capsule data by definition).
--- drivers/nvme/host/rdma.c | 54 +++++++++++++----------------------------------- 1 file changed, 14 insertions(+), 40 deletions(-)
drivers/nvme/host/rdma.c +14 -40
@@ -88,7 +88,6 @@ enum nvme_rdma_queue_flags {struct nvme_rdma_queue {struct nvme_rdma_qe *rsp_ring; -atomic_t sig_count;int queue_size;size_t cmnd_capsule_len;struct nvme_rdma_ctrl *ctrl;
@@ -521,7 +520,6 @@ static int nvme_rdma_alloc_queue(struct nvme_rdma_ctrl *ctrl, queue->cmnd_capsule_len =sizeof (struct nvme_command ); queue->queue_size = queue_size; -atomic_set (&queue->sig_count,0 ); queue->cm_id =rdma_create_id (&init_net, nvme_rdma_cm_handler, queue,RDMA_PS_TCP ,IB_QPT_RC );
@@ -1232,21 +1230,9 @@ static void nvme_rdma_send_done(struct ib_cq *cq, struct ib_wc *wc)nvme_end_request (rq, req->status, req->result); } -/* -* We want to signal completion at least every queue depth/2. This returns the -* largest power of two that is not above half of (queue size + 1) to optimize -* (avoid divisions). -*/ -static inline bool nvme_rdma_queue_sig_limit (struct nvme_rdma_queue *queue) -{ -int limit =1 <<ilog2 ((queue->queue_size +1 ) /2 ); - -return (atomic_inc_return (&queue->sig_count) & (limit -1 )) ==0 ; -} -static int nvme_rdma_post_send (struct nvme_rdma_queue *queue,struct nvme_rdma_qe *qe,struct ib_sge *sge,u32 num_sge, -struct ib_send_wr *first,bool flush) +struct ib_send_wr *first) {struct ib_send_wr wr, *bad_wr;int ret;
@@ -1255,31 +1241,12 @@ static int nvme_rdma_post_send(struct nvme_rdma_queue *queue, sge->length =sizeof (struct nvme_command ), sge->lkey = queue->device->pd->local_dma_lkey; - qe->cqe.done = nvme_rdma_send_done; - wr.next =NULL ; wr.wr_cqe = &qe->cqe; wr.sg_list = sge; wr.num_sge = num_sge; wr.opcode =IB_WR_SEND ; - wr.send_flags =0 ; - -/* -* Unsignalled send completions are another giant desaster in the -* IB Verbs spec: If we don't regularly post signalled sends -* the send queue will fill up and only a QP reset will rescue us. -* Would have been way to obvious to handle this in hardware or -* at least the RDMA stack.. -* -* Always signal the flushes. The magic request used for the flush -* sequencer is not allocated in our driver's tagset and it's -* triggered to be freed by blk_cleanup_queue(). So we need to -* always mark it as signaled to ensure that the "wr_cqe", which is -* embedded in request's payload, is not freed when __ib_process_cq() -* calls wr_cqe->done(). -*/ -if (nvme_rdma_queue_sig_limit (queue) || flush) - wr.send_flags |=IB_SEND_SIGNALED ; + wr.send_flags =IB_SEND_SIGNALED ;if (first) first->next = ≀
@@ -1329,6 +1296,12 @@ static struct blk_mq_tags *nvme_rdma_tagset(struct nvme_rdma_queue *queue)return queue->ctrl->tag_set.tags[queue_idx -1 ]; } +static void nvme_rdma_async_done (struct ib_cq *cq,struct ib_wc *wc) +{ +if (unlikely (wc->status !=IB_WC_SUCCESS )) +nvme_rdma_wr_error (cq, wc,"ASYNC" ); +} +static void nvme_rdma_submit_async_event (struct nvme_ctrl *arg,int aer_idx) {struct nvme_rdma_ctrl *ctrl =to_rdma_ctrl (arg);
@@ -1350,10 +1323,12 @@ static void nvme_rdma_submit_async_event(struct nvme_ctrl *arg, int aer_idx) cmd->common.flags |=NVME_CMD_SGL_METABUF ;nvme_rdma_set_sg_null (cmd); + sqe->cqe.done = nvme_rdma_async_done; +ib_dma_sync_single_for_device (dev, sqe->dma,sizeof (*cmd),DMA_TO_DEVICE ); - ret =nvme_rdma_post_send (queue, sqe, &sge,1 ,NULL , false); + ret =nvme_rdma_post_send (queue, sqe, &sge,1 ,NULL );WARN_ON_ONCE (ret); }
@@ -1639,7 +1614,6 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,struct nvme_rdma_request *req =blk_mq_rq_to_pdu (rq);struct nvme_rdma_qe *sqe = &req->sqe;struct nvme_command *c = sqe->data; -bool flush = false;struct ib_device *dev;blk_status_t ret;int err;
@@ -1668,13 +1642,13 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx, goto err; } + sqe->cqe.done = nvme_rdma_send_done; +ib_dma_sync_single_for_device (dev, sqe->dma,sizeof (struct nvme_command ),DMA_TO_DEVICE ); -if (req_op (rq) ==REQ_OP_FLUSH ) - flush = true; err =nvme_rdma_post_send (queue, sqe, req->sge, req->num_sge, - req->mr->need_inval ? &req->reg_wr.wr :NULL , flush); + req->mr->need_inval ? &req->reg_wr.wr :NULL );if (unlikely (err)) {nvme_rdma_unmap_data (queue, rq); goto err;
Signature · 1 line
2.14.1
Greg KH
maintainerPATCH 1/1 nvme-rdma: don't suppress send completions
Quoted 8 lines · Sagi Grimberg
The entire completions suppress mechanism is currently broken because the HCA might retry a send operation (due to dropped ack) after the nvme transaction has completed.
In order to handle this, we signal all send completions and introduce a separate done handler for async events as they will be handled differently (as they don't include in-capsule data by definition).
Quoted 7 lines
Cc <stable at vger.kernel.org> # v4.14+Signed-off-by Sagi Grimberg <sagi at grimberg.me>Reviewed-by Max Gurtovoy <maxg at mellanox.com>Signed-off-by Christoph Hellwig <hch at lst.de>--- drivers/nvme/host/rdma.c | 54 +++++++++++++----------------------------------- 1 file changed, 14 insertions(+), 40 deletions(-)
What is the git commit id for this patch in Linus's tree?
thanks,
greg k-h
PATCH 1/1 nvme-rdma: don't suppress send completions
Quoted 17 lines · Greg KH
Quoted 8 lines · Sagi Grimberg
The entire completions suppress mechanism is currently broken because the HCA might retry a send operation (due to dropped ack) after the nvme transaction has completed.
In order to handle this, we signal all send completions and introduce a separate done handler for async events as they will be handled differently (as they don't include in-capsule data by definition).
Quoted 7 lines
Cc <stable at vger.kernel.org> # v4.14+Signed-off-by Sagi Grimberg <sagi at grimberg.me>Reviewed-by Max Gurtovoy <maxg at mellanox.com>Signed-off-by Christoph Hellwig <hch at lst.de>--- drivers/nvme/host/rdma.c | 54 +++++++++++++----------------------------------- 1 file changed, 14 insertions(+), 40 deletions(-)
Quoted 1 line
What is the git commit id for this patch in Linus's tree?
Commit b4b591c87f2b0f4ebaf3a68d4f13873b241aa584 upstream
Greg KH
maintainerPATCH 1/1 nvme-rdma: don't suppress send completions
Quoted 18 lines · Keith Busch
Quoted 17 lines · Greg KH
Quoted 15 lines · Sagi Grimberg
The entire completions suppress mechanism is currently broken because the HCA might retry a send operation (due to dropped ack) after the nvme transaction has completed.
In order to handle this, we signal all send completions and introduce a separate done handler for async events as they will be handled differently (as they don't include in-capsule data by definition).
Cc <[email protected]> # v4.14+Signed-off-by Sagi Grimberg <[email protected]>Reviewed-by Max Gurtovoy <[email protected]>Signed-off-by Christoph Hellwig <[email protected]>--- drivers/nvme/host/rdma.c | 54 +++++++++++++----------------------------------- 1 file changed, 14 insertions(+), 40 deletions(-)
Quoted 3 lines
Quoted 1 line
What is the git commit id for this patch in Linus's tree?
Commit
b4b591c87f2b0f4ebaf3a68d4f13873b241aa584upstream
thanks, now queued up.
greg k-h