RE: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-23 Thread Steve Wise
> -Original Message- > From: linux-kernel-ow...@vger.kernel.org > [mailto:linux-kernel-ow...@vger.kernel.org] On Behalf Of Sagi Grimberg > Sent: Monday, November 23, 2015 4:36 AM > To: Steve Wise; 'Christoph Hellwig'; linux-r...@vger.kernel.org > Cc: bart.vanass...@sandisk.com;

RE: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-23 Thread Steve Wise
> -Original Message- > From: Sagi Grimberg [mailto:sa...@dev.mellanox.co.il] > Sent: Monday, November 23, 2015 4:29 AM > To: Steve Wise; 'Christoph Hellwig'; linux-r...@vger.kernel.org > Cc: bart.vanass...@sandisk.com; ax...@fb.com; linux-s...@vger.kernel.org; >

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-23 Thread 'Christoph Hellwig'
On Mon, Nov 23, 2015 at 12:35:44PM +0200, Sagi Grimberg wrote: > >> So Maybe we should have: >> void ib_drain_qp(struct ib_qp *qp) > > Christoph suggested that this flushing would be taken care > of by rdma_disconnect which sounds even better I think.. Note that will only work once we've

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-23 Thread Sagi Grimberg
So Maybe we should have: void ib_drain_qp(struct ib_qp *qp) Christoph suggested that this flushing would be taken care of by rdma_disconnect which sounds even better I think.. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-23 Thread Sagi Grimberg
That won't work for iWARP. Is this code new? I didn't see any errors that would result from this code when I tested iSER over cxgb4 with the old iwarp support patches. Steve, I think I figured out why this works with iWARP. For iWARP, rdma_disconnect() calls iw_cm_disconnect() with

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-23 Thread Sagi Grimberg
So Maybe we should have: void ib_drain_qp(struct ib_qp *qp) Christoph suggested that this flushing would be taken care of by rdma_disconnect which sounds even better I think.. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-23 Thread Sagi Grimberg
That won't work for iWARP. Is this code new? I didn't see any errors that would result from this code when I tested iSER over cxgb4 with the old iwarp support patches. Steve, I think I figured out why this works with iWARP. For iWARP, rdma_disconnect() calls iw_cm_disconnect() with

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-23 Thread 'Christoph Hellwig'
On Mon, Nov 23, 2015 at 12:35:44PM +0200, Sagi Grimberg wrote: > >> So Maybe we should have: >> void ib_drain_qp(struct ib_qp *qp) > > Christoph suggested that this flushing would be taken care > of by rdma_disconnect which sounds even better I think.. Note that will only work once we've

RE: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-23 Thread Steve Wise
> -Original Message- > From: Sagi Grimberg [mailto:sa...@dev.mellanox.co.il] > Sent: Monday, November 23, 2015 4:29 AM > To: Steve Wise; 'Christoph Hellwig'; linux-r...@vger.kernel.org > Cc: bart.vanass...@sandisk.com; ax...@fb.com; linux-s...@vger.kernel.org; >

