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] IB/ipoib: Change sendq threshold size

On Tue, Nov 17, 2015 at 10:11:17AM +0200, Or Gerlitz wrote: > On 11/16/2015 8:54 PM, Yuval Shaia wrote: > >-test_bit(IPOIB_FLAG_ADMIN_UP, >flags)) > >+likely(test_bit(IPOIB_FLAG_ADMIN_UP, >flags))) > > this hunk has nothing to do with the proposed change, remove it from here. np >

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

On Tue, Nov 17, 2015 at 12:26 PM, Sagi Grimberg wrote: > >> 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

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

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

On 11/13/2015 11:02 PM, Christoph Hellwig wrote: On Fri, Nov 13, 2015 at 11:19:24AM -0800, Bart Van Assche wrote: On 11/13/2015 05:46 AM, Christoph Hellwig wrote: The new name is irq_poll as iopoll is already taken. Better suggestions welcome. Would it be possible to provide more background

Re: [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues

On Mon, Nov 16, 2015 at 05:32:34PM -0500, ira.we...@intel.com wrote: > From: Jubin John > > Correct the checks on diag.c with the latest checkpatch > > Reviewed-by: Dennis Dalessandro > Reviewed-by: Mike Marciniszyn

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

On 11/17/2015 09:16 AM, Bart Van Assche wrote: On 11/13/2015 11:02 PM, Christoph Hellwig wrote: On Fri, Nov 13, 2015 at 11:19:24AM -0800, Bart Van Assche wrote: On 11/13/2015 05:46 AM, Christoph Hellwig wrote: The new name is irq_poll as iopoll is already taken. Better suggestions welcome.

Re: [PATCH 5/9] srpt: use the new CQ API

On 11/13/2015 05:46 AM, Christoph Hellwig wrote: > [ ... ] The previous patch and this patch look like great work to me. However, this patch not only reworks the SRP target driver but also prevents users to move the SRP completion thread to another CPU core than the CPU core that processes

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

On 11/13/2015 05:46 AM, Christoph Hellwig wrote: + * context and does not ask from completion interrupts from the HCA. Should this perhaps be changed into "for" ? + */ +void ib_process_cq_direct(struct ib_cq *cq) +{ + WARN_ON_ONCE(cq->poll_ctx !=

Re: [PATCH 5/9] srpt: use the new CQ API

On 11/13/2015 05:46 AM, Christoph Hellwig wrote: [ ... ] This patch contains two logical changes: - Conversion to the new CQ API. - Removal of the ib_srpt_compl thread. Had it been considered to implement these changes as two separate patches ? Thanks, Bart. -- To unsubscribe from this

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

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 */ +

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 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 involved

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 =

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 08/10] iser-target: Support the remote invalidation exception

On 11/17/2015 11:50 AM, Sagi Grimberg wrote: same comment as the one I posted for the initiator patch, check for IB_DEVICE_MEM_MGT_EXTENSIONS SEND_WITH_INV doesn't have anything to do with IB_DEVICE_MEM_MGT_EXTENSIONS does it? What is the relations between the device capability to do FRWR and

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

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 = 6; memset(_hdr, 0,

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

> + 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 (device->ib_device->alloc_fmr && device->ib_device->dealloc_fmr && Should

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_cm private

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

On Tue, Nov 17, 2015 at 12:04:03PM +0200, Or Gerlitz wrote: > Also, do we refuse to queuecommand a bidi? where? Or, can you please do the basic research first? Thanks! (Hint: check how few drivers support bidi commands, and how it's enabled) -- To unsubscribe from this list: send the line

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 00/10] iSER support for remote invalidate

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_cm private data. On which public git tree/branch

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

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 IB_WR_SEND_WITH_INV with the correct rkey for the

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 registration is valid

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 bidirectional

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

On 11/17/2015 11:41 AM, Sagi Grimberg wrote: what's the role of this hunk? why it belongs here? you are testing device->mr but this is something global and has nothing to do specially with specific IOs for which this patch aims to act It's because device->mr might not be allocated at all

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

On 11/16/2015 6:37 PM, Sagi Grimberg wrote: With remote invalidate we won't local invalidate but we still want to increment the rkey. To ease with review, as this is low level pre-patch, please push it to downstream area of the series, e.g before the patch that introduces include/scsi/iser.h

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

Looks good, Reviewed-by: Christoph Hellwig -- 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 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 00/10] iSER support for remote invalidate

On Mon, Nov 16, 2015 at 06:37:31PM +0200, 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_cm > private data. Oh, okay

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

On Tue, Nov 17, 2015 at 11:58:26AM +0200, Sagi Grimberg wrote: > I don't know... Shouldn't it? > > I just copied include/scsi/srp.h and changed it... Not a major issue, it just seemed a little strange to me. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of

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

On 11/17/2015 11:48 AM, Sagi Grimberg wrote: 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 involved since struct iser_cm_hdr doesn't exist for LIO

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

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 would be nice. Otherwise looks good: Reviewed-by:

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

On 11/17/2015 11:35 AM, Sagi Grimberg wrote: 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. Why?

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] IB/ipoib: Change sendq threshold size

On 11/16/2015 8:54 PM, Yuval Shaia wrote: - test_bit(IPOIB_FLAG_ADMIN_UP, >flags)) + likely(test_bit(IPOIB_FLAG_ADMIN_UP, >flags))) this hunk has nothing to do with the proposed change, remove it from here. upstream patches should have one logical change, -- To unsubscribe

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 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 involved

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

