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

2015-11-11 Thread Sagi Grimberg
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 12/13] staging/rdma/hfi1: Read EFI variable for device description

2015-11-11 Thread Dan Carpenter
On Wed, Nov 11, 2015 at 02:33:32AM -0500, Jubin John wrote: > +static int read_efi_var(const char *name, unsigned long *size, > + void **return_data) > +{ > + int ret; > + > + /* set failure return values */ > + *size = 0; > + *return_data = NULL; > + > + /*

Re: [PATCH 4/8] staging/rdma/hfi1: remove unneeded goto done

2015-11-11 Thread Dan Carpenter
On Wed, Nov 11, 2015 at 12:43:05AM -0500, ira.we...@intel.com wrote: > From: Ira Weiny > > This goto done is followed by an if (ret) break in the outer switch clause. > It > is unnecessary. > > Signed-off-by: Dennis Dalessandro >

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

2015-11-11 Thread Christoph Hellwig
On Wed, Nov 11, 2015 at 11:09:03AM +0200, Sagi Grimberg wrote: > It does, but it doesn't look like something we'd want to check for each > IO... Both potential callers already have a access flags variable in object that's assigned to at setup time so I don't see a problem here. -- To unsubscribe

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

2015-11-11 Thread Sagi Grimberg
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

2015-11-11 Thread Sagi Grimberg
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 4/8] staging/rdma/hfi1: remove unneeded goto done

2015-11-11 Thread Dan Carpenter
On Wed, Nov 11, 2015 at 12:43:05AM -0500, ira.we...@intel.com wrote: > From: Ira Weiny > > This goto done is followed by an if (ret) break in the outer switch clause. > It > is unnecessary. > > Signed-off-by: Dennis Dalessandro >

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

2015-11-11 Thread Sagi Grimberg
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: [PATCH RFC 1/3] IB/core: Expose a device attribute for rdma_read access flags

2015-11-11 Thread Christoph Hellwig
On Wed, Nov 11, 2015 at 11:07:14AM +0200, Sagi Grimberg wrote: > > > 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

Re: [PATCH] staging/rdma/hfi1: Reduce number of parameters passed to send handlers

2015-11-11 Thread Dennis Dalessandro
On Wed, Nov 11, 2015 at 08:25:35AM +0200, Leon Romanovsky wrote: On Wed, Nov 11, 2015 at 12:34:37AM -0500, ira.we...@intel.com wrote: From: Dennis Dalessandro +int snoop_send_dma_handler(struct hfi1_qp *qp, struct hfi1_pkt_state *ps, +

RE: [PATCH 12/13] staging/rdma/hfi1: Read EFI variable for device description

2015-11-11 Thread Luick, Dean
> -Original Message- > From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma- > ow...@vger.kernel.org] On Behalf Of Dan Carpenter > Sent: Wednesday, November 11, 2015 2:45 AM > To: John, Jubin > Cc: gre...@linuxfoundation.org; de...@driverdev.osuosl.org; linux-

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

2015-11-11 Thread Jason Gunthorpe
On Wed, Nov 11, 2015 at 11:25:06AM +0200, Sagi Grimberg wrote: > For RDMA READs, a HCA will perform the memory scatters when on the RX > path, when receiving the read responses containing the data. This means > that the HCA needs to perform a lookup of the relevant scatter entries > upon each read

Re: srp state in current mainline

2015-11-11 Thread Sagi Grimberg
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 RFC 2/3] svcrdma: Use device rdma_read_access_flags

2015-11-11 Thread Jason Gunthorpe
On Wed, Nov 11, 2015 at 12:02:25AM -0800, Christoph Hellwig wrote: > > No need for the else, !rdma_cap_needs_rdma_read_mr means pd->local_dma_lkey > > is okay > > to use. > > What would happen if someone tried to use NFS on usnic without this? Honestly, I have no idea how usnic fits into the

Re: [PATCH 12/13] staging/rdma/hfi1: Read EFI variable for device description

2015-11-11 Thread gre...@linuxfoundation.org
On Wed, Nov 11, 2015 at 03:03:12PM +, Luick, Dean wrote: > > -Original Message- > > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > > Sent: Wednesday, November 11, 2015 8:39 AM > > To: Luick, Dean > > Cc: John, Jubin ;

Re: srp state in current mainline

2015-11-11 Thread Christoph Hellwig
On Wed, Nov 11, 2015 at 07:35:47AM -0800, Bart Van Assche wrote: > On 11/10/2015 09:15 AM, Christoph Hellwig wrote: > >This is a simply xfstests run using XFS on a remote LIO ramdisk. > > Hello Christoph, > > Which version of the kernel and LIO were installed at the target side ? I've tried a

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

2015-11-11 Thread Chuck Lever
Hi Sagi- > On Nov 11, 2015, at 4:28 AM, Sagi Grimberg wrote: > >> 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

Re: srp state in current mainline

2015-11-11 Thread Christoph Hellwig
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 /etc/modprobe.d/ib_srp.conf >

Re: [PATCH 4/8] staging/rdma/hfi1: remove unneeded goto done

2015-11-11 Thread ira.weiny
On Wed, Nov 11, 2015 at 12:01:08PM +0300, Dan Carpenter wrote: > On Wed, Nov 11, 2015 at 12:43:05AM -0500, ira.we...@intel.com wrote: > > From: Ira Weiny > > > > This goto done is followed by an if (ret) break in the outer switch clause. > > It > > is unnecessary. > > > >

RE: [PATCH 12/13] staging/rdma/hfi1: Read EFI variable for device description

2015-11-11 Thread Luick, Dean
> -Original Message- > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > Sent: Wednesday, November 11, 2015 8:39 AM > To: Luick, Dean > Cc: John, Jubin ; de...@driverdev.osuosl.org; > gre...@linuxfoundation.org; dledf...@redhat.com; linux-

Re: srp state in current mainline

2015-11-11 Thread Bart Van Assche
On 11/10/2015 09:15 AM, Christoph Hellwig wrote: This is a simply xfstests run using XFS on a remote LIO ramdisk. Hello Christoph, Which version of the kernel and LIO were installed at the target side ? Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the

Re: [PATCH] staging/rdma/hfi1: Reduce number of parameters passed to send handlers

2015-11-11 Thread Leon Romanovsky
On Wed, Nov 11, 2015 at 08:39:08AM -0500, Dennis Dalessandro wrote: > On Wed, Nov 11, 2015 at 08:25:35AM +0200, Leon Romanovsky wrote: > >On Wed, Nov 11, 2015 at 12:34:37AM -0500, ira.we...@intel.com wrote: > >>From: Dennis Dalessandro > >> > >>+int

Re: srp state in current mainline

2015-11-11 Thread Bart Van Assche
On 11/11/2015 07:46 AM, Christoph Hellwig wrote: On Wed, Nov 11, 2015 at 07:35:47AM -0800, Bart Van Assche wrote: On 11/10/2015 09:15 AM, Christoph Hellwig wrote: This is a simply xfstests run using XFS on a remote LIO ramdisk. Hello Christoph, Which version of the kernel and LIO were

Re: [PATCH 5/8] staging/rdma/hfi1: return early if setlink state was specified

2015-11-11 Thread Dan Carpenter
On Wed, Nov 11, 2015 at 12:43:06AM -0500, ira.we...@intel.com wrote: > From: Ira Weiny > > Set link state was not supported and so we can return early in the parameter > checks rather than falling through the switch clause. > > Signed-off-by: Dennis Dalessandro

Re: [PATCH v4 5/9] staging/rdma/hfi1: Add function stubs for TID caching

2015-11-11 Thread Dan Carpenter
On Wed, Nov 11, 2015 at 01:10:40AM -0500, ira.weiny wrote: > The original author and I have been going through the code to see what we can > do. We have identified a couple of other pieces which can be split. > > One question. Is it ok to have functionality which is added which is unused > in

Re: [PATCH 12/13] staging/rdma/hfi1: Read EFI variable for device description

2015-11-11 Thread Dan Carpenter
On Wed, Nov 11, 2015 at 02:03:02PM +, Luick, Dean wrote: > > > > -Original Message- > > From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma- > > ow...@vger.kernel.org] On Behalf Of Dan Carpenter > > Sent: Wednesday, November 11, 2015 2:45 AM > > To: John, Jubin

Re: [PATCH 4/8] staging/rdma/hfi1: remove unneeded goto done

2015-11-11 Thread Dennis Dalessandro
On Wed, Nov 11, 2015 at 12:03:36PM +0300, Dan Carpenter wrote: On Wed, Nov 11, 2015 at 12:43:05AM -0500, ira.we...@intel.com wrote: From: Ira Weiny This goto done is followed by an if (ret) break in the outer switch clause. It is unnecessary. Signed-off-by: Dennis

Re: [PATCH] staging/rdma/hfi1: Reduce number of parameters passed to send handlers

2015-11-11 Thread Or Gerlitz
On Wed, Nov 11, 2015 at 3:39 PM, Dennis Dalessandro wrote: > On Wed, Nov 11, 2015 at 08:25:35AM +0200, Leon Romanovsky wrote: >> On Wed, Nov 11, 2015 at 12:34:37AM -0500, ira.we...@intel.com wrote: >>> From: Dennis Dalessandro >>> +int

Re: [PATCH 5/9] staging/rdma/hfi1: Add function stubs for TID caching

2015-11-11 Thread Or Gerlitz
On Sat, Oct 31, 2015 at 12:41 AM, wrote: > From: Mitko Haralanov > > Add mmu notify helper functions and TID caching function stubs in preparation > for the TID caching implementation. > > TID caching makes use of the MMU notifier to allow the

Re: [PATCH] svcrdma: Do not send XDR roundup bytes for a write chunk

2015-11-11 Thread Chuck Lever
> On Nov 3, 2015, at 3:10 PM, Chuck Lever wrote: > > >> On Oct 16, 2015, at 9:30 AM, Chuck Lever wrote: >> >> Minor optimization: when dealing with write chunk XDR roundup, do >> not post a Write WR for the zero bytes in the pad. Simply update

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

2015-11-11 Thread Chuck Lever
> On Nov 11, 2015, at 11:29 AM, Sagi Grimberg wrote: > > > >> An alternate explanation is that the provider is not setting >> device->max_sge_rd properly. rdma_read_chunks_lcl() seems to >> be the only thing in my copy of the kernel tree that relies on >> that value.

Re: srp state in current mainline

2015-11-11 Thread Bart Van Assche
On 11/10/2015 09:15 AM, Christoph Hellwig wrote: scsi host3: ib_srp: failed receive status WR flushed (5) for iu 880313f4ca40 Can you also post the logs from the target system from around the time this message was logged on the initiator system ? Usually this message means that the

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

2015-11-11 Thread Christoph Hellwig
On Tue, Nov 10, 2015 at 11:25:46AM -0700, Jason Gunthorpe wrote: > I like this, my only comment is we should have a rdma_cap for this > behavior, rdma_cap_needs_rdma_read_mr(pd) or something? Yes, that's better than checking the protocol. > > + if (!(dev->device_cap_flags & > >

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

2015-11-11 Thread Christoph Hellwig
On Tue, Nov 10, 2015 at 11:36:27AM -0700, Jason Gunthorpe wrote: > > n = ib_map_mr_sg(desc->mr, state->sg, state->sg_nents, > > -dev->mr_page_size); > > +dev->mr_page_size, > > +/* > > + * XXX: add a bool write

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

2015-11-11 Thread Christoph Hellwig
On Tue, Nov 10, 2015 at 04:55:51PM -0700, Jason Gunthorpe wrote: > IMHO, the break point for when it makes sense to switch from a RDMA > READ chain to a MR is going to be a RDMA core tunable. The performance > curve won't have much to do with the ULP. core/device, a lot of it depends on when we'd

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

2015-11-11 Thread Christoph Hellwig
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, rdma_start_port(pd->device))) >

Re: [PATCH 10/13] staging/rdma/hfi1: adding per SDMA engine stats to hfistats

2015-11-11 Thread Dan Carpenter
On Wed, Nov 11, 2015 at 02:33:30AM -0500, Jubin John wrote: > @@ -8288,6 +8367,21 @@ static int init_cntrs(struct hfi1_devdata *dd) > dd->ndevcntrs++; > index++; > } > + } else if (dev_cntrs[i].flags &