RE: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-23 Thread Steve Wise
> -Original Message- > From: linux-kernel-ow...@vger.kernel.org > [mailto:linux-kernel-ow...@vger.kernel.org] On Behalf Of Sagi Grimberg > Sent: Monday, November 23, 2015 4:36 AM > To: Steve Wise; 'Christoph Hellwig'; linux-r...@vger.kernel.org > Cc: bart.vanass...@sandisk.com;

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-18 Thread Steve Wise
On 11/18/2015 8:06 AM, Christoph Hellwig wrote: On Wed, Nov 18, 2015 at 01:32:19PM +0200, Sagi Grimberg wrote: Christoph, Given the discussion around this patch I think it would be a good idea remove it from the patchset since it's not mandatory for the CQ abstraction. I think that we should

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-18 Thread Christoph Hellwig
On Wed, Nov 18, 2015 at 01:32:19PM +0200, Sagi Grimberg wrote: > Christoph, > > Given the discussion around this patch I think it would > be a good idea remove it from the patchset since it's not > mandatory for the CQ abstraction. I think that we should > take it with Steve to come up with a

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-18 Thread Sagi Grimberg
Christoph, Given the discussion around this patch I think it would be a good idea remove it from the patchset since it's not mandatory for the CQ abstraction. I think that we should take it with Steve to come up with a complete solution for this bit. Thoughts? -- To unsubscribe from this list:

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-18 Thread Steve Wise
On 11/18/2015 8:06 AM, Christoph Hellwig wrote: On Wed, Nov 18, 2015 at 01:32:19PM +0200, Sagi Grimberg wrote: Christoph, Given the discussion around this patch I think it would be a good idea remove it from the patchset since it's not mandatory for the CQ abstraction. I think that we should

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-18 Thread Sagi Grimberg
Christoph, Given the discussion around this patch I think it would be a good idea remove it from the patchset since it's not mandatory for the CQ abstraction. I think that we should take it with Steve to come up with a complete solution for this bit. Thoughts? -- To unsubscribe from this list:

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-18 Thread Christoph Hellwig
On Wed, Nov 18, 2015 at 01:32:19PM +0200, Sagi Grimberg wrote: > Christoph, > > Given the discussion around this patch I think it would > be a good idea remove it from the patchset since it's not > mandatory for the CQ abstraction. I think that we should > take it with Steve to come up with a

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-17 Thread Sagi Grimberg
On 17/11/2015 19:06, Bart Van Assche wrote: On 11/15/2015 01:34 AM, Sagi Grimberg wrote: This is taken from srp, and srp drains using a recv wr due to a race causing a use-after-free condition in srp which re-posts a recv buffer in the recv completion handler. Hello Sagi, Would it be

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-17 Thread Bart Van Assche
On 11/15/2015 01:34 AM, Sagi Grimberg wrote: This is taken from srp, and srp drains using a recv wr due to a race causing a use-after-free condition in srp which re-posts a recv buffer in the recv completion handler. Hello Sagi, Would it be possible to clarify this ? Does this refer to an

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-17 Thread Sagi Grimberg
That won't work for iWARP. Is this code new? I didn't see any errors that would result from this code when I tested iSER over cxgb4 with the old iwarp support patches. It's there since ~3.17 I think... Perhaps we need another way to do this? Like a completion object in the QP that

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-17 Thread Bart Van Assche
On 11/15/2015 01:34 AM, Sagi Grimberg wrote: This is taken from srp, and srp drains using a recv wr due to a race causing a use-after-free condition in srp which re-posts a recv buffer in the recv completion handler. Hello Sagi, Would it be possible to clarify this ? Does this refer to an

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-17 Thread Sagi Grimberg
That won't work for iWARP. Is this code new? I didn't see any errors that would result from this code when I tested iSER over cxgb4 with the old iwarp support patches. It's there since ~3.17 I think... Perhaps we need another way to do this? Like a completion object in the QP that

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-17 Thread Sagi Grimberg
On 17/11/2015 19:06, Bart Van Assche wrote: On 11/15/2015 01:34 AM, Sagi Grimberg wrote: This is taken from srp, and srp drains using a recv wr due to a race causing a use-after-free condition in srp which re-posts a recv buffer in the recv completion handler. Hello Sagi, Would it be

RE: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-16 Thread Steve Wise
> -Original Message- > From: linux-rdma-ow...@vger.kernel.org > [mailto:linux-rdma-ow...@vger.kernel.org] On Behalf Of Sagi Grimberg > Sent: Monday, November 16, 2015 12:38 PM > To: Steve Wise; 'Christoph Hellwig'; linux-r...@vger.kernel.org > Cc: bart.vanass...@sandisk.com;

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-16 Thread Sagi Grimberg
After looking at the nes driver, I don't see any common way to support drain w/o some serious driver mods. Since SRP is the only user, perhaps we can ignore iWARP for this function... But iser/isert essentially does it too (and I think xprtrdma will have it soon)... the modify_qp is

