[PATCH v1 03/10] IB/iser: Don't register memory for all immediatedata writes

havetz <jen...@mellanox.com> Signed-off-by: Sagi Grimberg <sa...@mellanox.com> --- drivers/infiniband/ulp/iser/iscsi_iser.h | 3 ++- drivers/infiniband/ulp/iser/iser_initiator.c | 5 +++-- drivers/infiniband/ulp/iser/iser_memory.c| 13 + 3 files changed, 14 insertio

[PATCH v1 00/10] iSER support for remote invalidate

er: set intuitive values for mr_valid iser-target: Declare correct flags when accepting a connection iser-target: Support the remote invalidation exception IB/iser: Support the remote invalidation exception Roi Dayan (1): IB/iser: Fix module init not cleaning up on error flow Sagi Grimbe

[PATCH v1 01/10] IB/iser: Fix module init not cleaning up on error flow

From: Roi Dayan <r...@mellanox.com> Destroy workqueue on transport register error, also release kmem cache on workqueue alloc error. Signed-off-by: Roi Dayan <r...@mellanox.com> Signed-off-by: Sagi Grimberg <sa...@mellanox.com> Reviewed-by: Christoph Hellwig <h...@lst.de>

[PATCH v1 06/10] iser-target: Remove unused file iser_proto.h

We don't need iser_proto.h anymore, remove it and move (non-protocol) declarations to ib_isert.h Signed-off-by: Sagi Grimberg <sa...@mellanox.com> Signed-off-by: Jenny Derzhavetz <jen...@mellanox.com> --- drivers/infiniband/ulp/isert/ib_isert.c| 1 - drivers/infiniband/ulp/iser

[PATCH v1 09/10] IB/iser: Increment the rkey when registering and not when invalidating

With remote invalidate we won't local invalidate but we still want to increment the rkey. Signed-off-by: Sagi Grimberg <sa...@mellanox.com> Signed-off-by: Jenny Derzhavetz <jen...@mellanox.com> --- drivers/infiniband/ulp/iser/iser_memory.c | 20 ++-- 1 file changed, 1

Re: [PATCH v1 3/9] xprtrdma: Introduce ro_unmap_sync method

On 24/11/2015 08:45, Christoph Hellwig wrote: On Mon, Nov 23, 2015 at 05:14:14PM -0500, Chuck Lever wrote: In the current xprtrdma implementation, some memreg strategies implement ro_unmap synchronously (the MR is knocked down before the method returns) and some asynchonously (the MR will be

Re: [PATCH for-next 03/10] IB/iser: Don't register memory for all immediatedata writes

Jason and Or, I'm with Or on this, this is really goofy looking. This routine probably should not be setting the rkey at all, it makes no sense to have a routine that returns a lkey and a rkey. Those are always different flows. Once that is fixed then the above if can be hoisted to the

Re: [PATCH v1 3/9] xprtrdma: Introduce ro_unmap_sync method

Hey Tom, On 11/24/2015 5:59 AM, Sagi Grimberg wrote: As I see it, if we don't wait for local-invalidate to complete before unmap and IO completion (and no one does) For the record, that is false. Windows quite strictly performs these steps, I meant Linux ULPs. I'm not very familiar

Re: [PATCH v1 3/9] xprtrdma: Introduce ro_unmap_sync method

Hey Chuck, It is painful, too painful. The entire value proposition of RDMA is low-latency and waiting for the extra HW round-trip for a local invalidation to complete is unacceptable, moreover it adds a huge loads of extra interrupts and cache-line pollutions. The killer is the extra

Re: memory registration updates

of the series which is a cleanup by nature. I suggest you send out patches 10,11 as a separate RFC series. For patches 1-9, Reviewed-by: Sagi Grimberg <sa...@mellanox.com> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...

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

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-rdma" in the body of a message to

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

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: memory registration updates

I send 1-9 out separately earlier :) The other two sit on top of them and they are prep patches in a sense as they remove a lot of users of struct ib_mr that i don't have to modify in patches 10 and 11. Still, patches 10,11 are not really a part of this patchset. I think they need to stand

