diff mbox

[V2,4/5] RDMA/iser: support iWARP devices

Message ID 20150629213624.4188.94135.stgit@build.ogc.int (mailing list archive)
State Changes Requested
Headers show

Commit Message

Steve Wise June 29, 2015, 9:36 p.m. UTC
Limit the sg tablesize based on the device fast reg depth.

Use rdma_get_dma_mr() to allocate the DMA MR.

Use rdma_fast_reg_access_flags() to set the access_flags for fast
register work requests.

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c  |    7 +++++++
 drivers/infiniband/ulp/iser/iser_memory.c |    7 +++----
 drivers/infiniband/ulp/iser/iser_verbs.c  |    7 ++++---
 3 files changed, 14 insertions(+), 7 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Sagi Grimberg June 30, 2015, 9:26 a.m. UTC | #1
On 6/30/2015 12:36 AM, Steve Wise wrote:
> Limit the sg tablesize based on the device fast reg depth.
>
> Use rdma_get_dma_mr() to allocate the DMA MR.
>
> Use rdma_fast_reg_access_flags() to set the access_flags for fast
> register work requests.

Steve,

I wander if it would make more sense to get the iser/isert stuff in
without the mr stuff (I'd like the mr to go in a different direction).

Whatever you prefer.

Sagi.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Wise June 30, 2015, 2:33 p.m. UTC | #2
> -----Original Message-----
> From: Sagi Grimberg [mailto:sagig@dev.mellanox.co.il]
> Sent: Tuesday, June 30, 2015 4:26 AM
> To: Steve Wise; dledford@redhat.com
> Cc: roid@mellanox.com; sagig@mellanox.com; linux-rdma@vger.kernel.org; jgunthorpe@obsidianresearch.com; infinipath@intel.com;
> eli@mellanox.com; ogerlitz@mellanox.com; sean.hefty@intel.com
> Subject: Re: [PATCH V2 4/5] RDMA/iser: support iWARP devices
> 
> On 6/30/2015 12:36 AM, Steve Wise wrote:
> > Limit the sg tablesize based on the device fast reg depth.
> >
> > Use rdma_get_dma_mr() to allocate the DMA MR.
> >
> > Use rdma_fast_reg_access_flags() to set the access_flags for fast
> > register work requests.
> 
> Steve,
> 
> I wander if it would make more sense to get the iser/isert stuff in
> without the mr stuff (I'd like the mr to go in a different direction).
> 
> Whatever you prefer.
>

I prefer to decouple the iSER changes with this core work.  Jason/Sean... thoughts?   I could do the iSER w/o patch 3, and the follow up with a series that includes our final solution on transport independent memory registration and change all the TI kernel users (iser and nfsrdma) along with it.

Steve.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe June 30, 2015, 4:45 p.m. UTC | #3
On Tue, Jun 30, 2015 at 09:33:21AM -0500, Steve Wise wrote:

> I prefer to decouple the iSER changes with this core work.
> Jason/Sean... thoughts?  I could do the iSER w/o patch 3, and the
> follow up with a series that includes our final solution on
> transport independent memory registration and change all the TI
> kernel users (iser and nfsrdma) along with it.

There has a been a big push lately to drop the strange transport
specific stuff - if you are comitted to seeing the access flag clean
up series through then I don't see a problem with using whatever order
you like.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hefty, Sean June 30, 2015, 5:03 p.m. UTC | #4
> > I prefer to decouple the iSER changes with this core work.
> > Jason/Sean... thoughts?  I could do the iSER w/o patch 3, and the
> > follow up with a series that includes our final solution on
> > transport independent memory registration and change all the TI
> > kernel users (iser and nfsrdma) along with it.
> 
> There has a been a big push lately to drop the strange transport
> specific stuff - if you are comitted to seeing the access flag clean
> up series through then I don't see a problem with using whatever order
> you like.

I agree.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Wise June 30, 2015, 6:42 p.m. UTC | #5
> -----Original Message-----
> From: Hefty, Sean [mailto:sean.hefty@intel.com]
> Sent: Tuesday, June 30, 2015 12:04 PM
> To: Jason Gunthorpe; Steve Wise
> Cc: 'Sagi Grimberg'; dledford@redhat.com; roid@mellanox.com; sagig@mellanox.com; linux-rdma@vger.kernel.org; infinipath;
> eli@mellanox.com; ogerlitz@mellanox.com
> Subject: RE: [PATCH V2 4/5] RDMA/iser: support iWARP devices
> 
> > > I prefer to decouple the iSER changes with this core work.
> > > Jason/Sean... thoughts?  I could do the iSER w/o patch 3, and the
> > > follow up with a series that includes our final solution on
> > > transport independent memory registration and change all the TI
> > > kernel users (iser and nfsrdma) along with it.
> >
> > There has a been a big push lately to drop the strange transport
> > specific stuff - if you are comitted to seeing the access flag clean
> > up series through then I don't see a problem with using whatever order
> > you like.
> 
> I agree.

Ok, then I'll do this:

1) series with iser changes to enable iwarp including mlx/ipath/qib patches to set max_sge_rd
2) series on transport independent access flags / memory reg - updating the kernel ULPs too.

