diff mbox

RDMA/cxgb3: fail get_dma_mr if the memory footprint can exceed 32b

Message ID 20150722191417.9306.17387.stgit@build2.ogc.int (mailing list archive)
State Accepted
Headers show

Commit Message

Steve Wise July 22, 2015, 7:14 p.m. UTC
T3 HW only supports MRs of length < 4GB.  If the system can have more
than that we need to fail dma mr allocation so we con't create a MR that
cannot span the entire possible memory space.

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
---

 drivers/infiniband/hw/cxgb3/iwch_provider.c |    4 ++++
 1 files changed, 4 insertions(+), 0 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

Doug Ledford July 23, 2015, 9:32 p.m. UTC | #1
On 07/22/2015 03:14 PM, Steve Wise wrote:
> T3 HW only supports MRs of length < 4GB.  If the system can have more
> than that we need to fail dma mr allocation so we con't create a MR that
> cannot span the entire possible memory space.
> 
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> ---
> 
>  drivers/infiniband/hw/cxgb3/iwch_provider.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> index b1b7323..bbbe018 100644
> --- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
> +++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> @@ -736,6 +736,10 @@ static struct ib_mr *iwch_get_dma_mr(struct ib_pd *pd, int acc)
>  	/*
>  	 * T3 only supports 32 bits of size.
>  	 */
> +	if (sizeof(phys_addr_t) > 4) {
> +		pr_warn_once(MOD "Cannot support dma_mrs on this platform.\n");
> +		return ERR_PTR(-ENOTSUPP);
> +	}
>  	bl.size = 0xffffffff;
>  	bl.addr = 0;
>  	kva = 0;

Should this be a static check of the pointer size versus installed
memory?  Would it be possible to have this work for machines with less
than 4GB of physical memory even if they have 64bit pointers, or are you
concerned that hotplug memory could take us over the limit after
registration and cause problems?
Steve Wise July 23, 2015, 10:47 p.m. UTC | #2
> -----Original Message-----
> From: Doug Ledford [mailto:dledford@redhat.com]
> Sent: Thursday, July 23, 2015 4:33 PM
> To: Steve Wise
> Cc: linux-rdma@vger.kernel.org
> Subject: Re: [PATCH] RDMA/cxgb3: fail get_dma_mr if the memory footprint can exceed 32b
> 
> On 07/22/2015 03:14 PM, Steve Wise wrote:
> > T3 HW only supports MRs of length < 4GB.  If the system can have more
> > than that we need to fail dma mr allocation so we con't create a MR that
> > cannot span the entire possible memory space.
> >
> > Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> > ---
> >
> >  drivers/infiniband/hw/cxgb3/iwch_provider.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> > index b1b7323..bbbe018 100644
> > --- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
> > +++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> > @@ -736,6 +736,10 @@ static struct ib_mr *iwch_get_dma_mr(struct ib_pd *pd, int acc)
> >  	/*
> >  	 * T3 only supports 32 bits of size.
> >  	 */
> > +	if (sizeof(phys_addr_t) > 4) {
> > +		pr_warn_once(MOD "Cannot support dma_mrs on this platform.\n");
> > +		return ERR_PTR(-ENOTSUPP);
> > +	}
> >  	bl.size = 0xffffffff;
> >  	bl.addr = 0;
> >  	kva = 0;
> 
> Should this be a static check of the pointer size versus installed
> memory?  Would it be possible to have this work for machines with less
> than 4GB of physical memory even if they have 64bit pointers, or are you
> concerned that hotplug memory could take us over the limit after
> registration and cause problems?