Re: srp state in current mainline

On 22/11/2015 17:10, Christoph Hellwig wrote: On Sun, Nov 22, 2015 at 04:55:49PM +0200, Sagi Grimberg wrote: Also note that 4.4-rc prefer_fr=y register_always=n !register_always still blows up badly with XFS and ext4 due to data integrity errors. So the register_always=N makes bad things

Re: srp state in current mainline

On 22/11/2015 16:32, Christoph Hellwig wrote: On Sun, Nov 22, 2015 at 05:53:43AM -0800, Christoph Hellwig wrote: To me this sounds like another argument to just allocate one FR per request and don't allow non-contiguous SGLs. Also note that 4.4-rc prefer_fr=y register_always=n

Re: [PATCH 2/9] IB: add a proper completion queue abstraction

I think that bart wants to allow the caller to select cpu affinity per CQ. In this case ib_alloc_cq in workqueue mode would need to accept a affinity_hint from the caller (default to wild-card WORK_CPU_UNBOUND). Hmm, true. How would be set that hint from userspace? I'd really prefer to see

Re: [PATCH 2/9] IB: add a proper completion queue abstraction

Hello Christoph, The comment about locality in the above quote is interesting. How about modifying patch 2/9 as indicated below ? The modification below does not change the behavior of this patch if ib_cq.w.cpu is not modified. And it allows users who care about locality and who want to skip

Re: [PATCH] IB/cma: Add a missing rcu_read_unlock()

l.org> Looks good, Reviewed-by: Sagi Grimberg <sa...@mellanox.com> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/iser: use sector_div instead of do_div

:224:22: warning: passing argument 1 of '__div64_32' from incompatible pointer type This changes the code to use sector_div instead, which always produces optimal code. Signed-off-by: Arnd Bergmann <a...@arndb.de> Thanks Arnd, Reviewed-by: Sagi Grimberg <sa...@mellanox.com> -- To

Re: [PATCH] IB/srp: Fix a memory leak

ite+0x2f/0xf0 [] vfs_write+0xa4/0x100 [] SyS_write+0x54/0xc0 [] entry_SYSCALL_64_fastpath+0x12/0x6f Signed-off-by: Bart Van Assche <bart.vanass...@sandisk.com> Cc: Sagi Grimberg <sa...@mellanox.com> Cc: Sebastian Parschauer <sebastian.rie...@profitbricks.com> Cc: Christoph H

Re: [PATCH for-next 05/10] iser: Have initiator and target to share protocol structures and definitions

On 19/11/2015 09:20, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: +/** + * struct iser_hello - iSER Hello header + * + * @opcode: opcode (must be set to ISER_HELLO) + * @max_min_ver: maximum and minimum iser versions + * @iser_ird: iSER IRD + * @rsvd

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

Sagi, do we still do IN, OUT, both or none? if not, where this stopped to be supported? how large would be the fix? Or, it's hard for me to say where exactly it stopped being supported because I've never tested it and I'm not convinced it was ever supported. I'm exhausted with this

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

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 for-next 10/10] IB/iser: Support the remote invalidation exception

AFAIR, in the past there weren't explicit means to do that. What's makes iscsi tcp eligible to support bidi's that we don't have @ iser? In theory, nothing. In practice, iser is missing customer demands, iser targets that support bidi and testing... If someone cared enough about it then I

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

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

Re: [PATCH 4/9] srpt: chain RDMA READ/WRITE requests

On 18/11/2015 03:17, Bart Van Assche wrote: On 11/13/2015 05:46 AM, Christoph Hellwig wrote: -ret = ib_post_send(ch->qp, , _wr); -if (ret) -break; +if (i == n_rdma - 1) { +/* only get completion event for the last rdma read */ +if

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

Sagi, it works in TGT and AFAIR with the initiator too. Looking on this paper of Pete Wyckoff [1] I see that he says that few changes to the initiator were needed, not sure which. I see. I wasn't aware that TGT supports bidi. However, AFAICT the initiator support was never fully introduced

