[PATCH v1 1/3] RDMA/rxe: Increase value of RXE_MAX_UCONTEXT

[PATCH v1 1/3] RDMA/rxe: Increase value of RXE_MAX_UCONTEXT

[Rao Shoaib]

From: Rao Shoaib <rao.shoaib@oracle.com>

In our internal testing we have found that the
current limit is too small, this patch bumps it
up to a higher value required for our tests, which
are indicative of our customer usage.

Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
---
 drivers/infiniband/sw/rxe/rxe_param.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
index 25ab50d9b7c2..2fd5af44258c 100644
--- a/drivers/infiniband/sw/rxe/rxe_param.h
+++ b/drivers/infiniband/sw/rxe/rxe_param.h
@@ -76,7 +76,7 @@ enum rxe_device_param {
 	RXE_MAX_PKEYS			= 1,
 	RXE_LOCAL_CA_ACK_DELAY		= 15,
 
-	RXE_MAX_UCONTEXT		= 512,
+	RXE_MAX_UCONTEXT		= 17408,
 
 	RXE_NUM_PORT			= 1,
 
-- 
2.31.1

[PATCH v1 2/3] RDMA/rxe: Increase value of RXE_MAX_SRQ

[Rao Shoaib]

From: Rao Shoaib <rao.shoaib@oracle.com>

In our internal testing we have found that the
current limit is too small, this patch bumps it
up to a higher value required for our tests, which
are indicative of our customer usage.

Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
---
 drivers/infiniband/sw/rxe/rxe_param.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
index 2fd5af44258c..3b9b1ff4234f 100644
--- a/drivers/infiniband/sw/rxe/rxe_param.h
+++ b/drivers/infiniband/sw/rxe/rxe_param.h
@@ -67,7 +67,7 @@ enum rxe_device_param {
 	RXE_MAX_MCAST_QP_ATTACH		= 56,
 	RXE_MAX_TOT_MCAST_QP_ATTACH	= 0x70000,
 	RXE_MAX_AH			= 100,
-	RXE_MAX_SRQ			= 960,
+	RXE_MAX_SRQ			= 17408,
 	RXE_MAX_SRQ_WR			= 0x4000,
 	RXE_MIN_SRQ_WR			= 1,
 	RXE_MAX_SRQ_SGE			= 27,
-- 
2.31.1

Re: [PATCH v1 2/3] RDMA/rxe: Increase value of RXE_MAX_SRQ

[Jason Gunthorpe]

On Thu, Jun 17, 2021 at 11:25:10AM -0700, Rao Shoaib wrote:
> From: Rao Shoaib <rao.shoaib@oracle.com>
> 
> In our internal testing we have found that the
> current limit is too small, this patch bumps it
> up to a higher value required for our tests, which
> are indicative of our customer usage.
> 
> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_param.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Well, this doesn't apply anymore so please resend the series. I'd
prefer you try to get rid of any hard limits that are not exposed in
uverbs someplace - like ucontext.

Jason

Re: [PATCH v1 2/3] RDMA/rxe: Increase value of RXE_MAX_SRQ

[Shoaib Rao]

On 6/22/21 11:35 AM, Jason Gunthorpe wrote:
> On Thu, Jun 17, 2021 at 11:25:10AM -0700, Rao Shoaib wrote:
>> From: Rao Shoaib <rao.shoaib@oracle.com>
>>
>> In our internal testing we have found that the
>> current limit is too small, this patch bumps it
>> up to a higher value required for our tests, which
>> are indicative of our customer usage.
>>
>> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
>> ---
>>   drivers/infiniband/sw/rxe/rxe_param.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> Well, this doesn't apply anymore so please resend the series. I'd
> prefer you try to get rid of any hard limits that are not exposed in
> uverbs someplace - like ucontext.
>
> Jason

Jason,

Most of the limits are exposed via uverbs. For example, following limits 
are most likely to be hit by a user application.

RXE_MAX_QP
RXE_MAX_QP_WR
RXE_MAX_CQ
RXE_MAX_MR
RXE_MAX_PD
RXE_MAX_AH
RXE_MAX_FMR
RXE_MAX_SRQ
RXE_MAX_SRQ_WR
RXE_MAX_PKEYS
RXE_MAX_UCONTEXT

However, they are reported in ib_device_attr/ib_uverbs_query_device and 
these are the limits that I am interested in. So unless I misunderstand 
you the current patch still applies? Kindly elaborate.

Regards,

Shoaib

[PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Rao Shoaib]

From: Rao Shoaib <rao.shoaib@oracle.com>

In our internal testing we have found that the
current limit is too small, this patch bumps it
up to a higher value required for our tests, which
are indicative of our customer usage.

Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
---
 drivers/infiniband/sw/rxe/rxe_param.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
index 3b9b1ff4234f..d375f2cff484 100644
--- a/drivers/infiniband/sw/rxe/rxe_param.h
+++ b/drivers/infiniband/sw/rxe/rxe_param.h
@@ -66,7 +66,7 @@ enum rxe_device_param {
 	RXE_MAX_MCAST_GRP		= 8192,
 	RXE_MAX_MCAST_QP_ATTACH		= 56,
 	RXE_MAX_TOT_MCAST_QP_ATTACH	= 0x70000,
-	RXE_MAX_AH			= 100,
+	RXE_MAX_AH			= 64000,
 	RXE_MAX_SRQ			= 17408,
 	RXE_MAX_SRQ_WR			= 0x4000,
 	RXE_MIN_SRQ_WR			= 1,
-- 
2.31.1

Re: [PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Bob Pearson]

On 6/17/21 1:25 PM, Rao Shoaib wrote:
> From: Rao Shoaib <rao.shoaib@oracle.com>
> 
> In our internal testing we have found that the
> current limit is too small, this patch bumps it
> up to a higher value required for our tests, which
> are indicative of our customer usage.
> 
> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_param.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
> index 3b9b1ff4234f..d375f2cff484 100644
> --- a/drivers/infiniband/sw/rxe/rxe_param.h
> +++ b/drivers/infiniband/sw/rxe/rxe_param.h
> @@ -66,7 +66,7 @@ enum rxe_device_param {
>  	RXE_MAX_MCAST_GRP		= 8192,
>  	RXE_MAX_MCAST_QP_ATTACH		= 56,
>  	RXE_MAX_TOT_MCAST_QP_ATTACH	= 0x70000,
> -	RXE_MAX_AH			= 100,
> +	RXE_MAX_AH			= 64000,
>  	RXE_MAX_SRQ			= 17408,
>  	RXE_MAX_SRQ_WR			= 0x4000,
>  	RXE_MIN_SRQ_WR			= 1,
> 

Interesting. There is no real reason to pick most of these values since it's just memory and does not reflect underlying hardware resources. It is advantageous to also be able to set them smaller to verify whether test cases correctly limit resources. It seems that there should be a way (module parameter or other) to adjust these values without having to recompile the driver. Thoughts?

Regards,

Bob Pearson

Re: [PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Shoaib Rao]

On 6/17/21 1:45 PM, Bob Pearson wrote:
> On 6/17/21 1:25 PM, Rao Shoaib wrote:
>> From: Rao Shoaib <rao.shoaib@oracle.com>
>>
>> In our internal testing we have found that the
>> current limit is too small, this patch bumps it
>> up to a higher value required for our tests, which
>> are indicative of our customer usage.
>>
>> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
>> ---
>>   drivers/infiniband/sw/rxe/rxe_param.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
>> index 3b9b1ff4234f..d375f2cff484 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_param.h
>> +++ b/drivers/infiniband/sw/rxe/rxe_param.h
>> @@ -66,7 +66,7 @@ enum rxe_device_param {
>>   	RXE_MAX_MCAST_GRP		= 8192,
>>   	RXE_MAX_MCAST_QP_ATTACH		= 56,
>>   	RXE_MAX_TOT_MCAST_QP_ATTACH	= 0x70000,
>> -	RXE_MAX_AH			= 100,
>> +	RXE_MAX_AH			= 64000,
>>   	RXE_MAX_SRQ			= 17408,
>>   	RXE_MAX_SRQ_WR			= 0x4000,
>>   	RXE_MIN_SRQ_WR			= 1,
>>
> Interesting. There is no real reason to pick most of these values since it's just memory and does not reflect underlying hardware resources. It is advantageous to also be able to set them smaller to verify whether test cases correctly limit resources. It seems that there should be a way (module parameter or other) to adjust these values without having to recompile the driver. Thoughts?

I agree with you 100% but it seems like the original design did not 
intent to make them configurable at run time. I see that recently some 
other values were bumped up by others so I went with hard coded values. 
In the inhouse kernel version we used to decide on the values had them 
configurable because we did not know what values we wanted, and 
recompiling was time consuming.

While some tests may require small values we want to test with lots of 
users/connections.

If there is consensus I can submit a different patch to make these 
values configurable. In the meantime I hope we can bump up these values.

Kindly let me know.

Shoaib

>
> Regards,
>
> Bob Pearson

Re: [PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Bob Pearson]

On 6/17/21 5:57 PM, Shoaib Rao wrote:
> 
> On 6/17/21 1:45 PM, Bob Pearson wrote:
>> On 6/17/21 1:25 PM, Rao Shoaib wrote:
>>> From: Rao Shoaib <rao.shoaib@oracle.com>
>>>
>>> In our internal testing we have found that the
>>> current limit is too small, this patch bumps it
>>> up to a higher value required for our tests, which
>>> are indicative of our customer usage.
>>>
>>> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
>>> ---
>>>   drivers/infiniband/sw/rxe/rxe_param.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
>>> index 3b9b1ff4234f..d375f2cff484 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_param.h
>>> +++ b/drivers/infiniband/sw/rxe/rxe_param.h
>>> @@ -66,7 +66,7 @@ enum rxe_device_param {
>>>       RXE_MAX_MCAST_GRP        = 8192,
>>>       RXE_MAX_MCAST_QP_ATTACH        = 56,
>>>       RXE_MAX_TOT_MCAST_QP_ATTACH    = 0x70000,
>>> -    RXE_MAX_AH            = 100,
>>> +    RXE_MAX_AH            = 64000,
>>>       RXE_MAX_SRQ            = 17408,
>>>       RXE_MAX_SRQ_WR            = 0x4000,
>>>       RXE_MIN_SRQ_WR            = 1,
>>>
>> Interesting. There is no real reason to pick most of these values since it's just memory and does not reflect underlying hardware resources. It is advantageous to also be able to set them smaller to verify whether test cases correctly limit resources. It seems that there should be a way (module parameter or other) to adjust these values without having to recompile the driver. Thoughts?
> 
> I agree with you 100% but it seems like the original design did not intent to make them configurable at run time. I see that recently some other values were bumped up by others so I went with hard coded values. In the inhouse kernel version we used to decide on the values had them configurable because we did not know what values we wanted, and recompiling was time consuming.
> 
> While some tests may require small values we want to test with lots of users/connections.
> 
> If there is consensus I can submit a different patch to make these values configurable. In the meantime I hope we can bump up these values.
> 
> Kindly let me know.
> 
> Shoaib
> 
>>
>> Regards,
>>
>> Bob Pearson

It isn't my call. But I am in favor of tunable parameters. -- Bob Pearson

Re: [PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Jason Gunthorpe]

On Thu, Jun 17, 2021 at 10:56:58PM -0500, Bob Pearson wrote:
 
> It isn't my call. But I am in favor of tunable parameters. -- Bob Pearson

Can we just delete the concept completely?

Jason

Re: [PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Bob Pearson]

On 6/18/21 11:33 AM, Jason Gunthorpe wrote:
> On Thu, Jun 17, 2021 at 10:56:58PM -0500, Bob Pearson wrote:
>  
>> It isn't my call. But I am in favor of tunable parameters. -- Bob Pearson
> 
> Can we just delete the concept completely?
> 
> Jason
> 
Not sure where you are headed here. Do you mean just lift the limits all together?
That would not be in the spirit of the IBA even if that is not very meaningful.
One of the useful things rxe does is to provide a cheap 'reference' implementation of the spec.
If OTOH you want to put it in production then the limits are not very useful.

Bob

Re: [PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Jason Gunthorpe]

On Fri, Jun 18, 2021 at 01:58:48PM -0500, Bob Pearson wrote:
> On 6/18/21 11:33 AM, Jason Gunthorpe wrote:
> > On Thu, Jun 17, 2021 at 10:56:58PM -0500, Bob Pearson wrote:
> >  
> >> It isn't my call. But I am in favor of tunable parameters. -- Bob Pearson
> > 
> > Can we just delete the concept completely?
> > 
> > Jason
> > 
> Not sure where you are headed here. Do you mean just lift the limits
> all together?

Yes.. The spec doesn't have like a UCONTEXT limit for instance, and
real HW like mlx5 has huge reported limits anyhow. 

Jason

Re: [PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Shoaib Rao]

On 6/18/21 4:25 PM, Jason Gunthorpe wrote:
> On Fri, Jun 18, 2021 at 01:58:48PM -0500, Bob Pearson wrote:
>> On 6/18/21 11:33 AM, Jason Gunthorpe wrote:
>>> On Thu, Jun 17, 2021 at 10:56:58PM -0500, Bob Pearson wrote:
>>>   
>>>> It isn't my call. But I am in favor of tunable parameters. -- Bob Pearson
>>> Can we just delete the concept completely?
>>>
>>> Jason
>>>
>> Not sure where you are headed here. Do you mean just lift the limits
>> all together?
> Yes.. The spec doesn't have like a UCONTEXT limit for instance, and
> real HW like mlx5 has huge reported limits anyhow.
>
> Jason

So do you want me to submit a patch that removes all limits or just 
these limits or something else?

Shoaib

Re: [PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Shoaib Rao]

On 6/18/21 4:25 PM, Jason Gunthorpe wrote:
> On Fri, Jun 18, 2021 at 01:58:48PM -0500, Bob Pearson wrote:
>> On 6/18/21 11:33 AM, Jason Gunthorpe wrote:
>>> On Thu, Jun 17, 2021 at 10:56:58PM -0500, Bob Pearson wrote:
>>>   
>>>> It isn't my call. But I am in favor of tunable parameters. -- Bob Pearson
>>> Can we just delete the concept completely?
>>>
>>> Jason
>>>
>> Not sure where you are headed here. Do you mean just lift the limits
>> all together?
> Yes.. The spec doesn't have like a UCONTEXT limit for instance, and
> real HW like mlx5 has huge reported limits anyhow.

These limits are reported via uverbs, so what do we report without 
current applications. Creating pool also requires limits but I guess we 
can use something like -1 to indicate there is no limit. I would have to 
look at all the values to see if we can implement this.

Shoaib


>
> Jason

Re: [PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Bob Pearson]

On 6/24/21 4:21 PM, Shoaib Rao wrote:
> 
> On 6/18/21 4:25 PM, Jason Gunthorpe wrote:
>> On Fri, Jun 18, 2021 at 01:58:48PM -0500, Bob Pearson wrote:
>>> On 6/18/21 11:33 AM, Jason Gunthorpe wrote:
>>>> On Thu, Jun 17, 2021 at 10:56:58PM -0500, Bob Pearson wrote:
>>>>  
>>>>> It isn't my call. But I am in favor of tunable parameters. -- Bob Pearson
>>>> Can we just delete the concept completely?
>>>>
>>>> Jason
>>>>
>>> Not sure where you are headed here. Do you mean just lift the limits
>>> all together?
>> Yes.. The spec doesn't have like a UCONTEXT limit for instance, and
>> real HW like mlx5 has huge reported limits anyhow.
> 
> These limits are reported via uverbs, so what do we report without current applications. Creating pool also requires limits but I guess we can use something like -1 to indicate there is no limit. I would have to look at all the values to see if we can implement this.
> 
> Shoaib
> 
> 
>>
>> Jason
The object create in pools (rxe_alloc_locked) just calls kzalloc for objects allocated by rxe and checks the limits. For objects allocated by rdma-core (__rxe_add_to_pool) it just checks the limits. The only place where the limit really matters is when a pool is indexed (RXE_POOL_INDEXED). Then there is a bitmask used to allocate the indices for the objects which consumes one byte for each 8 objects. 

From Jason's comment I think just setting the limits fairly large but not excessive is the right approach.

Bob

Re: [PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Jason Gunthorpe]

On Fri, Jun 25, 2021 at 12:13:57AM -0500, Bob Pearson wrote:
> On 6/24/21 4:21 PM, Shoaib Rao wrote:
> > 
> > On 6/18/21 4:25 PM, Jason Gunthorpe wrote:
> >> On Fri, Jun 18, 2021 at 01:58:48PM -0500, Bob Pearson wrote:
> >>> On 6/18/21 11:33 AM, Jason Gunthorpe wrote:
> >>>> On Thu, Jun 17, 2021 at 10:56:58PM -0500, Bob Pearson wrote:
> >>>>  
> >>>>> It isn't my call. But I am in favor of tunable parameters. -- Bob Pearson
> >>>> Can we just delete the concept completely?
> >>>>
> >>>> Jason
> >>>>
> >>> Not sure where you are headed here. Do you mean just lift the limits
> >>> all together?
> >> Yes.. The spec doesn't have like a UCONTEXT limit for instance, and
> >> real HW like mlx5 has huge reported limits anyhow.
> > 
> > These limits are reported via uverbs, so what do we report without current applications. Creating pool also requires limits but I guess we can use something like -1 to indicate there is no limit. I would have to look at all the values to see if we can implement this.
> > 
> > Shoaib
> > 
> > 
> >>
> >> Jason
>
> The object create in pools (rxe_alloc_locked) just calls kzalloc for
> objects allocated by rxe and checks the limits. For objects
> allocated by rdma-core (__rxe_add_to_pool) it just checks the
> limits. The only place where the limit really matters is when a pool
> is indexed (RXE_POOL_INDEXED). Then there is a bitmask used to
> allocate the indices for the objects which consumes one byte for
> each 8 objects.

Use an ida or xarray instead?

Jason

Re: [PATCH v1 3/3] RDMA/rxe: Increase value of RXE_MAX_AH

[Shoaib Rao]

On 6/25/21 5:49 AM, Jason Gunthorpe wrote:
> On Fri, Jun 25, 2021 at 12:13:57AM -0500, Bob Pearson wrote:
>> On 6/24/21 4:21 PM, Shoaib Rao wrote:
>>> On 6/18/21 4:25 PM, Jason Gunthorpe wrote:
>>>> On Fri, Jun 18, 2021 at 01:58:48PM -0500, Bob Pearson wrote:
>>>>> On 6/18/21 11:33 AM, Jason Gunthorpe wrote:
>>>>>> On Thu, Jun 17, 2021 at 10:56:58PM -0500, Bob Pearson wrote:
>>>>>>   
>>>>>>> It isn't my call. But I am in favor of tunable parameters. -- Bob Pearson
>>>>>> Can we just delete the concept completely?
>>>>>>
>>>>>> Jason
>>>>>>
>>>>> Not sure where you are headed here. Do you mean just lift the limits
>>>>> all together?
>>>> Yes.. The spec doesn't have like a UCONTEXT limit for instance, and
>>>> real HW like mlx5 has huge reported limits anyhow.
>>> These limits are reported via uverbs, so what do we report without current applications. Creating pool also requires limits but I guess we can use something like -1 to indicate there is no limit. I would have to look at all the values to see if we can implement this.
>>>
>>> Shoaib
>>>
>>>
>>>> Jason
>> The object create in pools (rxe_alloc_locked) just calls kzalloc for
>> objects allocated by rxe and checks the limits. For objects
>> allocated by rdma-core (__rxe_add_to_pool) it just checks the
>> limits. The only place where the limit really matters is when a pool
>> is indexed (RXE_POOL_INDEXED). Then there is a bitmask used to
>> allocate the indices for the objects which consumes one byte for
>> each 8 objects.
> Use an ida or xarray instead?
>
> Jason

We do not have to change any data structures, just decide on the max limit.

RXE_POOL_INDEX flag as shown below is used to indicate what type of 
index to use and is not an issue

         if (rxe_type_info[type].flags & RXE_POOL_INDEX) {
                 err = rxe_pool_init_index(pool,
rxe_type_info[type].max_index,
rxe_type_info[type].min_index);
                 if (err)
                         goto out;
         }

And

static int rxe_pool_init_index(struct rxe_pool *pool, u32 max, u32 min)
{

So the maximum index can be U32_MAX or for now we can limit is to (1 << 
20) i.e 1048576

I am assuming that changing the limits for the objects I listed earlier 
(list below) is fine.

Shoaib

RXE_MAX_QP
RXE_MAX_QP_WR
RXE_MAX_CQ
RXE_MAX_MR
RXE_MAX_PD
RXE_MAX_AH
RXE_MAX_FMR
RXE_MAX_SRQ
RXE_MAX_SRQ_WR
RXE_MAX_PKEYS
RXE_MAX_UCONTEXT