Thanks,

Steve.


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Ledford June 30, 2015, 8:20 p.m. UTC | #6
On 06/30/2015 02:42 PM, Steve Wise wrote:
> 
> 
>> -----Original Message-----
>> From: Hefty, Sean [mailto:sean.hefty@intel.com]
>> Sent: Tuesday, June 30, 2015 12:04 PM
>> To: Jason Gunthorpe; Steve Wise
>> Cc: 'Sagi Grimberg'; dledford@redhat.com; roid@mellanox.com; sagig@mellanox.com; linux-rdma@vger.kernel.org; infinipath;
>> eli@mellanox.com; ogerlitz@mellanox.com
>> Subject: RE: [PATCH V2 4/5] RDMA/iser: support iWARP devices
>>
>>>> I prefer to decouple the iSER changes with this core work.
>>>> Jason/Sean... thoughts?  I could do the iSER w/o patch 3, and the
>>>> follow up with a series that includes our final solution on
>>>> transport independent memory registration and change all the TI
>>>> kernel users (iser and nfsrdma) along with it.
>>>
>>> There has a been a big push lately to drop the strange transport
>>> specific stuff - if you are comitted to seeing the access flag clean
>>> up series through then I don't see a problem with using whatever order
>>> you like.
>>
>> I agree.
> 
> Ok, then I'll do this:
> 
> 1) series with iser changes to enable iwarp including mlx/ipath/qib patches to set max_sge_rd
> 2) series on transport independent access flags / memory reg - updating the kernel ULPs too.