Re: [PATCH for-next 02/10] IB/iser: Default to fastreg instead of fmr

Why? the invalidate is just one part of the story, we are doing a mapping on IO submission and CX3 has strong ordering on FRWRs, right? Yes, this is correct. We'll test on CX3 to see if this introduces a regression. We should make sure not to introduce performance regression for HW which

Re: [PATCH for-next 06/10] iser-target: Remove unused file iser_proto.h

On 17/11/2015 09:57, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: -/* From iscsi_iser.h */ - -struct iser_hdr { -u8flags; -u8rsvd[3]; -__be32write_stag; /* write rkey */ -__be64write_va; -__be32read_stag; /* read rkey */ -__be64

Re: [PATCH for-next 00/10] iSER support for remote invalidate

On 17/11/2015 10:10, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: This patchset adds remote invalidation support to iser initiator and target. The support negotiation for this feature is based on IBTA annex 12 "Support for iSCSI Extensions for RDMA" carried in rdma_

Re: [PATCH for-next 03/10] IB/iser: Don't register memory for all immediatedata writes

It's because device->mr might not be allocated at all if always_register=Y, however in this case for all-immediatedata writes I don't need memory registration and I can use pd->local_dma_lkey. This hunk prevents a NULL dereference (as I mentioned, device->mr might not be allocated at all). I

Re: [PATCH for-next 01/10] IB/iser: Fix module init not cleaning up on error flow

Sagi and Co Can you make sure to avoid skipping usage of capital letters when beginning a sentence and use punctuations: comma/s, period/s? Will do. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo

Re: [PATCH for-next 04/10] IB/iser: set intuitive values for mr_valid

On 17/11/2015 09:43, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: This parameter is described as "is mr valid indicator". In other words, it indicates whether memory registration is valid or not. So intuitive values would be: mr_valid=True, when memory registratio

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

On 17/11/2015 10:05, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: +if (iser_task->dir[ISER_DIR_IN]) +reg = _task->rdma_reg[ISER_DIR_IN]; +else +reg = _task->rdma_reg[ISER_DIR_OUT]; and what happens with bidi

Re: [PATCH for-next 02/10] IB/iser: Default to fastreg instead of fmr

On 17/11/2015 10:57, Christoph Hellwig wrote: + if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) { + iser_info("FastReg supported, using FastReg for registration\n"); + device->reg_ops = _ops; + } else if

Re: [PATCH for-next 02/10] IB/iser: Default to fastreg instead of fmr

On 11/16/2015 6:37 PM, Sagi Grimberg wrote: This is a pre-step before adding remote invalidate support. Wouldn't that introduce performance regression on ConnectX3 devices? With remote invalidate it shouldn't make much of a difference. Plus, I'd really like us to start phasing out from

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

Why? we don't support and when did we broke it after the initial 2.6.18 upstreaming of the driver Also, do we refuse to queuecommand a bidi? where? Or, bidirectional support is not assumed and needs to be actively set as a request queue flag (see iscsi_sw_tcp_slave_alloc()). AFAICT iser

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

On 17/11/2015 10:03, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: void iser_rcv_completion(struct iser_rx_desc *rx_desc, - unsigned long rx_xfer_len, + struct ib_wc *wc, struct ib_conn *ib_conn) { struct iser_conn *iser_conn

Re: [PATCH for-next 07/10] iser-target: Declare correct flags when accepting a connection

On 11/16/2015 6:37 PM, Sagi Grimberg wrote: From: Jenny Derzhavetz<jen...@mellanox.com> iser target does not support zero based virtual addresses and send with invalidate, so it should declare that it doesn't. This better go to stable kernels too, however there's little ugliness in

Re: [PATCH for-next 08/10] iser-target: Support the remote invalidation exception

On 17/11/2015 10:09, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: We'll use remote invalidate, according to negotiation result during connection establishment. If the initiator declared that it supports the remote invalidate exception then the target will use

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

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 for-next 07/10] iser-target: Declare correct flags when accepting a connection