NFSRDMA doesn't need dma-mrs for T3 since it has FRMR + local dma lkey support.  And since the deficiency really can cause problems on 64b systems if the memory grows > 4GB after dma-mr allocation, I decided to just not allow them for potential large memory systems.

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 July 24, 2015, 2:44 p.m. UTC | #3
On 07/23/2015 06:47 PM, Steve Wise wrote:
>> -----Original Message----- From: Doug Ledford
>> Should this be a static check of the pointer size versus installed 
>> memory?  Would it be possible to have this work for machines with
>> less than 4GB of physical memory even if they have 64bit pointers,
>> or are you concerned that hotplug memory could take us over the
>> limit after registration and cause problems?
> 
> NFSRDMA doesn't need dma-mrs for T3 since it has FRMR + local dma
> lkey support.  And since the deficiency really can cause problems on
> 64b systems if the memory grows > 4GB after dma-mr allocation, I
> decided to just not allow them for potential large memory systems.

Ok.  I've pulled this for 4.2-rc then.
Steve Wise July 24, 2015, 2:50 p.m. UTC | #4
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of Doug Ledford
> Sent: Friday, July 24, 2015 9:45 AM
> To: Steve Wise
> Cc: linux-rdma@vger.kernel.org
> Subject: Re: [PATCH] RDMA/cxgb3: fail get_dma_mr if the memory footprint can exceed 32b
> 
> On 07/23/2015 06:47 PM, Steve Wise wrote:
> >> -----Original Message----- From: Doug Ledford
> >> Should this be a static check of the pointer size versus installed
> >> memory?  Would it be possible to have this work for machines with
> >> less than 4GB of physical memory even if they have 64bit pointers,
> >> or are you concerned that hotplug memory could take us over the
> >> limit after registration and cause problems?
> >
> > NFSRDMA doesn't need dma-mrs for T3 since it has FRMR + local dma
> > lkey support.  And since the deficiency really can cause problems on
> > 64b systems if the memory grows > 4GB after dma-mr allocation, I
> > decided to just not allow them for potential large memory systems.
> 
> Ok.  I've pulled this for 4.2-rc then.
> 

The problem has been there since day one, so it doesn't represent a regression.  It is your call, but I think 4.3 is fine.

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 July 24, 2015, 2:55 p.m. UTC | #5
On 07/24/2015 10:50 AM, Steve Wise wrote:
> 
> 
>> -----Original Message-----
>> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of Doug Ledford
>> Sent: Friday, July 24, 2015 9:45 AM
>> To: Steve Wise
>> Cc: linux-rdma@vger.kernel.org
>> Subject: Re: [PATCH] RDMA/cxgb3: fail get_dma_mr if the memory footprint can exceed 32b
>>
>> On 07/23/2015 06:47 PM, Steve Wise wrote:
>>>> -----Original Message----- From: Doug Ledford
>>>> Should this be a static check of the pointer size versus installed
>>>> memory?  Would it be possible to have this work for machines with
>>>> less than 4GB of physical memory even if they have 64bit pointers,
>>>> or are you concerned that hotplug memory could take us over the
>>>> limit after registration and cause problems?
>>>
>>> NFSRDMA doesn't need dma-mrs for T3 since it has FRMR + local dma
>>> lkey support.  And since the deficiency really can cause problems on
>>> 64b systems if the memory grows > 4GB after dma-mr allocation, I
>>> decided to just not allow them for potential large memory systems.
>>
>> Ok.  I've pulled this for 4.2-rc then.
>>
> 
> The problem has been there since day one, so it doesn't represent a regression.  It is your call, but I think 4.3 is fine.

It's a long standing problem, but it's a potential memory wrap issue, so
might as well fix it (especially given the simple nature of the fix).
I've got a few other things heading in for 4.2-rc, so no biggie.
diff mbox

Patch

diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
index b1b7323..bbbe018 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
@@ -736,6 +736,10 @@  static struct ib_mr *iwch_get_dma_mr(struct ib_pd *pd, int acc)
 	/*
 	 * T3 only supports 32 bits of size.
 	 */
+	if (sizeof(phys_addr_t) > 4) {
+		pr_warn_once(MOD "Cannot support dma_mrs on this platform.\n");
+		return ERR_PTR(-ENOTSUPP);
+	}
 	bl.size = 0xffffffff;
 	bl.addr = 0;
 	kva = 0;