I've quickly reviewed the series and you plan here seems fine.  I'm
bouncing this out of my queue and will wait for the next version.
Or Gerlitz July 1, 2015, 7:39 a.m. UTC | #7
On 6/30/2015 9:42 PM, Steve Wise wrote:
>>>> I prefer to decouple the iSER changes with this core work.
>>>> Jason/Sean... thoughts?  I could do the iSER w/o patch 3, and the
>>>> follow up with a series that includes our final solution on
>>>> transport independent memory registration and change all the TI
>>>> kernel users (iser and nfsrdma) along with it.
>>> There has a been a big push lately to drop the strange transport
>>> specific stuff - if you are comitted to seeing the access flag clean
>>> up series through then I don't see a problem with using whatever order
>>> you like.
>> I agree.
> Ok, then I'll do this:
>
> 1) series with iser changes to enable iwarp including mlx/ipath/qib patches to set max_sge_rd
> 2) series on transport independent access flags / memory reg - updating the kernel ULPs too.
not following... OTOH for iser to work over iwarp too we **only** (your 
#1) need to patch some IB/RoCE drivers to properly set their advertized 
max_sge_rd but no core changes but OTOH we **also** need  (your #2) an 
IB core patch/series with independent access flags for porting (say) 
NFSoRDMA to use the same code for multiple transports? explain...

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Wise July 1, 2015, 2:08 p.m. UTC | #8
> -----Original Message-----
> From: Or Gerlitz [mailto:ogerlitz@mellanox.com]
> Sent: Wednesday, July 01, 2015 2:40 AM
> To: Steve Wise
> Cc: 'Hefty, Sean'; 'Jason Gunthorpe'; 'Sagi Grimberg'; dledford@redhat.com; roid@mellanox.com; sagig@mellanox.com; linux-
> rdma@vger.kernel.org
> Subject: Re: [PATCH V2 4/5] RDMA/iser: support iWARP devices
> 
> On 6/30/2015 9:42 PM, Steve Wise wrote:
> >>>> I prefer to decouple the iSER changes with this core work.
> >>>> Jason/Sean... thoughts?  I could do the iSER w/o patch 3, and the
> >>>> follow up with a series that includes our final solution on
> >>>> transport independent memory registration and change all the TI
> >>>> kernel users (iser and nfsrdma) along with it.
> >>> There has a been a big push lately to drop the strange transport
> >>> specific stuff - if you are comitted to seeing the access flag clean
> >>> up series through then I don't see a problem with using whatever order
> >>> you like.
> >> I agree.
> > Ok, then I'll do this:
> >
> > 1) series with iser changes to enable iwarp including mlx/ipath/qib patches to set max_sge_rd
> > 2) series on transport independent access flags / memory reg - updating the kernel ULPs too.
> not following... OTOH for iser to work over iwarp too we **only** (your
> #1) need to patch some IB/RoCE drivers to properly set their advertized
> max_sge_rd but no core changes but OTOH we **also** need  (your #2) an
> IB core patch/series with independent access flags for porting (say)
> NFSoRDMA to use the same code for multiple transports? explain...
> 
>

First, I'll submit iSER patches that enable iWARP in the same manner that NFSRDMA does.  Namely, have iSER look at the transport
type and set the MR access flags accordingly.  That is a very simple set of changes.  I submitted this as the first series earlier
in the week.  The main comment was to fix the rdma drivers to set max_sge_rd and use that to adjust the read sge depth.  

Next, I'll submit the transport independent core changes to remove the ULPS from having to know about transport access_flag
differences.  Included in this series will be changes to iSER and NFSRDMA to make use of the new services.

Steve.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 6a594aa..ec692f7 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -640,6 +640,13 @@  iscsi_iser_session_create(struct iscsi_endpoint *ep,
 						   SHOST_DIX_GUARD_CRC);
 		}
 
+		/*
+		 * Limit the sg_tablesize based on the device max fastreg page
+		 * list length.
+		 */
+		shost->sg_tablesize = min_t(u32, shost->sg_tablesize,
+			ib_conn->device->dev_attr.max_fast_reg_page_list_len);
+
 		if (iscsi_host_add(shost,
 				   ib_conn->device->ib_device->dma_device)) {
 			mutex_unlock(&iser_conn->state_mutex);
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index f0cdc96..3a19483 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -757,10 +757,9 @@  static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 	fastreg_wr.wr.fast_reg.page_shift = SHIFT_4K;
 	fastreg_wr.wr.fast_reg.length = size;
 	fastreg_wr.wr.fast_reg.rkey = mr->rkey;
-	fastreg_wr.wr.fast_reg.access_flags = (IB_ACCESS_LOCAL_WRITE  |
-					       IB_ACCESS_REMOTE_WRITE |
-					       IB_ACCESS_REMOTE_READ);
-
+	fastreg_wr.wr.fast_reg.access_flags =
+		rdma_fast_reg_access_flags(device->pd, RDMA_MRR_WRITE_DEST |
+					   RDMA_MRR_READ_SOURCE, 0);
 	if (!wr)
 		wr = &fastreg_wr;
 	else
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 5c9f565..4da368c 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -80,6 +80,7 @@  static int iser_create_device_ib_res(struct iser_device *device)
 {
 	struct ib_device_attr *dev_attr = &device->dev_attr;
 	int ret, i, max_cqe;
+	int mr_roles;
 
 	ret = ib_query_device(device->ib_device, dev_attr);
 	if (ret) {
@@ -149,9 +150,9 @@  static int iser_create_device_ib_res(struct iser_device *device)
 			     (unsigned long)comp);
 	}
 
-	device->mr = ib_get_dma_mr(device->pd, IB_ACCESS_LOCAL_WRITE |
-				   IB_ACCESS_REMOTE_WRITE |
-				   IB_ACCESS_REMOTE_READ);
+	mr_roles = RDMA_MRR_SEND | RDMA_MRR_RECV | RDMA_MRR_WRITE_DEST |
+		   RDMA_MRR_READ_SOURCE;
+	device->mr = rdma_get_dma_mr(device->pd, mr_roles, 0);
 	if (IS_ERR(device->mr))
 		goto dma_mr_err;