+ struct iser_cm_hdr rsp_hdr; memset(, 0, sizeof(struct rdma_conn_param)); cp.initiator_depth = isert_conn->initiator_depth; cp.retry_count = 7; cp.rnr_retry_count = 7; + memset(_hdr, 0, sizeof(rsp_hdr)); + rsp_hdr.flags = (ISERT_ZBVA_NOT_USED

Re: [PATCH for-next 06/10] iser-target: Remove unused file iser_proto.h

On 17/11/2015 11:00, Christoph Hellwig wrote: Sorry for the nitpicking, but: +/*Constant PDU lengths calculations */ This really needs a space before the 'C'. +#define ISER_HEADERS_LEN (sizeof(struct iser_ctrl) + sizeof(struct iscsi_hdr)) Also fixing lines over 8- characters

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

On 17/11/2015 10:08, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: --- a/drivers/infiniband/ulp/iser/iser_verbs.c +++ b/drivers/infiniband/ulp/iser/iser_verbs.c @@ -847,7 +847,7 @@ static void iser_route_handler(struct rdma_cm_id *cma_id) conn_param.rnr_retry_count

Re: [PATCH for-next 08/10] iser-target: Support the remote invalidation exception

On 17/11/2015 12:14, Christoph Hellwig wrote: On Tue, Nov 17, 2015 at 11:50:15AM +0200, Sagi Grimberg wrote: On 17/11/2015 10:09, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: We'll use remote invalidate, according to negotiation result during connection establishment

Re: [PATCH for-next 03/10] IB/iser: Don't register memory for all immediatedata writes

On 11/16/2015 6:37 PM, Sagi Grimberg wrote: --- a/drivers/infiniband/ulp/iser/iser_memory.c +++ b/drivers/infiniband/ulp/iser/iser_memory.c @@ -250,7 +250,7 @@ iser_reg_dma(struct iser_device *device, struct iser_data_buf *mem, struct scatterlist *sg = mem->sg; reg->sg

Re: [PATCH 2/9] IB: add a proper completion queue abstraction

Hi Bart, + */ +void ib_process_cq_direct(struct ib_cq *cq) +{ +WARN_ON_ONCE(cq->poll_ctx != IB_POLL_DIRECT); + +__ib_process_cq(cq, INT_MAX); +} +EXPORT_SYMBOL(ib_process_cq_direct); My proposal is to drop this function and to export __ib_process_cq() instead (with or without

[PATCH for-next 03/10] IB/iser: Don't register memory for all immediatedata writes

From: Jenny Derzhavetz <jen...@mellanox.com> When all the task data is sent as immeidatedata, we are allowed to use the local_dma_lkey as it is not sent to the wire. Signed-off-by: Jenny Derzhavetz <jen...@mellanox.com> Signed-off-by: Sagi Grimberg <sa...@mellanox.com> ---

[PATCH for-next 01/10] IB/iser: Fix module init not cleaning up on error flow

From: Roi Dayan <r...@mellanox.com> destroy workqueue on transport register error release kmem cache on workqueue alloc error Signed-off-by: Roi Dayan <r...@mellanox.com> Signed-off-by: Sagi Grimberg <sa...@mellanox.com> --- drivers/infiniband/ulp/iser/iscsi_iser.c | 9 ++-

[PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

sponse) completion. Signed-off-by: Jenny Derzhavetz <jen...@mellanox.com> Signed-off-by: Sagi Grimberg <sa...@mellanox.com> --- drivers/infiniband/ulp/iser/iscsi_iser.h | 3 +- drivers/infiniband/ulp/iser/iser_initiator.c | 55 +++- drivers/infiniband/ulp

[PATCH for-next 06/10] iser-target: Remove unused file iser_proto.h

We don't need iser_proto.h anymore, remove it and move (non-protocol) declarations to ib_isert.h Signed-off-by: Sagi Grimberg <sa...@mellanox.com> Signed-off-by: Jenny Derzhavetz <jen...@mellanox.com> --- drivers/infiniband/ulp/isert/ib_isert.c| 1 - drivers/infiniband/ulp/iser

[PATCH for-next 07/10] iser-target: Declare correct flags when accepting a connection

From: Jenny Derzhavetz <jen...@mellanox.com> iser target does not support zero based virtual addresses and send with invalidate, so it should declare that it doesn't. Signed-off-by: Jenny Derzhavetz <jen...@mellanox.com> Signed-off-by: Sagi Grimberg <sa...@mellanox.com> ---

[PATCH for-next 09/10] IB/iser: Increment the rkey when registering and not when invalidating

With remote invalidate we won't local invalidate but we still want to increment the rkey. Signed-off-by: Sagi Grimberg <sa...@mellanox.com> Signed-off-by: Jenny Derzhavetz <jen...@mellanox.com> --- drivers/infiniband/ulp/iser/iser_memory.c | 20 ++-- 1 file changed, 1

[PATCH for-next 04/10] IB/iser: set intuitive values for mr_valid

ise. Signed-off-by: Jenny Derzhavetz <jen...@mellanox.com> Signed-off-by: Sagi Grimberg <sa...@mellanox.com> --- drivers/infiniband/ulp/iser/iser_memory.c | 8 drivers/infiniband/ulp/iser/iser_verbs.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a

[PATCH for-next 05/10] iser: Have initiator and target to share protocol structures and definitions

The iser RDMA_CM negotiation protocol is shared by the initiator and the target, so have a shared header for the defines and structure. Move relevant items from the initiator and target headers. Signed-off-by: Sagi Grimberg <sa...@mellanox.com> Signed-off-by: Jenny Derzhavetz <jen...@mel

[PATCH for-next 08/10] iser-target: Support the remote invalidation exception

sponse. Signed-off-by: Jenny Derzhavetz <jen...@mellanox.com> Signed-off-by: Sagi Grimberg <sa...@mellanox.com> --- drivers/infiniband/ulp/isert/ib_isert.c | 39 +++-- drivers/infiniband/ulp/isert/ib_isert.h | 2 ++ 2 files changed, 34 insertions(+), 7

Re: [PATCH 4/9] IB: remove in-kernel support for memory windows

Remove the unused ib_allow_mw and ib_bind_mw functions, remove the unused IB_WR_BIND_MW and IB_WC_BIND_MW opcodes and move ib_dealloc_mw into the uverbs module. Signed-off-by: Christoph Hellwig Will the user-space drivers posting via uverbs (qib, hfi, rxe) need the post_send

Re: [PATCH 4/9] IB: remove in-kernel support for memory windows

On 16/11/2015 19:02, Christoph Hellwig wrote: On Mon, Nov 16, 2015 at 07:00:06PM +0200, Sagi Grimberg wrote: Remove the unused ib_allow_mw and ib_bind_mw functions, remove the unused IB_WR_BIND_MW and IB_WC_BIND_MW opcodes and move ib_dealloc_mw into the uverbs module. Signed-off

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

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] ib_srp: initialize dma_length in srp_map_idb

On 15/11/2015 23:10, Or Gerlitz wrote: On Sun, Nov 15, 2015, Sagi Grimberg <sa...@dev.mellanox.co.il> wrote: On 15/11/2015 19:59, Christoph Hellwig wrote: Without this sg_dma_len will return 0 on architectures tha have the dma_length field. and what wrong with that? Becaus

Re: [PATCH] IB/mad: In validate_mad, validate CM method and attribute

Hello Hal, With which SRP target has this behavior been observed ? Has this patch been tested with the LIO SRP target ? Hi Bart, This issue was detected when testing a new array with SRP support. This does not involve LIO as the Linux CM stack does not behave in the way described in this

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

+ +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 1/9] move blk_iopoll to limit and make it generally available

On Fri, Nov 13, 2015 at 3:46 PM, Christoph Hellwig wrote: The new name is irq_poll as iopoll is already taken. Better suggestions welcome. Sagi (or Christoph if you can address that), @ some pointer over the last 18 months there was a port done at mellanox for iser to use

Re: [PATCH 2/9] IB: add a proper completion queue abstraction

On 15/11/2015 14:55, Christoph Hellwig wrote: On Sun, Nov 15, 2015 at 11:40:02AM +0200, Sagi Grimberg wrote: I doubt INT_MAX is useful as a budget in any use-case. it can easily hog the CPU. If the consumer is given access to poll a CQ, it must be able to provide some way to budget it. Why

Re: [PATCH 1/9] move blk_iopoll to limit and make it generally available

On 15/11/2015 11:04, Or Gerlitz wrote: On Sun, Nov 15, 2015 at 10:48 AM, Sagi Grimberg <sa...@dev.mellanox.co.il> wrote: Or is correct, I have attempted to convert iser to use blk_iopoll in the past, however I've seen inconsistent performance and latency skews (comparing to tasklet

Re: [PATCH] ib_srp: initialize dma_length in srp_map_idb

We should really get this properly map/unmap per IO at some point. Probably do it in both code paths... Having said that, Looks fine, Reviewed-by: Sagi Grimberg <sa...@mellanox.com> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord..

Re: [PATCH RFC 2/3] svcrdma: Use device rdma_read_access_flags

Jason, It is always acceptable to use a lkey MR instead of the local dma lkey, but ULPs should prefer to use the local dma lkey if possible, for performance reasons. I don't necessarily agree with this statement (at least with the second part of it), the world is not always perfect. For RDMA

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

On 10/11/2015 15:41, Christoph Hellwig wrote: FYI, this is the API I'd aim for (only SRP and no HW driver converted yet): This looks fine, although personally I find scope and direction flags more confusing than access_flags (but maybe it's just me). I think that the real issue here is the

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

