Re: [PATCH 10/13] IB/srp: use the new CQ API

[Doug Ledford]

On 12/13/2015 05:26 AM, Sagi Grimberg wrote:
> 
>> Allright.  How do you want to proceed?  The current rdma-cq branch
>> has all kinds of dependencies, but I've also prepared a new rdma-cq.2
>> branch that could go straight on top of your current queue:
>>
>> http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/rdma-cq.2
>>
>> If you're ready to start the 4.5 tree I can send those out as a patch
>> series.
> 
> Will this get on top of iser-remote-inv? Or should I resend atop of this?

I'm going through my inbox right now.  I expect somewhere in there Or
will make his case for why he doesn't like Christoph's patch to get rid
of the attr struct.  I'll listen, and if I'm not convinced, I'll take
that patchset first and this one second. (I reviewed the patchset
alreadyaside from the fact that I *like* having the attr struct
elements in an organized sub-struct, it's fine and it definitely
improves on all of those query calls).

-- 
Doug Ledford 
  GPG KeyID: 0E572FDD




signature.asc
Description: OpenPGP digital signature

Re: [PATCH 10/13] IB/srp: use the new CQ API

[Sagi Grimberg]

Allright.  How do you want to proceed?  The current rdma-cq branch
has all kinds of dependencies, but I've also prepared a new rdma-cq.2
branch that could go straight on top of your current queue:

http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/rdma-cq.2

If you're ready to start the 4.5 tree I can send those out as a patch
series.


Will this get on top of iser-remote-inv? Or should I resend atop of this?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/13] IB/srp: use the new CQ API

[Christoph Hellwig]

On Fri, Dec 11, 2015 at 12:59:01PM -0500, Doug Ledford wrote:
> On 12/11/2015 09:22 AM, Christoph Hellwig wrote:
> > Hi Bart,
> > 
> > thanks for all the reviews.  I've updated the git branch with your
> > suggestions and reviewed-by tags.  I'm going to wait a little bit
> > longer for other reviews to come in before reposting the series.
> 
> Indeed, thanks for all the catches Bart.  This patchset, with Bart's
> fixups, looks good to me.

Allright.  How do you want to proceed?  The current rdma-cq branch
has all kinds of dependencies, but I've also prepared a new rdma-cq.2
branch that could go straight on top of your current queue:

http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/rdma-cq.2

If you're ready to start the 4.5 tree I can send those out as a patch
series.

> 
> 
> -- 
> Doug Ledford 
>   GPG KeyID: 0E572FDD
> 
> 


---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/13] IB/srp: use the new CQ API

[Christoph Hellwig]

Hi Bart,

thanks for all the reviews.  I've updated the git branch with your
suggestions and reviewed-by tags.  I'm going to wait a little bit
longer for other reviews to come in before reposting the series.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/13] IB/srp: use the new CQ API

[Doug Ledford]

On 12/11/2015 09:22 AM, Christoph Hellwig wrote:
> Hi Bart,
> 
> thanks for all the reviews.  I've updated the git branch with your
> suggestions and reviewed-by tags.  I'm going to wait a little bit
> longer for other reviews to come in before reposting the series.

Indeed, thanks for all the catches Bart.  This patchset, with Bart's
fixups, looks good to me.


-- 
Doug Ledford 
  GPG KeyID: 0E572FDD




signature.asc
Description: OpenPGP digital signature

Re: [PATCH 10/13] IB/srp: use the new CQ API

[Bart Van Assche]

On 12/07/2015 12:51 PM, Christoph Hellwig wrote:

+static void srp_send_done(struct ib_cq *cq, struct ib_wc *wc)
+{
+   struct srp_iu *iu = container_of(wc->wr_cqe, struct srp_iu, cqe);
+   struct srp_rdma_ch *ch = cq->cq_context;
+
+   if (likely(wc->status != IB_WC_SUCCESS)) {
+   srp_handle_qp_err(cq, wc, "SEND");
+   return;
+   }
+
+   list_add(>list, >free_tx);
+}


Please change likely() in the above code into unlikely().

Thanks,

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/13] IB/srp: use the new CQ API

[Christoph Hellwig]

This also moves recv completion handling from hardirq context into
softirq context.

Signed-off-by: Christoph Hellwig 
---
 drivers/infiniband/ulp/srp/ib_srp.c | 173 +---
 drivers/infiniband/ulp/srp/ib_srp.h |   7 +-
 2 files changed, 86 insertions(+), 94 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c 
b/drivers/infiniband/ulp/srp/ib_srp.c
index 784dd97..d61489e 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -132,8 +132,9 @@ MODULE_PARM_DESC(ch_count,
 
 static void srp_add_one(struct ib_device *device);
 static void srp_remove_one(struct ib_device *device, void *client_data);
-static void srp_recv_completion(struct ib_cq *cq, void *ch_ptr);
-static void srp_send_completion(struct ib_cq *cq, void *ch_ptr);
+static void srp_recv_done(struct ib_cq *cq, struct ib_wc *wc);
+static void srp_handle_qp_err(struct ib_cq *cq, struct ib_wc *wc,
+   const char *opname);
 static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event);
 
 static struct scsi_transport_template *ib_srp_transport_template;