On 11/17/2015 11:53 AM, Sagi Grimberg wrote: [...] iSER lacks support for bidirectional commands for as long as I'm involved with it 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? -- To

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. If

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

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

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

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 05/10] iser: Have initiator and target to share protocol structures and definitions

On Mon, Nov 16, 2015 at 06:37:36PM +0200, Sagi Grimberg wrote: > 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. Nice! But should a header that

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->sge.lkey =

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

Re: [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues

On Tue, Nov 17, 2015 at 10:17:26PM +0530, Sudip Mukherjee wrote: > On Mon, Nov 16, 2015 at 05:32:34PM -0500, ira.we...@intel.com wrote: > > From: Jubin John > > > > Correct the checks on diag.c with the latest checkpatch > > > > Reviewed-by: Dennis Dalessandro

[PATCH libmlx5 v2 5/5] libmlx5: Work with CQE version 1

From: Haggai Abramovsky >From now on, if the kernel supports CQE version 1, the library will choose to work with it. Signed-off-by: Haggai Abramovsky --- src/mlx5.c | 3 +++ src/verbs.c | 9 +++-- 2 files changed, 6 insertions(+), 6 deletions(-)

[PATCH libmlx5 v2 3/5] libmlx5: Added new poll_cq according to the new CQE format

From: Haggai Abramovsky When working with CQE version 1, the CQE format is different, therefore we need a new poll_cq function that is compatible with the new CQE format. When allocating the user-context, we decide on the CQE version, and according to this, we choose

[PATCH libmlx5 v2 4/5] libmlx5: Added QP and XSRQ create/destroy flow with user index

From: Haggai Abramovsky When wokring with CQE version 1, the library allocates a user-index for each new QP/XSRQ, and this user-index is passed to the kernel. Also in the destruction of a QP/XSRQ, the library needs to free the user-index, so it can be reused. In this

[PATCH libmlx5 v2 0/5] Support CQE versions

From: Haggai Abramovsky This series adds support for CQE (cookie) versions in libmlx5 Non-default cookie versions are a must for RAW PACKET Eth QPs. The code is capable to work over kernels which don't have this support yet for the sake of creating other QP types. Haggai

[PATCH libmlx5 v2 2/5] libmlx5: Add QPs and XSRQs resource tracking

From: Haggai Abramovsky Add new database that stores all the QPs and XSRQs context. Insertions and deletions to the database are done using the object's user-index. This database will allow us to retrieve the objects; QPs and XSRQs; by their user-index in the poll_one.

[PATCH libmlx5 v2 1/5] libmlx5: Add infrastructure for resource identification

From: Haggai Abramovsky Add new struct member, mlx5_resource, for each tracked object, mlx5_srq and mlx5_qp, to allow retrieving the object in the poll_cq once we need it. Signed-off-by: Haggai Abramovsky --- src/mlx5.h | 18 +- 1

Re: [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues

On Tue, Nov 17, 2015 at 03:30:24PM -0500, ira.weiny wrote: > On Tue, Nov 17, 2015 at 10:17:26PM +0530, Sudip Mukherjee wrote: > > On Mon, Nov 16, 2015 at 05:32:34PM -0500, ira.we...@intel.com wrote: > > > From: Jubin John > > > > > > Correct the checks on diag.c with the