[PATCH v2 0/2] rxe should use same buffer size for SGE's and inline data

[PATCH v2 0/2] rxe should use same buffer size for SGE's and inline data

[rao Shoaib]

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

I have incorportaed suggestions from Jason. There are two patches.
Patch #1 introduces max WQE size as suggested by Jason
Patch #2 allocates resources requested and makes sure that the buffer size
         is same for SG entries and inline data, maximum of the two values
	 requested is used.

Rao Shoaib (2):
  Introduce maximum WQE size to check limits
  SGE buffer and max_inline data must have same size

 drivers/infiniband/sw/rxe/rxe_param.h |  3 ++-
 drivers/infiniband/sw/rxe/rxe_qp.c    | 26 ++++++++++++++------------
 2 files changed, 16 insertions(+), 13 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/2] Introduce maximum WQE size to check limits

[rao Shoaib]

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

Introduce maximum WQE size to impose limits on max SGE's and inline data

Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
---
 drivers/infiniband/sw/rxe/rxe_param.h | 3 ++-
 drivers/infiniband/sw/rxe/rxe_qp.c    | 7 +++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
index 1b596fb..31fb5c7 100644
--- a/drivers/infiniband/sw/rxe/rxe_param.h
+++ b/drivers/infiniband/sw/rxe/rxe_param.h
@@ -68,7 +68,6 @@ enum rxe_device_param {
 	RXE_HW_VER			= 0,
 	RXE_MAX_QP			= 0x10000,
 	RXE_MAX_QP_WR			= 0x4000,
-	RXE_MAX_INLINE_DATA		= 400,
 	RXE_DEVICE_CAP_FLAGS		= IB_DEVICE_BAD_PKEY_CNTR
 					| IB_DEVICE_BAD_QKEY_CNTR
 					| IB_DEVICE_AUTO_PATH_MIG
@@ -79,7 +78,9 @@ enum rxe_device_param {
 					| IB_DEVICE_RC_RNR_NAK_GEN
 					| IB_DEVICE_SRQ_RESIZE
 					| IB_DEVICE_MEM_MGT_EXTENSIONS,
+	RXE_MAX_WQE_SIZE		= 0x2d0, /* For RXE_MAX_SGE */
 	RXE_MAX_SGE			= 32,
+	RXE_MAX_INLINE_DATA		= RXE_MAX_WQE_SIZE,
 	RXE_MAX_SGE_RD			= 32,
 	RXE_MAX_CQ			= 16384,
 	RXE_MAX_LOG_CQE			= 15,
diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index aeea994..323e43d 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -78,9 +78,12 @@ static int rxe_qp_chk_cap(struct rxe_dev *rxe, struct ib_qp_cap *cap,
 		}
 	}
 
-	if (cap->max_inline_data > rxe->max_inline_data) {
+	if (cap->max_inline_data >
+	    rxe->max_inline_data - sizeof(struct rxe_send_wqe)) {
 		pr_warn("invalid max inline data = %d > %d\n",
-			cap->max_inline_data, rxe->max_inline_data);
+			cap->max_inline_data,
+			rxe->max_inline_data -
+			    (u32)sizeof(struct rxe_send_wqe));
 		goto err1;
 	}
 
-- 
1.8.3.1

Re: [PATCH v2 1/2] Introduce maximum WQE size to check limits

[Jason Gunthorpe]

On Mon, Nov 18, 2019 at 11:54:38AM -0800, rao Shoaib wrote:
> From: Rao Shoaib <rao.shoaib@oracle.com>
> 
> Introduce maximum WQE size to impose limits on max SGE's and inline data
> 
> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
>  drivers/infiniband/sw/rxe/rxe_param.h | 3 ++-
>  drivers/infiniband/sw/rxe/rxe_qp.c    | 7 +++++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
> index 1b596fb..31fb5c7 100644
> +++ b/drivers/infiniband/sw/rxe/rxe_param.h
> @@ -68,7 +68,6 @@ enum rxe_device_param {
>  	RXE_HW_VER			= 0,
>  	RXE_MAX_QP			= 0x10000,
>  	RXE_MAX_QP_WR			= 0x4000,
> -	RXE_MAX_INLINE_DATA		= 400,
>  	RXE_DEVICE_CAP_FLAGS		= IB_DEVICE_BAD_PKEY_CNTR
>  					| IB_DEVICE_BAD_QKEY_CNTR
>  					| IB_DEVICE_AUTO_PATH_MIG
> @@ -79,7 +78,9 @@ enum rxe_device_param {
>  					| IB_DEVICE_RC_RNR_NAK_GEN
>  					| IB_DEVICE_SRQ_RESIZE
>  					| IB_DEVICE_MEM_MGT_EXTENSIONS,
> +	RXE_MAX_WQE_SIZE		= 0x2d0, /* For RXE_MAX_SGE */

This shouldn't just be a random constant, I think you are trying to
say:

  RXE_MAX_WQE_SIZE = sizeof(struct rxe_send_wqe) + sizeof(struct ib_sge)*RXE_MAX_SGE

Just say that

>  	RXE_MAX_SGE			= 32,
> +	RXE_MAX_INLINE_DATA		= RXE_MAX_WQE_SIZE,

This is mixed up now, it should be

  RXE_MAX_INLINE_DATA = RXE_MAX_WQE_SIZE - sizeof(rxe_send_wqe)

> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
> index aeea994..323e43d 100644
> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
> @@ -78,9 +78,12 @@ static int rxe_qp_chk_cap(struct rxe_dev *rxe, struct ib_qp_cap *cap,
>  		}
>  	}
>  
> -	if (cap->max_inline_data > rxe->max_inline_data) {
> +	if (cap->max_inline_data >
> +	    rxe->max_inline_data - sizeof(struct rxe_send_wqe)) {
>  		pr_warn("invalid max inline data = %d > %d\n",
> -			cap->max_inline_data, rxe->max_inline_data);
> +			cap->max_inline_data,
> +			rxe->max_inline_data -
> +			    (u32)sizeof(struct rxe_send_wqe));

Then this isn't needed

And this code in the other patch:

+	wqe_size = max_t(int, init->cap.max_send_sge * sizeof(struct ib_sge),
+			 init->cap.max_inline_data);
+	qp->sq.max_sge = wqe_size/sizeof(struct ib_sge);
+	qp->sq.max_inline = wqe_size;

Makes sense as both max_inline_data and 'init->cap.max_send_sge *
sizeof(struct ib_sge)' are exclusive of the wqe header

Jason

Re: [PATCH v2 1/2] Introduce maximum WQE size to check limits

[Rao Shoaib]

On 11/19/19 12:31 PM, Jason Gunthorpe wrote:
> On Mon, Nov 18, 2019 at 11:54:38AM -0800, rao Shoaib wrote:
>> From: Rao Shoaib <rao.shoaib@oracle.com>
>>
>> Introduce maximum WQE size to impose limits on max SGE's and inline data
>>
>> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
>>   drivers/infiniband/sw/rxe/rxe_param.h | 3 ++-
>>   drivers/infiniband/sw/rxe/rxe_qp.c    | 7 +++++--
>>   2 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
>> index 1b596fb..31fb5c7 100644
>> +++ b/drivers/infiniband/sw/rxe/rxe_param.h
>> @@ -68,7 +68,6 @@ enum rxe_device_param {
>>   	RXE_HW_VER			= 0,
>>   	RXE_MAX_QP			= 0x10000,
>>   	RXE_MAX_QP_WR			= 0x4000,
>> -	RXE_MAX_INLINE_DATA		= 400,
>>   	RXE_DEVICE_CAP_FLAGS		= IB_DEVICE_BAD_PKEY_CNTR
>>   					| IB_DEVICE_BAD_QKEY_CNTR
>>   					| IB_DEVICE_AUTO_PATH_MIG
>> @@ -79,7 +78,9 @@ enum rxe_device_param {
>>   					| IB_DEVICE_RC_RNR_NAK_GEN
>>   					| IB_DEVICE_SRQ_RESIZE
>>   					| IB_DEVICE_MEM_MGT_EXTENSIONS,
>> +	RXE_MAX_WQE_SIZE		= 0x2d0, /* For RXE_MAX_SGE */
> This shouldn't just be a random constant, I think you are trying to
> say:
>
>    RXE_MAX_WQE_SIZE = sizeof(struct rxe_send_wqe) + sizeof(struct ib_sge)*RXE_MAX_SGE
I thought you wanted this value to be independent of RXE_MAX_SGE, else 
why are defining it.
>
> Just say that
>
>>   	RXE_MAX_SGE			= 32,
>> +	RXE_MAX_INLINE_DATA		= RXE_MAX_WQE_SIZE,
> This is mixed up now, it should be
>
>    RXE_MAX_INLINE_DATA = RXE_MAX_WQE_SIZE - sizeof(rxe_send_wqe)

I agree to what you are suggesting, it will make the current patch 
better. However, In my previous patch I had

RXE_MAX_INLINE_DATA		= RXE_MAX_SGE * sizeof(struct ib_sge)

IMHO that conveys the intent much better. I do not see the reason for 
defining RXE_MAX_WQE_SIZE, ib_device_attr does not even have an entry 
for it and hence the value is not used anywhere by rxe or by any other 
relevant driver.

I will re-submit with the changes you have suggested.

Shoaib


>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
>> index aeea994..323e43d 100644
>> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
>> @@ -78,9 +78,12 @@ static int rxe_qp_chk_cap(struct rxe_dev *rxe, struct ib_qp_cap *cap,
>>   		}
>>   	}
>>   
>> -	if (cap->max_inline_data > rxe->max_inline_data) {
>> +	if (cap->max_inline_data >
>> +	    rxe->max_inline_data - sizeof(struct rxe_send_wqe)) {
>>   		pr_warn("invalid max inline data = %d > %d\n",
>> -			cap->max_inline_data, rxe->max_inline_data);
>> +			cap->max_inline_data,
>> +			rxe->max_inline_data -
>> +			    (u32)sizeof(struct rxe_send_wqe));
> Then this isn't needed
>
> And this code in the other patch:
>
> +	wqe_size = max_t(int, init->cap.max_send_sge * sizeof(struct ib_sge),
> +			 init->cap.max_inline_data);
> +	qp->sq.max_sge = wqe_size/sizeof(struct ib_sge);
> +	qp->sq.max_inline = wqe_size;
>
> Makes sense as both max_inline_data and 'init->cap.max_send_sge *
> sizeof(struct ib_sge)' are exclusive of the wqe header
>
> Jason

Re: [PATCH v2 1/2] Introduce maximum WQE size to check limits

[Jason Gunthorpe]

On Tue, Nov 19, 2019 at 02:38:23PM -0800, Rao Shoaib wrote:
> 
> On 11/19/19 12:31 PM, Jason Gunthorpe wrote:
> > On Mon, Nov 18, 2019 at 11:54:38AM -0800, rao Shoaib wrote:
> > > From: Rao Shoaib <rao.shoaib@oracle.com>
> > > 
> > > Introduce maximum WQE size to impose limits on max SGE's and inline data
> > > 
> > > Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
> > >   drivers/infiniband/sw/rxe/rxe_param.h | 3 ++-
> > >   drivers/infiniband/sw/rxe/rxe_qp.c    | 7 +++++--
> > >   2 files changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
> > > index 1b596fb..31fb5c7 100644
> > > +++ b/drivers/infiniband/sw/rxe/rxe_param.h
> > > @@ -68,7 +68,6 @@ enum rxe_device_param {
> > >   	RXE_HW_VER			= 0,
> > >   	RXE_MAX_QP			= 0x10000,
> > >   	RXE_MAX_QP_WR			= 0x4000,
> > > -	RXE_MAX_INLINE_DATA		= 400,
> > >   	RXE_DEVICE_CAP_FLAGS		= IB_DEVICE_BAD_PKEY_CNTR
> > >   					| IB_DEVICE_BAD_QKEY_CNTR
> > >   					| IB_DEVICE_AUTO_PATH_MIG
> > > @@ -79,7 +78,9 @@ enum rxe_device_param {
> > >   					| IB_DEVICE_RC_RNR_NAK_GEN
> > >   					| IB_DEVICE_SRQ_RESIZE
> > >   					| IB_DEVICE_MEM_MGT_EXTENSIONS,
> > > +	RXE_MAX_WQE_SIZE		= 0x2d0, /* For RXE_MAX_SGE */
> > This shouldn't just be a random constant, I think you are trying to
> > say:
> > 
> >    RXE_MAX_WQE_SIZE = sizeof(struct rxe_send_wqe) + sizeof(struct ib_sge)*RXE_MAX_SGE

> I thought you wanted this value to be independent of RXE_MAX_SGE, else why
> are defining it.

Then define 

   RXE_MAX_SGE = (RXE_MAX_WQE_SIZE - sizeof(rxe_send_wqe))/sizeof(rxe_sge)

And drive everything off RXE_MAX_WQE_SIZE, which sounds good

> > Just say that
> > 
> > >   	RXE_MAX_SGE			= 32,
> > > +	RXE_MAX_INLINE_DATA		= RXE_MAX_WQE_SIZE,
> > This is mixed up now, it should be
> > 
> >    RXE_MAX_INLINE_DATA = RXE_MAX_WQE_SIZE - sizeof(rxe_send_wqe)
> 
> I agree to what you are suggesting, it will make the current patch better.
> However, In my previous patch I had
> 
> RXE_MAX_INLINE_DATA		= RXE_MAX_SGE * sizeof(struct ib_sge)
> 
> IMHO that conveys the intent much better. I do not see the reason for
> defining RXE_MAX_WQE_SIZE, ib_device_attr does not even have an entry for it
> and hence the value is not used anywhere by rxe or by any other relevant
> driver.

Because WQE_SIZE is what you are actually concerned with here, using
MAX_SGE as a proxy for the max WQE is confusing

Jason

Re: [PATCH v2 1/2] Introduce maximum WQE size to check limits

[Rao Shoaib]

On 11/19/19 3:13 PM, Jason Gunthorpe wrote:
> On Tue, Nov 19, 2019 at 02:38:23PM -0800, Rao Shoaib wrote:
>> On 11/19/19 12:31 PM, Jason Gunthorpe wrote:
>>> On Mon, Nov 18, 2019 at 11:54:38AM -0800, rao Shoaib wrote:
>>>> From: Rao Shoaib <rao.shoaib@oracle.com>
>>>>
>>>> Introduce maximum WQE size to impose limits on max SGE's and inline data
>>>>
>>>> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
>>>>    drivers/infiniband/sw/rxe/rxe_param.h | 3 ++-
>>>>    drivers/infiniband/sw/rxe/rxe_qp.c    | 7 +++++--
>>>>    2 files changed, 7 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
>>>> index 1b596fb..31fb5c7 100644
>>>> +++ b/drivers/infiniband/sw/rxe/rxe_param.h
>>>> @@ -68,7 +68,6 @@ enum rxe_device_param {
>>>>    	RXE_HW_VER			= 0,
>>>>    	RXE_MAX_QP			= 0x10000,
>>>>    	RXE_MAX_QP_WR			= 0x4000,
>>>> -	RXE_MAX_INLINE_DATA		= 400,
>>>>    	RXE_DEVICE_CAP_FLAGS		= IB_DEVICE_BAD_PKEY_CNTR
>>>>    					| IB_DEVICE_BAD_QKEY_CNTR
>>>>    					| IB_DEVICE_AUTO_PATH_MIG
>>>> @@ -79,7 +78,9 @@ enum rxe_device_param {
>>>>    					| IB_DEVICE_RC_RNR_NAK_GEN
>>>>    					| IB_DEVICE_SRQ_RESIZE
>>>>    					| IB_DEVICE_MEM_MGT_EXTENSIONS,
>>>> +	RXE_MAX_WQE_SIZE		= 0x2d0, /* For RXE_MAX_SGE */
>>> This shouldn't just be a random constant, I think you are trying to
>>> say:
>>>
>>>     RXE_MAX_WQE_SIZE = sizeof(struct rxe_send_wqe) + sizeof(struct ib_sge)*RXE_MAX_SGE
>> I thought you wanted this value to be independent of RXE_MAX_SGE, else why
>> are defining it.
> Then define
>
>     RXE_MAX_SGE = (RXE_MAX_WQE_SIZE - sizeof(rxe_send_wqe))/sizeof(rxe_sge)
>
> And drive everything off RXE_MAX_WQE_SIZE, which sounds good
>
>>> Just say that
>>>
>>>>    	RXE_MAX_SGE			= 32,
>>>> +	RXE_MAX_INLINE_DATA		= RXE_MAX_WQE_SIZE,
>>> This is mixed up now, it should be
>>>
>>>     RXE_MAX_INLINE_DATA = RXE_MAX_WQE_SIZE - sizeof(rxe_send_wqe)
>> I agree to what you are suggesting, it will make the current patch better.
>> However, In my previous patch I had
>>
>> RXE_MAX_INLINE_DATA		= RXE_MAX_SGE * sizeof(struct ib_sge)
>>
>> IMHO that conveys the intent much better. I do not see the reason for
>> defining RXE_MAX_WQE_SIZE, ib_device_attr does not even have an entry for it
>> and hence the value is not used anywhere by rxe or by any other relevant
>> driver.
> Because WQE_SIZE is what you are actually concerned with here, using
> MAX_SGE as a proxy for the max WQE is confusing
>
> Jason

My intent is that we calculate and use the maximum buffer size using the 
maximum of, number of SGE's and inline data requested, not controlling 
the size of WQE buffer. If I was trying to limit WQE size I would agree 
with you. Defining MAX_WQE_SIZE based on MAX_SGE and recalculating 
MAX_SGE does not make sense to me. MAX_SGE and inline_data are 
independent variables and define the size of wqe size not the other wise 
around. I did make inline_dependent on MAX_SGE.

Your and my preferences can be different but you are the maintainer and 
what you say goes. We have had a good discussion and I am going with 
your previous suggestion.

Kind Regards,

Shoaib

Re: [PATCH v2 1/2] Introduce maximum WQE size to check limits

[Jason Gunthorpe]

On Tue, Nov 19, 2019 at 03:55:35PM -0800, Rao Shoaib wrote:

> My intent is that we calculate and use the maximum buffer size using the
> maximum of, number of SGE's and inline data requested, not controlling the
> size of WQE buffer. If I was trying to limit WQE size I would agree with
> you. Defining MAX_WQE_SIZE based on MAX_SGE and recalculating MAX_SGE does
> not make sense to me. MAX_SGE and inline_data are independent variables and
> define the size of wqe size not the other wise around. I did make
> inline_dependent on MAX_SGE.

What you are trying to do is limit the size of the WQE to some maximum
and from there you can compute the upper limit on the SGE and the
inline data arrays, depending on how the WQE is being used.

If a limit must be had then the limit is the WQE size. It is also
reasonable to ask why rxe has a limit at all, or why the limit is so
small ie why can't it be 2k or something? But that is something else

Jason

Re: [PATCH v2 1/2] Introduce maximum WQE size to check limits

[Rao Shoaib]

Any update on my patch?

If there is some change needed please let me know.

Shoaib

On 11/19/19 4:08 PM, Jason Gunthorpe wrote:
> On Tue, Nov 19, 2019 at 03:55:35PM -0800, Rao Shoaib wrote:
>
>> My intent is that we calculate and use the maximum buffer size using the
>> maximum of, number of SGE's and inline data requested, not controlling the
>> size of WQE buffer. If I was trying to limit WQE size I would agree with
>> you. Defining MAX_WQE_SIZE based on MAX_SGE and recalculating MAX_SGE does
>> not make sense to me. MAX_SGE and inline_data are independent variables and
>> define the size of wqe size not the other wise around. I did make
>> inline_dependent on MAX_SGE.
> What you are trying to do is limit the size of the WQE to some maximum
> and from there you can compute the upper limit on the SGE and the
> inline data arrays, depending on how the WQE is being used.
>
> If a limit must be had then the limit is the WQE size. It is also
> reasonable to ask why rxe has a limit at all, or why the limit is so
> small ie why can't it be 2k or something? But that is something else
>
> Jason

Re: [PATCH v2 1/2] Introduce maximum WQE size to check limits

[Jason Gunthorpe]

On Tue, Dec 17, 2019 at 11:38:52AM -0800, Rao Shoaib wrote:
> Any update on my patch?
> 
> If there is some change needed please let me know.

You need to repost it with the comments addressed

https://patchwork.kernel.org/patch/11250179/

Jason

Re: [PATCH v2 1/2] Introduce maximum WQE size to check limits

[Rao Shoaib]

Hi Jason,

I thought I had addressed the comments and literally did what you 
suggested. Sorry if I missed something, can you please point it out.

Shoaib

On 12/19/19 10:25 AM, Jason Gunthorpe wrote:
> On Tue, Dec 17, 2019 at 11:38:52AM -0800, Rao Shoaib wrote:
>> Any update on my patch?
>>
>> If there is some change needed please let me know.
> You need to repost it with the comments addressed
>
> https://patchwork.kernel.org/patch/11250179/
>
> Jason
>

Re: [PATCH v2 1/2] Introduce maximum WQE size to check limits

[Rao Shoaib]

On 12/19/19 10:25 AM, Jason Gunthorpe wrote:
> On Tue, Dec 17, 2019 at 11:38:52AM -0800, Rao Shoaib wrote:
>> Any update on my patch?
>>
>> If there is some change needed please let me know.
> You need to repost it with the comments addressed
>
> https://patchwork.kernel.org/patch/11250179/
>
> Jason
>
Jason,

Following is a pointer to the patch that I posted in response to your 
comments

https://www.spinics.net/lists/linux-rdma/msg86241.html

I posted this on Nov 18. Can you please take a look and let me know what 
else has to be done.

Shoaib

Re: [PATCH v2 1/2] Introduce maximum WQE size to check limits

[Jason Gunthorpe]

On Mon, Jan 13, 2020 at 10:35:14AM -0800, Rao Shoaib wrote:
> 
> On 12/19/19 10:25 AM, Jason Gunthorpe wrote:
> > On Tue, Dec 17, 2019 at 11:38:52AM -0800, Rao Shoaib wrote:
> > > Any update on my patch?
> > > 
> > > If there is some change needed please let me know.
> > You need to repost it with the comments addressed
> > 
> > https://patchwork.kernel.org/patch/11250179/
> > 
> > Jason
> > 
> Jason,
> 
> Following is a pointer to the patch that I posted in response to your
> comments
> 
> https://www.spinics.net/lists/linux-rdma/msg86241.html
> 
> I posted this on Nov 18. Can you please take a look and let me know what
> else has to be done.

You mean this:

https://www.spinics.net/lists/linux-rdma/msg86333.html

?

Don't mix the inline size and the # SGEs. They both drive the maximum
WQE size and all the math should be directly connected.

Jason

Re: [PATCH v2 1/2] Introduce maximum WQE size to check limits

[Rao Shoaib]

On 1/13/20 10:47 AM, Jason Gunthorpe wrote:
> On Mon, Jan 13, 2020 at 10:35:14AM -0800, Rao Shoaib wrote:
>> On 12/19/19 10:25 AM, Jason Gunthorpe wrote:
>>> On Tue, Dec 17, 2019 at 11:38:52AM -0800, Rao Shoaib wrote:
>>>> Any update on my patch?
>>>>
>>>> If there is some change needed please let me know.
>>> You need to repost it with the comments addressed
>>>
>>> https://patchwork.kernel.org/patch/11250179/
>>>
>>> Jason
>>>
>> Jason,
>>
>> Following is a pointer to the patch that I posted in response to your
>> comments
>>
>> https://www.spinics.net/lists/linux-rdma/msg86241.html
>>
>> I posted this on Nov 18. Can you please take a look and let me know what
>> else has to be done.
> You mean this:
>
> https://www.spinics.net/lists/linux-rdma/msg86333.html
>
> ?
>
> Don't mix the inline size and the # SGEs. They both drive the maximum
> WQE size and all the math should be directly connected.
>
> Jason

Nope. I have just resent the patch. Sorry for the confusion.

Shoaib.

[PATCH v2 2/2] SGE buffer and max_inline data must have same size

[rao Shoaib]

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

SGE buffer size and max_inline data should be same. Maximum of the
two values requested is used.

Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
---
 drivers/infiniband/sw/rxe/rxe_qp.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 323e43d..1062f60 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -238,18 +238,17 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp,
 		return err;
 	qp->sk->sk->sk_user_data = qp;
 
-	qp->sq.max_wr		= init->cap.max_send_wr;
-	qp->sq.max_sge		= init->cap.max_send_sge;
-	qp->sq.max_inline	= init->cap.max_inline_data;
-
-	wqe_size = max_t(int, sizeof(struct rxe_send_wqe) +
-			 qp->sq.max_sge * sizeof(struct ib_sge),
-			 sizeof(struct rxe_send_wqe) +
-			 qp->sq.max_inline);
-
-	qp->sq.queue = rxe_queue_init(rxe,
-				      &qp->sq.max_wr,
-				      wqe_size);
+	wqe_size = max_t(int, init->cap.max_send_sge * sizeof(struct ib_sge),
+			 init->cap.max_inline_data);
+	qp->sq.max_sge = wqe_size/sizeof(struct ib_sge);
+	qp->sq.max_inline = wqe_size;
+
+	wqe_size += sizeof(struct rxe_send_wqe);
+
+	qp->sq.max_wr = init->cap.max_send_wr;
+
+	qp->sq.queue = rxe_queue_init(rxe, &qp->sq.max_wr, wqe_size);
+
 	if (!qp->sq.queue)
 		return -ENOMEM;
 
-- 
1.8.3.1