@@ -445,6 +446,17 @@ static struct srp_fr_pool *srp_alloc_fr_pool(struct 
srp_target_port *target)
  dev->max_pages_per_mr);
 }
 
+static void srp_drain_done(struct ib_cq *cq, struct ib_wc *wc)
+{
+   struct srp_rdma_ch *ch = cq->cq_context;
+
+   complete(>done);
+}
+
+static struct ib_cqe srp_drain_cqe = {
+   .done   = srp_drain_done,
+};
+
 /**
  * srp_destroy_qp() - destroy an RDMA queue pair
  * @ch: SRP RDMA channel.
@@ -461,7 +473,7 @@ static void srp_destroy_qp(struct srp_rdma_ch *ch)
struct ib_recv_wr *bad_wr;
int ret;
 
-   wr.wr_id = SRP_LAST_WR_ID;
+   wr.wr_cqe = _drain_cqe;
/* Destroying a QP and reusing ch->done is only safe if not connected */
WARN_ON_ONCE(ch->connected);
 
@@ -490,34 +502,27 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
struct ib_fmr_pool *fmr_pool = NULL;
struct srp_fr_pool *fr_pool = NULL;
const int m = 1 + dev->use_fast_reg;
-   struct ib_cq_init_attr cq_attr = {};
int ret;
 
init_attr = kzalloc(sizeof *init_attr, GFP_KERNEL);
if (!init_attr)
return -ENOMEM;
 
-   /* + 1 for SRP_LAST_WR_ID */
-   cq_attr.cqe = target->queue_size + 1;
-   cq_attr.comp_vector = ch->comp_vector;
-   recv_cq = ib_create_cq(dev->dev, srp_recv_completion, NULL, ch,
-  _attr);
+   /* queue_size + 1 for ib_drain_qp */
+   recv_cq = ib_alloc_cq(dev->dev, ch, target->queue_size + 1,
+   ch->comp_vector, IB_POLL_SOFTIRQ);
if (IS_ERR(recv_cq)) {
ret = PTR_ERR(recv_cq);
goto err;
}
 
-   cq_attr.cqe = m * target->queue_size;
-   cq_attr.comp_vector = ch->comp_vector;
-   send_cq = ib_create_cq(dev->dev, srp_send_completion, NULL, ch,
-  _attr);
+   send_cq = ib_alloc_cq(dev->dev, ch, m * target->queue_size,
+   ch->comp_vector, IB_POLL_DIRECT);
if (IS_ERR(send_cq)) {
ret = PTR_ERR(send_cq);
goto err_recv_cq;
}
 
-   ib_req_notify_cq(recv_cq, IB_CQ_NEXT_COMP);
-
init_attr->event_handler   = srp_qp_event;
init_attr->cap.max_send_wr = m * target->queue_size;
init_attr->cap.max_recv_wr = target->queue_size + 1;
@@ -559,9 +564,9 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
if (ch->qp)
srp_destroy_qp(ch);
if (ch->recv_cq)
-   ib_destroy_cq(ch->recv_cq);
+   ib_free_cq(ch->recv_cq);
if (ch->send_cq)
-   ib_destroy_cq(ch->send_cq);
+   ib_free_cq(ch->send_cq);
 
ch->qp = qp;
ch->recv_cq = recv_cq;
@@ -581,13 +586,13 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
return 0;
 
 err_qp:
-   ib_destroy_qp(qp);
+   srp_destroy_qp(ch);
 
 err_send_cq:
-   ib_destroy_cq(send_cq);
+   ib_free_cq(send_cq);
 
 err_recv_cq:
-   ib_destroy_cq(recv_cq);
+   ib_free_cq(recv_cq);
 
 err:
kfree(init_attr);
@@ -623,9 +628,10 @@ static void srp_free_ch_ib(struct srp_target_port *target,
if (ch->fmr_pool)
ib_destroy_fmr_pool(ch->fmr_pool);
}
+
srp_destroy_qp(ch);
-   ib_destroy_cq(ch->send_cq);
-   ib_destroy_cq(ch->recv_cq);
+   ib_free_cq(ch->send_cq);
+   ib_free_cq(ch->recv_cq);
 
/*
 * Avoid that the SCSI error handler tries to use this channel after
@@ -1038,7 +1044,13 @@ static int srp_connect_ch(struct srp_rdma_ch *ch, bool 
multich)
}
 }
 
-static int srp_inv_rkey(struct srp_rdma_ch *ch, u32 rkey)
+static void srp_inv_rkey_err_done(struct ib_cq *cq, struct ib_wc *wc)
+{
+