On 11/11/2015 10:08, Christoph Hellwig wrote: On Tue, Nov 10, 2015 at 11:01:56AM -0700, Jason Gunthorpe wrote: No need to change every driver. I'd suggest something like unsigned int rdma_cap_rdma_read_mr_flags(const struct ib_pd *pd) { if (rdma_protocol_iwarp(pd->device,

Re: [PATCH RFC 2/3] svcrdma: Use device rdma_read_access_flags

I’d like to see our NFS server use the local DMA lkey where it makes sense, to avoid the cost of registering and invalidating memory. I have to agree with Tom that once the device’s s/g limit is exceeded, the server has to post an RDMA Read WR every few pages, and appears to get expensive

Re: srp state in current mainline

On 11/11/2015 18:18, Christoph Hellwig wrote: On Wed, Nov 11, 2015 at 08:03:46AM -0800, Bart Van Assche wrote: Hello Christoph, The SRP initiator from kernel 4.3 is working fine on my test setup. I will start a test with Linus' tree and with the following SRP kernel module parameters: # cat

Re: [PATCH v2 0/2] Handle mlx4 max_sge_rd correctly

On 28/10/2015 13:28, Sagi Grimberg wrote: This addresses a specific mlx4 issue where the max_sge_rd is actually smaller than max_sge (rdma reads with max_sge entries completes with error). The second patch removes the explicit work-around from the iser target code. Changes from v1: - Fixed