RE: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-16 Thread Steve Wise
> -Original Message- > From: Steve Wise [mailto:sw...@opengridcomputing.com] > Sent: Monday, November 16, 2015 10:38 AM > To: Sagi Grimberg; Christoph Hellwig; linux-r...@vger.kernel.org > Cc: bart.vanass...@sandisk.com; ax...@fb.com; linux-s...@vger.kernel.org; >

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-16 Thread Steve Wise
On 11/15/2015 3:34 AM, Sagi Grimberg wrote: + +struct ib_stop_cqe { +struct ib_cqecqe; +struct completion done; +}; + +static void ib_stop_done(struct ib_cq *cq, struct ib_wc *wc) +{ +struct ib_stop_cqe *stop = +container_of(wc->wr_cqe, struct ib_stop_cqe, cqe); + +

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-16 Thread Steve Wise
On 11/15/2015 3:34 AM, Sagi Grimberg wrote: + +struct ib_stop_cqe { +struct ib_cqecqe; +struct completion done; +}; + +static void ib_stop_done(struct ib_cq *cq, struct ib_wc *wc) +{ +struct ib_stop_cqe *stop = +container_of(wc->wr_cqe, struct ib_stop_cqe, cqe); + +

RE: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-16 Thread Steve Wise
> -Original Message- > From: linux-rdma-ow...@vger.kernel.org > [mailto:linux-rdma-ow...@vger.kernel.org] On Behalf Of Sagi Grimberg > Sent: Monday, November 16, 2015 12:38 PM > To: Steve Wise; 'Christoph Hellwig'; linux-r...@vger.kernel.org > Cc: bart.vanass...@sandisk.com;

RE: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-16 Thread Steve Wise
> -Original Message- > From: Steve Wise [mailto:sw...@opengridcomputing.com] > Sent: Monday, November 16, 2015 10:38 AM > To: Sagi Grimberg; Christoph Hellwig; linux-r...@vger.kernel.org > Cc: bart.vanass...@sandisk.com; ax...@fb.com; linux-s...@vger.kernel.org; >

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-16 Thread Sagi Grimberg
After looking at the nes driver, I don't see any common way to support drain w/o some serious driver mods. Since SRP is the only user, perhaps we can ignore iWARP for this function... But iser/isert essentially does it too (and I think xprtrdma will have it soon)... the modify_qp is

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-15 Thread Sagi Grimberg
+ +struct ib_stop_cqe { + struct ib_cqe cqe; + struct completion done; +}; + +static void ib_stop_done(struct ib_cq *cq, struct ib_wc *wc) +{ + struct ib_stop_cqe *stop = + container_of(wc->wr_cqe, struct ib_stop_cqe, cqe); + + complete(>done); +} + +/* +

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-15 Thread Sagi Grimberg
+ +struct ib_stop_cqe { + struct ib_cqe cqe; + struct completion done; +}; + +static void ib_stop_done(struct ib_cq *cq, struct ib_wc *wc) +{ + struct ib_stop_cqe *stop = + container_of(wc->wr_cqe, struct ib_stop_cqe, cqe); + + complete(>done); +} + +/* +

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-13 Thread Christoph Hellwig
On Fri, Nov 13, 2015 at 10:16:04AM -0600, Steve Wise wrote: > So how can we do this for iwarp? It seems like all that might be needed is > to modify the QP state to idle, retrying until it succeeds: > >If the QP is transitioning to the Error state, or has not yet >finished flushing the

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-13 Thread Steve Wise
On 11/13/2015 7:46 AM, Christoph Hellwig wrote: Signed-off-by: Christoph Hellwig --- drivers/infiniband/core/cq.c | 46 include/rdma/ib_verbs.h | 2 ++ 2 files changed, 48 insertions(+) diff --git a/drivers/infiniband/core/cq.c

[PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-13 Thread Christoph Hellwig
Signed-off-by: Christoph Hellwig --- drivers/infiniband/core/cq.c | 46 include/rdma/ib_verbs.h | 2 ++ 2 files changed, 48 insertions(+) diff --git a/drivers/infiniband/core/cq.c b/drivers/infiniband/core/cq.c index d9eb796..bf2a079 100644 ---

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-13 Thread Steve Wise
On 11/13/2015 7:46 AM, Christoph Hellwig wrote: Signed-off-by: Christoph Hellwig --- drivers/infiniband/core/cq.c | 46 include/rdma/ib_verbs.h | 2 ++ 2 files changed, 48 insertions(+) diff --git

Re: [PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-13 Thread Christoph Hellwig
On Fri, Nov 13, 2015 at 10:16:04AM -0600, Steve Wise wrote: > So how can we do this for iwarp? It seems like all that might be needed is > to modify the QP state to idle, retrying until it succeeds: > >If the QP is transitioning to the Error state, or has not yet >finished flushing the

[PATCH 3/9] IB: add a helper to safely drain a QP

2015-11-13 Thread Christoph Hellwig
Signed-off-by: Christoph Hellwig --- drivers/infiniband/core/cq.c | 46 include/rdma/ib_verbs.h | 2 ++ 2 files changed, 48 insertions(+) diff --git a/drivers/infiniband/core/cq.c b/drivers/infiniband/core/cq.c index