Re: [V9fs-developer] [PATCH] net/9p: convert to new CQ API

[Doug Ledford]

On 03/08/2016 09:38 AM, Dominique Martinet wrote:
> Christoph Hellwig wrote on Thu, Mar 03, 2016:
>> New version with the nits fixed below.  Now that checkpath started
>> a stupid warning about not using tabs for indentation which I've
>> ignored here and will take up in my usual fights against Joes
>> idicotic opinions separately..
> 
> Thanks for the nitpicks, I can confirm it works as expected as well so
> all good with me.
> I like the new CQ interface :)
> 
> (if someone adds an Acked-by please use dominique.marti...@cea.fr for my
> mail; sorry for the split personality)
> 

Since I haven't heard anyone else say they are picking this up, I've
grabbed it for 4.6.  Thanks.

-- 
Doug Ledford 
  GPG KeyID: 0E572FDD




signature.asc
Description: OpenPGP digital signature

Re: [V9fs-developer] [PATCH] net/9p: convert to new CQ API

[Dominique Martinet]

Christoph Hellwig wrote on Thu, Mar 03, 2016:
> New version with the nits fixed below.  Now that checkpath started
> a stupid warning about not using tabs for indentation which I've
> ignored here and will take up in my usual fights against Joes
> idicotic opinions separately..

Thanks for the nitpicks, I can confirm it works as expected as well so
all good with me.
I like the new CQ interface :)

(if someone adds an Acked-by please use dominique.marti...@cea.fr for my
mail; sorry for the split personality)

-- 
Dominique

Re: [V9fs-developer] [PATCH] net/9p: convert to new CQ API

[Christoph Hellwig]

New version with the nits fixed below.  Now that checkpath started
a stupid warning about not using tabs for indentation which I've
ignored here and will take up in my usual fights against Joes
idicotic opinions separately..

--
>From 0055e31f24c7b3642ece5ebb998fa63d772dcb1a Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Sat, 27 Feb 2016 10:22:40 +0100
Subject: net/9p: convert to new CQ API

Trivial conversion to the new RDMA CQ API.

Signed-off-by: Christoph Hellwig 
---
 net/9p/trans_rdma.c | 86 +++--
 1 file changed, 31 insertions(+), 55 deletions(-)

diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
index 52b4a2f..1852e38 100644
--- a/net/9p/trans_rdma.c
+++ b/net/9p/trans_rdma.c
@@ -109,14 +109,13 @@ struct p9_trans_rdma {
 /**
  * p9_rdma_context - Keeps track of in-process WR
  *
- * @wc_op: The original WR op for when the CQE completes in error.
  * @busa: Bus address to unmap when the WR completes
  * @req: Keeps track of requests (send)
  * @rc: Keepts track of replies (receive)
  */
 struct p9_rdma_req;
 struct p9_rdma_context {
-   enum ib_wc_opcode wc_op;
+   struct ib_cqe cqe;
dma_addr_t busa;
union {
struct p9_req_t *req;
@@ -284,9 +283,12 @@ p9_cm_event_handler(struct rdma_cm_id *id, struct 
rdma_cm_event *event)
 }
 
 static void
-handle_recv(struct p9_client *client, struct p9_trans_rdma *rdma,
-   struct p9_rdma_context *c, enum ib_wc_status status, u32 byte_len)
+recv_done(struct ib_cq *cq, struct ib_wc *wc)
 {
+   struct p9_client *client = cq->cq_context;
+   struct p9_trans_rdma *rdma = client->trans;
+   struct p9_rdma_context *c =
+   container_of(wc->wr_cqe, struct p9_rdma_context, cqe);
struct p9_req_t *req;
int err = 0;
int16_t tag;
@@ -295,7 +297,7 @@ handle_recv(struct p9_client *client, struct p9_trans_rdma 
*rdma,
ib_dma_unmap_single(rdma->cm_id->device, c->busa, client->msize,
 DMA_FROM_DEVICE);
 
-   if (status != IB_WC_SUCCESS)
+   if (wc->status != IB_WC_SUCCESS)
goto err_out;
 
err = p9_parse_header(c->rc, NULL, NULL, , 1);
@@ -316,21 +318,32 @@ handle_recv(struct p9_client *client, struct 
p9_trans_rdma *rdma,
req->rc = c->rc;
p9_client_cb(client, req, REQ_STATUS_RCVD);
 
+ out:
+   up(>rq_sem);
+   kfree(c);
return;
 
  err_out:
-   p9_debug(P9_DEBUG_ERROR, "req %p err %d status %d\n", req, err, status);
+   p9_debug(P9_DEBUG_ERROR, "req %p err %d status %d\n",
+   req, err, wc->status);
rdma->state = P9_RDMA_FLUSHING;
client->status = Disconnected;
+   goto out;
 }
 
 static void
-handle_send(struct p9_client *client, struct p9_trans_rdma *rdma,
-   struct p9_rdma_context *c, enum ib_wc_status status, u32 byte_len)
+send_done(struct ib_cq *cq, struct ib_wc *wc)
 {
+   struct p9_client *client = cq->cq_context;
+   struct p9_trans_rdma *rdma = client->trans;
+   struct p9_rdma_context *c =
+   container_of(wc->wr_cqe, struct p9_rdma_context, cqe);
+
ib_dma_unmap_single(rdma->cm_id->device,
c->busa, c->req->tc->size,
DMA_TO_DEVICE);
+   up(>sq_sem);
+   kfree(c);
 }
 
 static void qp_event_handler(struct ib_event *event, void *context)
@@ -339,42 +352,6 @@ static void qp_event_handler(struct ib_event *event, void 
*context)
 event->event, context);
 }
 
-static void cq_comp_handler(struct ib_cq *cq, void *cq_context)
-{
-   struct p9_client *client = cq_context;
-   struct p9_trans_rdma *rdma = client->trans;
-   int ret;
-   struct ib_wc wc;
-
-   ib_req_notify_cq(rdma->cq, IB_CQ_NEXT_COMP);
-   while ((ret = ib_poll_cq(cq, 1, )) > 0) {
-   struct p9_rdma_context *c = (void *) (unsigned long) wc.wr_id;
-
-   switch (c->wc_op) {
-   case IB_WC_RECV:
-   handle_recv(client, rdma, c, wc.status, wc.byte_len);
-   up(>rq_sem);
-   break;
-
-   case IB_WC_SEND:
-   handle_send(client, rdma, c, wc.status, wc.byte_len);
-   up(>sq_sem);
-   break;
-
-   default:
-   pr_err("unexpected completion type, c->wc_op=%d, 
wc.opcode=%d, status=%d\n",
-  c->wc_op, wc.opcode, wc.status);
-   break;
-   }
-   kfree(c);
-   }
-}
-
-static void cq_event_handler(struct ib_event *e, void *v)
-{
-   p9_debug(P9_DEBUG_ERROR, "CQ event %d context %p\n", e->event, v);
-}
-
 static void rdma_destroy_trans(struct p9_trans_rdma *rdma)
 {
if (!rdma)
@@ -387,7 +364,7 @@ static void rdma_destroy_trans(struct p9_trans_rdma 

Re: [V9fs-developer] [PATCH] net/9p: convert to new CQ API

[Dominique Martinet]

Hi,

Couple of checkpatch complains:

Christoph Hellwig wrote on Sat, Feb 27, 2016:
> -struct p9_rdma_context {
> - enum ib_wc_opcode wc_op;
> +struct p9_rdma_context { 

trailing tab

> - p9_debug(P9_DEBUG_ERROR, "req %p err %d status %d\n", req, err, status);
> + p9_debug(P9_DEBUG_ERROR, "req %p err %d status %d\n", req, err, 
> wc->status);

line over 80 chars


That aside it looks good ; I need to check on the new API (hadn't
noticed the change) but it looks nice.

Will do the actual testing likely only next week only though;
Eric has been taking my patches for 9p/RDMA so I suspect he'll take
your's as well eventually (get_maintainer.pl has a long-ish list of CC
for us usually)


BTW I think it's easy enough to do the testing if you have a server that
can dish it out. diod[1] and nfs-ganesha[2] are the only two I'm aware
of but there might be more (using ganesha myself; happy to help you set
it up in private if you need)

[1] https://github.com/chaos/diod
[2] https://github.com/nfs-ganesha/nfs-ganesha

-- 
Dominique Martinet