Re: [PATCH] IB/srp: Fix possible send queue overflow

Hi Doug, Kind reminder for picking this up for 4.4 Doug? Are you planning to pick this up? Note that this patch is stable material as well. Doug? any plans for this patch? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

On 10/11/2015 14:28, Sagi Grimberg wrote: Hi Yann, Why were those hw providers not modified to enforce IB_ACCESS_REMOTE_WRITE when needed, instead of asking users to set it for them ? Do you mean that ULPs will set IB_ACCESS_LOCAL_WRITE and iWARP providers executing the memory

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

Sagi, the Windows NDKPI has an NDK_MR_FLAG_RDMA_READ_SINK attribute which the upper layer can use to convey this information, I've mentioned it here before. https://msdn.microsoft.com/en-us/library/windows/hardware/hh439908(v=vs.85).aspx Thanks for the tip Tom. When this approach is used,

Re: [PATCH RFC 2/3] svcrdma: Use device rdma_read_access_flags

On 10/11/2015 13:38, Christoph Hellwig wrote: On Tue, Nov 10, 2015 at 12:44:14PM +0200, Sagi Grimberg wrote: --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c @@ -238,7 +238,7 @@ int rdma_read_chunk_frmr(struct svcxprt_rdma *xprt, read = min_t

[PATCH RFC 2/3] svcrdma: Use device rdma_read_access_flags

Instead of hard-coding remote access (which is not secured issue in IB). Signed-off-by: Sagi Grimberg <sa...@mellanox.com> --- net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/

Re: [PATCH RFC 2/3] svcrdma: Use device rdma_read_access_flags

On 10/11/2015 13:41, Christoph Hellwig wrote: Oh, and while we're at it. Can someone explain why we're even using rdma_read_chunk_frmr for IB? It seems to work around the fact tat iWarp only allow a single RDMA READ SGE, but it's used whenever the device has IB_DEVICE_MEM_MGT_EXTENSIONS,

Re: [PATCH RFC 3/3] RDS_IW: Use device rdma_read_access_flags

Looks reasonable, although currently this code is only used for iWarp anyway. I know... I'm hoping this will change at some point, and when it does, it will get it right hopefully. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

From all I can tell nes also is a iWarp driver. It is.. I don't know why I treated it as IB :) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH RFC 0/3] Introduce device attribute rdma_read_access_flags

attributes merge into struct ib_device. Sagi Grimberg (3): IB/core: Expose a device attribute for rdma_read access flags svcrdma: Use device rdma_read_access_flags RDS_IW: Use device rdma_read_access_flags drivers/infiniband/hw/cxgb3/iwch_provider.c | 2 ++ drivers/infiniband/hw/cxgb4

[PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

Signed-off-by: Sagi Grimberg <sa...@mellanox.com> --- drivers/infiniband/hw/cxgb3/iwch_provider.c | 2 ++ drivers/infiniband/hw/cxgb4/provider.c | 2 ++ drivers/infiniband/hw/mlx4/main.c| 1 + drivers/infiniband/hw/mlx5/main.c| 1 + drivers/infiniband/hw

Re: [PATCH RFC 0/3] Introduce device attribute rdma_read_access_flags

FYI, I've updated the git branch to be based on current linus' tree which required a few bit to be fixed. I'd also like to note that while everyone but Or seemed to be generally fine with it I'd really prefer and actualy revivewed-by or acked-by tag. You can add: Tested-by: Sagi Grimberg

Re: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

Hi Yann, Why were those hw providers not modified to enforce IB_ACCESS_REMOTE_WRITE when needed, instead of asking users to set it for them ? Do you mean that ULPs will set IB_ACCESS_LOCAL_WRITE and iWARP providers executing the memory registration will add IB_ACCESS_REMOTE_WRITE? That's

Re: [PATCH] IB: start documenting device capabilities

which must support FRs to comply +* to the iWarp verbs spec. iWarp devices also support the +* IB_WR_RDMA_READ_WITH_INV verb for RDMA READs that invalidate the +* stag. +*/ Kinda weird that READ_WITH_INV came in without a device cap for it. Looks good, Reviewe

Re: [PATCH 1/7] IB/srp: Fix a spelling error

Looks good, Reviewed-by: Sagi Grimberg <sa...@mellanox.com> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] IB/srp: Rename work request ID labels

Looks good, Reviewed-by: Sagi Grimberg <sa...@mellanox.com> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] IB/srp: Document srp_map_data() return value

Looks good, Reviewed-by: Sagi Grimberg <sa...@mellanox.com> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] IB/srp: Avoid that mapping failure triggers an infinite loop

On 03/11/2015 20:56, Bart Van Assche wrote: On 11/03/2015 09:44 AM, Sagi Grimberg wrote: Can you spare a few words on this change in the change log? Signed-off-by: Bart Van Assche <bart.vanass...@sandisk.com> Cc: Sagi Grimberg <sa...@mellanox.com> Cc: Sebastian Parschauer &l

Re: [PATCH] IB/srp: Fix possible send queue overflow

On 15/10/2015 12:26, Sagi Grimberg wrote: When using work request based memory registration (fast_reg) we must reserve SQ entries for registration and invalidation in addition to send operations. Each IO consumes 3 SQ entries (registration, send, invalidation) so we need to allocate 3x larger

Re: [PATCH 7/7] IB/srp: Avoid that mapping failure triggers an infinite loop

Can you spare a few words on this change in the change log? Signed-off-by: Bart Van Assche <bart.vanass...@sandisk.com> Cc: Sagi Grimberg <sa...@mellanox.com> Cc: Sebastian Parschauer <sebastian.rie...@profitbricks.com> --- drivers/infiniband/ulp/srp/ib_srp.c | 2 ++

[PATCH v5 27/26] IB/hfi1: Remove fast registration from the code

The driver does not support it anyway, and the support should be added to a generic layer shared by both hfi1, qib and softroce drivers. Signed-off-by: Sagi Grimberg <sa...@mellanox.com> --- drivers/staging/rdma/hfi1/keys.c | 55 - drivers/staging/rdm

[PATCH 28/26] IB/ipath: Remove fast registration from the code

The driver does not support it anyway, and the support should be added to a generic layer shared by both hfi1, qib and softroce drivers. Signed-off-by: Sagi Grimberg <sa...@mellanox.com> --- drivers/staging/rdma/ipath/ipath_verbs.c |3 --- drivers/staging/rdma/ipath/ipath_verbs.h |

Re: [PATCH v5 00/26] New fast registration API

I can provide a patch for hfi, anything else needed? It breaks all of them in staging, not just hgi1. So, hfi1, amso1100, ipath, and ehca. hfi1: Does not support FRWR at all, there are just some copy-paste sections that supposedly handle it - so I'll drop any sign of it from the code.

Re: [PATCH] IB/mlx: Expose max_fmr to ib_query_device

Hi Yuval, The title prefix should be IB/mlx4: Expose max_fmr so it will be available to ULPs. max_fmr is num_mpts minus reserved. Signed-off-by: Yuval Shaia --- drivers/infiniband/hw/mlx4/main.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git

[PATCH v2 2/2] iser-target: Remove explicit mlx4 work-around

The driver now exposes sufficient limits so we can avoid having mlx4 specific work-around. Signed-off-by: Sagi Grimberg <sa...@mellanox.com> Reviewed-by: Steve Wise <sw...@opengridcomputing.com> --- drivers/infiniband/ulp/isert/ib_isert.c | 13 +++-- 1 files changed,

Re: [PATCH 02/25] IB/mthca, net/mlx4: remove counting semaphores

Hi Arnd, Since we want to make counting semaphores go away, Why do we want to make counting semaphores go away? completely? or just for binary use cases? I have a use case in iser target code where a counting semaphore is the best suited synchronizing mechanism. I have a single thread

Re: [PATCH 0/7] Fix an infinite loop in the SRP initiator

Submitting a SCSI request through the SG_IO mechanism with a scatterlist that is longer than what is supported by the SRP initiator triggers an infinite loop. This patch series fixes that behavior. The individual patches in this series are as follows: 0001-IB-srp-Fix-a-spelling-error.patch

[PATCH 2/2] iser-target: Remove explicit mlx4 work-around

The driver now exposes sufficient limits so we can avoid having mlx4 specific work-around. Signed-off-by: Sagi Grimberg <sa...@mellanox.com> --- drivers/infiniband/ulp/isert/ib_isert.c | 10 ++ 1 files changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/infiniband/ulp

[PATCH 0/2] Expose max_sge_rd correctly

This addresses a specific mlx4 issue where the max_sge_rd is actually smaller than max_sge (rdma reads with max_sge entries completes with error). The second patch removes the explicit work-around from the iser target code. This applies on top of Christoph's device attributes modification. Sagi

<    1   2   3   4   5   6   7   8   9   10   >