* [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs
@ 2021-09-15 1:12 Rao Shoaib
2021-09-27 19:19 ` Jason Gunthorpe
2021-10-04 18:57 ` Jason Gunthorpe
0 siblings, 2 replies; 11+ messages in thread
From: Rao Shoaib @ 2021-09-15 1:12 UTC (permalink / raw)
To: linux-rdma, jgg; +Cc: rao.shoaib, Rao Shoaib
In our internal testing we have found that
default maximum values are too small.
Ideally there should be no limits, but since
maximum values are reported via ibv_query_device,
we have to return some value. So, the default
maximums have been changed to large values.
Signed-off-by: Rao Shoaib <Rao.Shoaib@oracle.com>
---
Resubmitting the patch after applying Bob's latest patches and testing
using via rping.
drivers/infiniband/sw/rxe/rxe_param.h | 30 ++++++++++++++-------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
index 742e6ec93686..d703a54df4ec 100644
--- a/drivers/infiniband/sw/rxe/rxe_param.h
+++ b/drivers/infiniband/sw/rxe/rxe_param.h
@@ -9,6 +9,8 @@
#include <uapi/rdma/rdma_user_rxe.h>
+#define DEFAULT_MAX_VALUE (1 << 20)
+
static inline enum ib_mtu rxe_mtu_int_to_enum(int mtu)
{
if (mtu < 256)
@@ -37,7 +39,7 @@ static inline enum ib_mtu eth_mtu_int_to_enum(int mtu)
enum rxe_device_param {
RXE_MAX_MR_SIZE = -1ull,
RXE_PAGE_SIZE_CAP = 0xfffff000,
- RXE_MAX_QP_WR = 0x4000,
+ RXE_MAX_QP_WR = DEFAULT_MAX_VALUE,
RXE_DEVICE_CAP_FLAGS = IB_DEVICE_BAD_PKEY_CNTR
| IB_DEVICE_BAD_QKEY_CNTR
| IB_DEVICE_AUTO_PATH_MIG
@@ -58,42 +60,42 @@ enum rxe_device_param {
RXE_MAX_INLINE_DATA = RXE_MAX_WQE_SIZE -
sizeof(struct rxe_send_wqe),
RXE_MAX_SGE_RD = 32,
- RXE_MAX_CQ = 16384,
+ RXE_MAX_CQ = DEFAULT_MAX_VALUE,
RXE_MAX_LOG_CQE = 15,
- RXE_MAX_PD = 0x7ffc,
+ RXE_MAX_PD = DEFAULT_MAX_VALUE,
RXE_MAX_QP_RD_ATOM = 128,
RXE_MAX_RES_RD_ATOM = 0x3f000,
RXE_MAX_QP_INIT_RD_ATOM = 128,
RXE_MAX_MCAST_GRP = 8192,
RXE_MAX_MCAST_QP_ATTACH = 56,
RXE_MAX_TOT_MCAST_QP_ATTACH = 0x70000,
- RXE_MAX_AH = 100,
- RXE_MAX_SRQ_WR = 0x4000,
+ RXE_MAX_AH = DEFAULT_MAX_VALUE,
+ RXE_MAX_SRQ_WR = DEFAULT_MAX_VALUE,
RXE_MIN_SRQ_WR = 1,
RXE_MAX_SRQ_SGE = 27,
RXE_MIN_SRQ_SGE = 1,
RXE_MAX_FMR_PAGE_LIST_LEN = 512,
- RXE_MAX_PKEYS = 1,
+ RXE_MAX_PKEYS = 64,
RXE_LOCAL_CA_ACK_DELAY = 15,
- RXE_MAX_UCONTEXT = 512,
+ RXE_MAX_UCONTEXT = DEFAULT_MAX_VALUE,
RXE_NUM_PORT = 1,
- RXE_MAX_QP = 0x10000,
RXE_MIN_QP_INDEX = 16,
- RXE_MAX_QP_INDEX = 0x00020000,
+ RXE_MAX_QP_INDEX = DEFAULT_MAX_VALUE,
+ RXE_MAX_QP = DEFAULT_MAX_VALUE - RXE_MIN_QP_INDEX,
- RXE_MAX_SRQ = 0x00001000,
RXE_MIN_SRQ_INDEX = 0x00020001,
- RXE_MAX_SRQ_INDEX = 0x00040000,
+ RXE_MAX_SRQ_INDEX = DEFAULT_MAX_VALUE,
+ RXE_MAX_SRQ = DEFAULT_MAX_VALUE - RXE_MIN_SRQ_INDEX,
- RXE_MAX_MR = 0x00001000,
- RXE_MAX_MW = 0x00001000,
RXE_MIN_MR_INDEX = 0x00000001,
- RXE_MAX_MR_INDEX = 0x00010000,
+ RXE_MAX_MR_INDEX = DEFAULT_MAX_VALUE,
+ RXE_MAX_MR = DEFAULT_MAX_VALUE - RXE_MIN_MR_INDEX,
RXE_MIN_MW_INDEX = 0x00010001,
RXE_MAX_MW_INDEX = 0x00020000,
+ RXE_MAX_MW = 0x00001000,
RXE_MAX_PKT_PER_ACK = 64,
--
2.27.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs
2021-09-15 1:12 [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs Rao Shoaib
@ 2021-09-27 19:19 ` Jason Gunthorpe
2021-09-28 1:46 ` Zhu Yanjun
2021-10-04 18:57 ` Jason Gunthorpe
1 sibling, 1 reply; 11+ messages in thread
From: Jason Gunthorpe @ 2021-09-27 19:19 UTC (permalink / raw)
To: Rao Shoaib, Bob Pearson, Zhu Yanjun; +Cc: linux-rdma
On Tue, Sep 14, 2021 at 06:12:20PM -0700, Rao Shoaib wrote:
> In our internal testing we have found that
> default maximum values are too small.
> Ideally there should be no limits, but since
> maximum values are reported via ibv_query_device,
> we have to return some value. So, the default
> maximums have been changed to large values.
>
> Signed-off-by: Rao Shoaib <Rao.Shoaib@oracle.com>
> ---
>
> Resubmitting the patch after applying Bob's latest patches and testing
> using via rping.
>
> drivers/infiniband/sw/rxe/rxe_param.h | 30 ++++++++++++++-------------
> 1 file changed, 16 insertions(+), 14 deletions(-)
So are we good with this? Bob? Zhu?
> - RXE_MAX_MR_INDEX = 0x00010000,
> + RXE_MAX_MR_INDEX = DEFAULT_MAX_VALUE,
> + RXE_MAX_MR = DEFAULT_MAX_VALUE - RXE_MIN_MR_INDEX,
Bob, were you saying this was what needed to be bigger to pass
blktests??
Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs
2021-09-27 19:19 ` Jason Gunthorpe
@ 2021-09-28 1:46 ` Zhu Yanjun
2021-09-28 4:38 ` Shoaib Rao
0 siblings, 1 reply; 11+ messages in thread
From: Zhu Yanjun @ 2021-09-28 1:46 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Rao Shoaib, Bob Pearson, RDMA mailing list
On Tue, Sep 28, 2021 at 3:19 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Tue, Sep 14, 2021 at 06:12:20PM -0700, Rao Shoaib wrote:
> > In our internal testing we have found that
> > default maximum values are too small.
> > Ideally there should be no limits, but since
> > maximum values are reported via ibv_query_device,
> > we have to return some value. So, the default
> > maximums have been changed to large values.
> >
> > Signed-off-by: Rao Shoaib <Rao.Shoaib@oracle.com>
> > ---
> >
> > Resubmitting the patch after applying Bob's latest patches and testing
> > using via rping.
> >
> > drivers/infiniband/sw/rxe/rxe_param.h | 30 ++++++++++++++-------------
> > 1 file changed, 16 insertions(+), 14 deletions(-)
>
> So are we good with this? Bob? Zhu?
I have already checked this commit. And I have found 2 problems with
this commit.
This commit changes many MAXs.
And now rxe is not stable enough. Not sure this commit will cause the
new problems.
Zhu Yanjun
>
> > - RXE_MAX_MR_INDEX = 0x00010000,
> > + RXE_MAX_MR_INDEX = DEFAULT_MAX_VALUE,
> > + RXE_MAX_MR = DEFAULT_MAX_VALUE - RXE_MIN_MR_INDEX,
>
> Bob, were you saying this was what needed to be bigger to pass
> blktests??
>
> Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs
2021-09-28 1:46 ` Zhu Yanjun
@ 2021-09-28 4:38 ` Shoaib Rao
2021-09-28 6:55 ` Zhu Yanjun
0 siblings, 1 reply; 11+ messages in thread
From: Shoaib Rao @ 2021-09-28 4:38 UTC (permalink / raw)
To: Zhu Yanjun, Jason Gunthorpe; +Cc: Bob Pearson, RDMA mailing list
On 9/27/21 6:46 PM, Zhu Yanjun wrote:
> On Tue, Sep 28, 2021 at 3:19 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>> On Tue, Sep 14, 2021 at 06:12:20PM -0700, Rao Shoaib wrote:
>>> In our internal testing we have found that
>>> default maximum values are too small.
>>> Ideally there should be no limits, but since
>>> maximum values are reported via ibv_query_device,
>>> we have to return some value. So, the default
>>> maximums have been changed to large values.
>>>
>>> Signed-off-by: Rao Shoaib <Rao.Shoaib@oracle.com>
>>> ---
>>>
>>> Resubmitting the patch after applying Bob's latest patches and testing
>>> using via rping.
>>>
>>> drivers/infiniband/sw/rxe/rxe_param.h | 30 ++++++++++++++-------------
>>> 1 file changed, 16 insertions(+), 14 deletions(-)
>> So are we good with this? Bob? Zhu?
> I have already checked this commit. And I have found 2 problems with
> this commit.
> This commit changes many MAXs.
> And now rxe is not stable enough. Not sure this commit will cause the
> new problems.
>
> Zhu Yanjun
Hi Zhu,
A generic statement without any technical data does not help. As far as
I am aware, currently there are no outstanding issues. If there are,
please provide data that clearly shows that the issue is caused by this
patch.
Thanks you.
Shoaib
>>> - RXE_MAX_MR_INDEX = 0x00010000,
>>> + RXE_MAX_MR_INDEX = DEFAULT_MAX_VALUE,
>>> + RXE_MAX_MR = DEFAULT_MAX_VALUE - RXE_MIN_MR_INDEX,
>> Bob, were you saying this was what needed to be bigger to pass
>> blktests??
>>
>> Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs
2021-09-28 4:38 ` Shoaib Rao
@ 2021-09-28 6:55 ` Zhu Yanjun
2021-09-28 9:41 ` Shoaib Rao
0 siblings, 1 reply; 11+ messages in thread
From: Zhu Yanjun @ 2021-09-28 6:55 UTC (permalink / raw)
To: Shoaib Rao; +Cc: Jason Gunthorpe, Bob Pearson, RDMA mailing list
On Tue, Sep 28, 2021 at 12:38 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
>
>
> On 9/27/21 6:46 PM, Zhu Yanjun wrote:
> > On Tue, Sep 28, 2021 at 3:19 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >> On Tue, Sep 14, 2021 at 06:12:20PM -0700, Rao Shoaib wrote:
> >>> In our internal testing we have found that
> >>> default maximum values are too small.
> >>> Ideally there should be no limits, but since
> >>> maximum values are reported via ibv_query_device,
> >>> we have to return some value. So, the default
> >>> maximums have been changed to large values.
> >>>
> >>> Signed-off-by: Rao Shoaib <Rao.Shoaib@oracle.com>
> >>> ---
> >>>
> >>> Resubmitting the patch after applying Bob's latest patches and testing
> >>> using via rping.
> >>>
> >>> drivers/infiniband/sw/rxe/rxe_param.h | 30 ++++++++++++++-------------
> >>> 1 file changed, 16 insertions(+), 14 deletions(-)
> >> So are we good with this? Bob? Zhu?
> > I have already checked this commit. And I have found 2 problems with
> > this commit.
> > This commit changes many MAXs.
> > And now rxe is not stable enough. Not sure this commit will cause the
> > new problems.
> >
> > Zhu Yanjun
>
> Hi Zhu,
>
> A generic statement without any technical data does not help. As far as
> I am aware, currently there are no outstanding issues. If there are,
> please provide data that clearly shows that the issue is caused by this
> patch.
Hi, Shoaib
With this commit, I found 2 problems.
This is why I suspect that this commit will introduce risks.
Before a commit is sent to the upstream, please make full tests with it.
Zhu Yanjun
>
> Thanks you.
>
> Shoaib
>
> >>> - RXE_MAX_MR_INDEX = 0x00010000,
> >>> + RXE_MAX_MR_INDEX = DEFAULT_MAX_VALUE,
> >>> + RXE_MAX_MR = DEFAULT_MAX_VALUE - RXE_MIN_MR_INDEX,
> >> Bob, were you saying this was what needed to be bigger to pass
> >> blktests??
> >>
> >> Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs
2021-09-28 6:55 ` Zhu Yanjun
@ 2021-09-28 9:41 ` Shoaib Rao
2021-09-28 9:58 ` Zhu Yanjun
0 siblings, 1 reply; 11+ messages in thread
From: Shoaib Rao @ 2021-09-28 9:41 UTC (permalink / raw)
To: Zhu Yanjun; +Cc: Jason Gunthorpe, Bob Pearson, RDMA mailing list
On 9/27/21 11:55 PM, Zhu Yanjun wrote:
> On Tue, Sep 28, 2021 at 12:38 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
>>
>> On 9/27/21 6:46 PM, Zhu Yanjun wrote:
>>> On Tue, Sep 28, 2021 at 3:19 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>>>> On Tue, Sep 14, 2021 at 06:12:20PM -0700, Rao Shoaib wrote:
>>>>> In our internal testing we have found that
>>>>> default maximum values are too small.
>>>>> Ideally there should be no limits, but since
>>>>> maximum values are reported via ibv_query_device,
>>>>> we have to return some value. So, the default
>>>>> maximums have been changed to large values.
>>>>>
>>>>> Signed-off-by: Rao Shoaib <Rao.Shoaib@oracle.com>
>>>>> ---
>>>>>
>>>>> Resubmitting the patch after applying Bob's latest patches and testing
>>>>> using via rping.
>>>>>
>>>>> drivers/infiniband/sw/rxe/rxe_param.h | 30 ++++++++++++++-------------
>>>>> 1 file changed, 16 insertions(+), 14 deletions(-)
>>>> So are we good with this? Bob? Zhu?
>>> I have already checked this commit. And I have found 2 problems with
>>> this commit.
>>> This commit changes many MAXs.
>>> And now rxe is not stable enough. Not sure this commit will cause the
>>> new problems.
>>>
>>> Zhu Yanjun
>> Hi Zhu,
>>
>> A generic statement without any technical data does not help. As far as
>> I am aware, currently there are no outstanding issues. If there are,
>> please provide data that clearly shows that the issue is caused by this
>> patch.
> Hi, Shoaib
>
> With this commit, I found 2 problems.
> This is why I suspect that this commit will introduce risks.
Hi Zhu,
I did full testing before I sent the patch, that is how I found that
rping did not work. What are the issues that you found? How to I
reproduce those issues?
Shoaib
>
> Before a commit is sent to the upstream, please make full tests with it.
>
> Zhu Yanjun
>
>> Thanks you.
>>
>> Shoaib
>>
>>>>> - RXE_MAX_MR_INDEX = 0x00010000,
>>>>> + RXE_MAX_MR_INDEX = DEFAULT_MAX_VALUE,
>>>>> + RXE_MAX_MR = DEFAULT_MAX_VALUE - RXE_MIN_MR_INDEX,
>>>> Bob, were you saying this was what needed to be bigger to pass
>>>> blktests??
>>>>
>>>> Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs
2021-09-28 9:41 ` Shoaib Rao
@ 2021-09-28 9:58 ` Zhu Yanjun
2021-09-29 4:01 ` Shoaib Rao
0 siblings, 1 reply; 11+ messages in thread
From: Zhu Yanjun @ 2021-09-28 9:58 UTC (permalink / raw)
To: Shoaib Rao; +Cc: Jason Gunthorpe, Bob Pearson, RDMA mailing list
On Tue, Sep 28, 2021 at 5:41 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
>
>
> On 9/27/21 11:55 PM, Zhu Yanjun wrote:
> > On Tue, Sep 28, 2021 at 12:38 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
> >>
> >> On 9/27/21 6:46 PM, Zhu Yanjun wrote:
> >>> On Tue, Sep 28, 2021 at 3:19 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >>>> On Tue, Sep 14, 2021 at 06:12:20PM -0700, Rao Shoaib wrote:
> >>>>> In our internal testing we have found that
> >>>>> default maximum values are too small.
> >>>>> Ideally there should be no limits, but since
> >>>>> maximum values are reported via ibv_query_device,
> >>>>> we have to return some value. So, the default
> >>>>> maximums have been changed to large values.
> >>>>>
> >>>>> Signed-off-by: Rao Shoaib <Rao.Shoaib@oracle.com>
> >>>>> ---
> >>>>>
> >>>>> Resubmitting the patch after applying Bob's latest patches and testing
> >>>>> using via rping.
> >>>>>
> >>>>> drivers/infiniband/sw/rxe/rxe_param.h | 30 ++++++++++++++-------------
> >>>>> 1 file changed, 16 insertions(+), 14 deletions(-)
> >>>> So are we good with this? Bob? Zhu?
> >>> I have already checked this commit. And I have found 2 problems with
> >>> this commit.
> >>> This commit changes many MAXs.
> >>> And now rxe is not stable enough. Not sure this commit will cause the
> >>> new problems.
> >>>
> >>> Zhu Yanjun
> >> Hi Zhu,
> >>
> >> A generic statement without any technical data does not help. As far as
> >> I am aware, currently there are no outstanding issues. If there are,
> >> please provide data that clearly shows that the issue is caused by this
> >> patch.
> > Hi, Shoaib
> >
> > With this commit, I found 2 problems.
> > This is why I suspect that this commit will introduce risks.
>
> Hi Zhu,
>
> I did full testing before I sent the patch, that is how I found that
> rping did not work. What are the issues that you found? How to I
> reproduce those issues?
Sorry. What tests do you make?
Do you make tests with the followings:
1. your commit + latest kernel <------rping------- > 5.10 stable kernel
2. your commit + latest kernel < ------rping------- > 5.11 stable kernel
3. your commit + latest kernel < ------rping------- > 5.12 stable kernel
4. your commit + latest kernel < ------rping------- > 5.13 stable kernel
5. your commit + latest kernel < ------rping------- > 5.14 stable kernel
6. rdma-core tests with your commit + latest kernel
Zhu Yanjun
>
> Shoaib
>
> >
> > Before a commit is sent to the upstream, please make full tests with it.
> >
> > Zhu Yanjun
> >
> >> Thanks you.
> >>
> >> Shoaib
> >>
> >>>>> - RXE_MAX_MR_INDEX = 0x00010000,
> >>>>> + RXE_MAX_MR_INDEX = DEFAULT_MAX_VALUE,
> >>>>> + RXE_MAX_MR = DEFAULT_MAX_VALUE - RXE_MIN_MR_INDEX,
> >>>> Bob, were you saying this was what needed to be bigger to pass
> >>>> blktests??
> >>>>
> >>>> Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs
2021-09-28 9:58 ` Zhu Yanjun
@ 2021-09-29 4:01 ` Shoaib Rao
2021-09-29 5:23 ` Zhu Yanjun
0 siblings, 1 reply; 11+ messages in thread
From: Shoaib Rao @ 2021-09-29 4:01 UTC (permalink / raw)
To: Zhu Yanjun; +Cc: Jason Gunthorpe, Bob Pearson, RDMA mailing list
On 9/28/21 2:58 AM, Zhu Yanjun wrote:
> On Tue, Sep 28, 2021 at 5:41 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
>>
>> On 9/27/21 11:55 PM, Zhu Yanjun wrote:
>>> On Tue, Sep 28, 2021 at 12:38 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
>>>> On 9/27/21 6:46 PM, Zhu Yanjun wrote:
>>>>> On Tue, Sep 28, 2021 at 3:19 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>>>>>> On Tue, Sep 14, 2021 at 06:12:20PM -0700, Rao Shoaib wrote:
>>>>>>> In our internal testing we have found that
>>>>>>> default maximum values are too small.
>>>>>>> Ideally there should be no limits, but since
>>>>>>> maximum values are reported via ibv_query_device,
>>>>>>> we have to return some value. So, the default
>>>>>>> maximums have been changed to large values.
>>>>>>>
>>>>>>> Signed-off-by: Rao Shoaib <Rao.Shoaib@oracle.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Resubmitting the patch after applying Bob's latest patches and testing
>>>>>>> using via rping.
>>>>>>>
>>>>>>> drivers/infiniband/sw/rxe/rxe_param.h | 30 ++++++++++++++-------------
>>>>>>> 1 file changed, 16 insertions(+), 14 deletions(-)
>>>>>> So are we good with this? Bob? Zhu?
>>>>> I have already checked this commit. And I have found 2 problems with
>>>>> this commit.
>>>>> This commit changes many MAXs.
>>>>> And now rxe is not stable enough. Not sure this commit will cause the
>>>>> new problems.
>>>>>
>>>>> Zhu Yanjun
>>>> Hi Zhu,
>>>>
>>>> A generic statement without any technical data does not help. As far as
>>>> I am aware, currently there are no outstanding issues. If there are,
>>>> please provide data that clearly shows that the issue is caused by this
>>>> patch.
>>> Hi, Shoaib
>>>
>>> With this commit, I found 2 problems.
>>> This is why I suspect that this commit will introduce risks.
>> Hi Zhu,
>>
>> I did full testing before I sent the patch, that is how I found that
>> rping did not work. What are the issues that you found? How to I
>> reproduce those issues?
> Sorry. What tests do you make?
>
> Do you make tests with the followings:
>
> 1. your commit + latest kernel <------rping------- > 5.10 stable kernel
> 2. your commit + latest kernel < ------rping------- > 5.11 stable kernel
> 3. your commit + latest kernel < ------rping------- > 5.12 stable kernel
> 4. your commit + latest kernel < ------rping------- > 5.13 stable kernel
> 5. your commit + latest kernel < ------rping------- > 5.14 stable kernel
> 6. rdma-core tests with your commit + latest kernel
>
> Zhu Yanjun
Hi Zhu,
With all due respect, I am a little surprised by the special treatment
being given to this rather simple patch. Normally comments to a patch
are submitted with technical data to back them up. In this case none
have been provided. If we are going by gut feelings, then there are more
involved patches that are being accepted without any tests as listed above.
My initial patch increased the value of 3 variables but the community
suggested that values of other variables should be increased as well.
The discussion happened on this mailing list and no objections were raised.
The two issues that you mention were mechanical issues, (one reported by
you and one by the kernel bot) that have been fixed. They had nothing to
do with the values being increased. As I have said I have tested the
patch several times, most recently about a week or so ago with Bob's
latest change.
I am not keen on changing the values of any other parameters, but the 3
that were contained in my original patch. The link to those patches is
https://www.spinics.net/lists/linux-rdma/msg103570.html
Please let me know if those are acceptable. They have been tested
*extensively* running several different kinds of Oracle DB workloads.
Regards,
Shoaib
>
>> Shoaib
>>
>>> Before a commit is sent to the upstream, please make full tests with it.
>>>
>>> Zhu Yanjun
>>>
>>>> Thanks you.
>>>>
>>>> Shoaib
>>>>
>>>>>>> - RXE_MAX_MR_INDEX = 0x00010000,
>>>>>>> + RXE_MAX_MR_INDEX = DEFAULT_MAX_VALUE,
>>>>>>> + RXE_MAX_MR = DEFAULT_MAX_VALUE - RXE_MIN_MR_INDEX,
>>>>>> Bob, were you saying this was what needed to be bigger to pass
>>>>>> blktests??
>>>>>>
>>>>>> Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs
2021-09-29 4:01 ` Shoaib Rao
@ 2021-09-29 5:23 ` Zhu Yanjun
2021-09-29 5:53 ` Shoaib Rao
0 siblings, 1 reply; 11+ messages in thread
From: Zhu Yanjun @ 2021-09-29 5:23 UTC (permalink / raw)
To: Shoaib Rao; +Cc: Jason Gunthorpe, Bob Pearson, RDMA mailing list
On Wed, Sep 29, 2021 at 12:02 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
>
>
> On 9/28/21 2:58 AM, Zhu Yanjun wrote:
> > On Tue, Sep 28, 2021 at 5:41 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
> >>
> >> On 9/27/21 11:55 PM, Zhu Yanjun wrote:
> >>> On Tue, Sep 28, 2021 at 12:38 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
> >>>> On 9/27/21 6:46 PM, Zhu Yanjun wrote:
> >>>>> On Tue, Sep 28, 2021 at 3:19 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >>>>>> On Tue, Sep 14, 2021 at 06:12:20PM -0700, Rao Shoaib wrote:
> >>>>>>> In our internal testing we have found that
> >>>>>>> default maximum values are too small.
> >>>>>>> Ideally there should be no limits, but since
> >>>>>>> maximum values are reported via ibv_query_device,
> >>>>>>> we have to return some value. So, the default
> >>>>>>> maximums have been changed to large values.
> >>>>>>>
> >>>>>>> Signed-off-by: Rao Shoaib <Rao.Shoaib@oracle.com>
> >>>>>>> ---
> >>>>>>>
> >>>>>>> Resubmitting the patch after applying Bob's latest patches and testing
> >>>>>>> using via rping.
> >>>>>>>
> >>>>>>> drivers/infiniband/sw/rxe/rxe_param.h | 30 ++++++++++++++-------------
> >>>>>>> 1 file changed, 16 insertions(+), 14 deletions(-)
> >>>>>> So are we good with this? Bob? Zhu?
> >>>>> I have already checked this commit. And I have found 2 problems with
> >>>>> this commit.
> >>>>> This commit changes many MAXs.
> >>>>> And now rxe is not stable enough. Not sure this commit will cause the
> >>>>> new problems.
> >>>>>
> >>>>> Zhu Yanjun
> >>>> Hi Zhu,
> >>>>
> >>>> A generic statement without any technical data does not help. As far as
> >>>> I am aware, currently there are no outstanding issues. If there are,
> >>>> please provide data that clearly shows that the issue is caused by this
> >>>> patch.
> >>> Hi, Shoaib
> >>>
> >>> With this commit, I found 2 problems.
> >>> This is why I suspect that this commit will introduce risks.
> >> Hi Zhu,
> >>
> >> I did full testing before I sent the patch, that is how I found that
> >> rping did not work. What are the issues that you found? How to I
> >> reproduce those issues?
> > Sorry. What tests do you make?
> >
> > Do you make tests with the followings:
> >
> > 1. your commit + latest kernel <------rping------- > 5.10 stable kernel
> > 2. your commit + latest kernel < ------rping------- > 5.11 stable kernel
> > 3. your commit + latest kernel < ------rping------- > 5.12 stable kernel
> > 4. your commit + latest kernel < ------rping------- > 5.13 stable kernel
> > 5. your commit + latest kernel < ------rping------- > 5.14 stable kernel
> > 6. rdma-core tests with your commit + latest kernel
> >
> > Zhu Yanjun
>
> Hi Zhu,
>
> With all due respect, I am a little surprised by the special treatment
> being given to this rather simple patch. Normally comments to a patch
> are submitted with technical data to back them up. In this case none
> have been provided. If we are going by gut feelings, then there are more
> involved patches that are being accepted without any tests as listed above.
All the commits that I reviewed will pass the mentioned tests. But your commit
failed to pass the tests at least 2 times. So I suggest that you make the above
tests in your local hosts. This will save us time.
I do not treat your commit specially.
>
> My initial patch increased the value of 3 variables but the community
> suggested that values of other variables should be increased as well.
> The discussion happened on this mailing list and no objections were raised.
>
> The two issues that you mention were mechanical issues, (one reported by
> you and one by the kernel bot) that have been fixed. They had nothing to
> do with the values being increased. As I have said I have tested the
> patch several times, most recently about a week or so ago with Bob's
> latest change.
I do not know how you tested your patch in your host. If your tests include
backward compatibility and rdma-core tests that I mentioned in the above mail,
I am fine with this commit.
Zhu Yanjun
>
> I am not keen on changing the values of any other parameters, but the 3
> that were contained in my original patch. The link to those patches is
>
> https://www.spinics.net/lists/linux-rdma/msg103570.html
>
> Please let me know if those are acceptable. They have been tested
> *extensively* running several different kinds of Oracle DB workloads.
>
> Regards,
>
> Shoaib
>
> >
> >> Shoaib
> >>
> >>> Before a commit is sent to the upstream, please make full tests with it.
> >>>
> >>> Zhu Yanjun
> >>>
> >>>> Thanks you.
> >>>>
> >>>> Shoaib
> >>>>
> >>>>>>> - RXE_MAX_MR_INDEX = 0x00010000,
> >>>>>>> + RXE_MAX_MR_INDEX = DEFAULT_MAX_VALUE,
> >>>>>>> + RXE_MAX_MR = DEFAULT_MAX_VALUE - RXE_MIN_MR_INDEX,
> >>>>>> Bob, were you saying this was what needed to be bigger to pass
> >>>>>> blktests??
> >>>>>>
> >>>>>> Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs
2021-09-29 5:23 ` Zhu Yanjun
@ 2021-09-29 5:53 ` Shoaib Rao
0 siblings, 0 replies; 11+ messages in thread
From: Shoaib Rao @ 2021-09-29 5:53 UTC (permalink / raw)
To: Zhu Yanjun; +Cc: Jason Gunthorpe, Bob Pearson, RDMA mailing list
On 9/28/21 10:23 PM, Zhu Yanjun wrote:
> On Wed, Sep 29, 2021 at 12:02 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
>>
>> On 9/28/21 2:58 AM, Zhu Yanjun wrote:
>>> On Tue, Sep 28, 2021 at 5:41 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
>>>> On 9/27/21 11:55 PM, Zhu Yanjun wrote:
>>>>> On Tue, Sep 28, 2021 at 12:38 PM Shoaib Rao <rao.shoaib@oracle.com> wrote:
>>>>>> On 9/27/21 6:46 PM, Zhu Yanjun wrote:
>>>>>>> On Tue, Sep 28, 2021 at 3:19 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>>>>>>>> On Tue, Sep 14, 2021 at 06:12:20PM -0700, Rao Shoaib wrote:
>>>>>>>>> In our internal testing we have found that
>>>>>>>>> default maximum values are too small.
>>>>>>>>> Ideally there should be no limits, but since
>>>>>>>>> maximum values are reported via ibv_query_device,
>>>>>>>>> we have to return some value. So, the default
>>>>>>>>> maximums have been changed to large values.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Rao Shoaib <Rao.Shoaib@oracle.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Resubmitting the patch after applying Bob's latest patches and testing
>>>>>>>>> using via rping.
>>>>>>>>>
>>>>>>>>> drivers/infiniband/sw/rxe/rxe_param.h | 30 ++++++++++++++-------------
>>>>>>>>> 1 file changed, 16 insertions(+), 14 deletions(-)
>>>>>>>> So are we good with this? Bob? Zhu?
>>>>>>> I have already checked this commit. And I have found 2 problems with
>>>>>>> this commit.
>>>>>>> This commit changes many MAXs.
>>>>>>> And now rxe is not stable enough. Not sure this commit will cause the
>>>>>>> new problems.
>>>>>>>
>>>>>>> Zhu Yanjun
>>>>>> Hi Zhu,
>>>>>>
>>>>>> A generic statement without any technical data does not help. As far as
>>>>>> I am aware, currently there are no outstanding issues. If there are,
>>>>>> please provide data that clearly shows that the issue is caused by this
>>>>>> patch.
>>>>> Hi, Shoaib
>>>>>
>>>>> With this commit, I found 2 problems.
>>>>> This is why I suspect that this commit will introduce risks.
>>>> Hi Zhu,
>>>>
>>>> I did full testing before I sent the patch, that is how I found that
>>>> rping did not work. What are the issues that you found? How to I
>>>> reproduce those issues?
>>> Sorry. What tests do you make?
>>>
>>> Do you make tests with the followings:
>>>
>>> 1. your commit + latest kernel <------rping------- > 5.10 stable kernel
>>> 2. your commit + latest kernel < ------rping------- > 5.11 stable kernel
>>> 3. your commit + latest kernel < ------rping------- > 5.12 stable kernel
>>> 4. your commit + latest kernel < ------rping------- > 5.13 stable kernel
>>> 5. your commit + latest kernel < ------rping------- > 5.14 stable kernel
>>> 6. rdma-core tests with your commit + latest kernel
>>>
>>> Zhu Yanjun
>> Hi Zhu,
>>
>> With all due respect, I am a little surprised by the special treatment
>> being given to this rather simple patch. Normally comments to a patch
>> are submitted with technical data to back them up. In this case none
>> have been provided. If we are going by gut feelings, then there are more
>> involved patches that are being accepted without any tests as listed above.
> All the commits that I reviewed will pass the mentioned tests. But your commit
> failed to pass the tests at least 2 times. So I suggest that you make the above
> tests in your local hosts. This will save us time.
> I do not treat your commit specially.
>
>> My initial patch increased the value of 3 variables but the community
>> suggested that values of other variables should be increased as well.
>> The discussion happened on this mailing list and no objections were raised.
>>
>> The two issues that you mention were mechanical issues, (one reported by
>> you and one by the kernel bot) that have been fixed. They had nothing to
>> do with the values being increased. As I have said I have tested the
>> patch several times, most recently about a week or so ago with Bob's
>> latest change.
> I do not know how you tested your patch in your host. If your tests include
> backward compatibility and rdma-core tests that I mentioned in the above mail,
> I am fine with this commit.
>
> Zhu Yanjun
Thanks. I did test backward compatibility. If any issues are found in
the future I will fix them.
Regards,
Shoaib
>
>> I am not keen on changing the values of any other parameters, but the 3
>> that were contained in my original patch. The link to those patches is
>>
>> https://urldefense.com/v3/__https://www.spinics.net/lists/linux-rdma/msg103570.html__;!!ACWV5N9M2RV99hQ!c_05bC74v_nDObwKFnI2y3b5EuwvlSIq8hKf_4iXStmPVUk6qxw1_Jii_AC7oiui$
>>
>> Please let me know if those are acceptable. They have been tested
>> *extensively* running several different kinds of Oracle DB workloads.
>>
>> Regards,
>>
>> Shoaib
>>
>>>> Shoaib
>>>>
>>>>> Before a commit is sent to the upstream, please make full tests with it.
>>>>>
>>>>> Zhu Yanjun
>>>>>
>>>>>> Thanks you.
>>>>>>
>>>>>> Shoaib
>>>>>>
>>>>>>>>> - RXE_MAX_MR_INDEX = 0x00010000,
>>>>>>>>> + RXE_MAX_MR_INDEX = DEFAULT_MAX_VALUE,
>>>>>>>>> + RXE_MAX_MR = DEFAULT_MAX_VALUE - RXE_MIN_MR_INDEX,
>>>>>>>> Bob, were you saying this was what needed to be bigger to pass
>>>>>>>> blktests??
>>>>>>>>
>>>>>>>> Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs
2021-09-15 1:12 [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs Rao Shoaib
2021-09-27 19:19 ` Jason Gunthorpe
@ 2021-10-04 18:57 ` Jason Gunthorpe
1 sibling, 0 replies; 11+ messages in thread
From: Jason Gunthorpe @ 2021-10-04 18:57 UTC (permalink / raw)
To: Rao Shoaib; +Cc: linux-rdma
On Tue, Sep 14, 2021 at 06:12:20PM -0700, Rao Shoaib wrote:
> In our internal testing we have found that
> default maximum values are too small.
> Ideally there should be no limits, but since
> maximum values are reported via ibv_query_device,
> we have to return some value. So, the default
> maximums have been changed to large values.
>
> Signed-off-by: Rao Shoaib <Rao.Shoaib@oracle.com>
> ---
>
> Resubmitting the patch after applying Bob's latest patches and testing
> using via rping.
Since nobody points to something wrong with this and Bob says it fixes
a blktests problem..
Applied to for-next, thanks
Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-10-04 18:58 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-15 1:12 [PATCH v1] RDMA/rxe: Bump up default maximum values used via uverbs Rao Shoaib
2021-09-27 19:19 ` Jason Gunthorpe
2021-09-28 1:46 ` Zhu Yanjun
2021-09-28 4:38 ` Shoaib Rao
2021-09-28 6:55 ` Zhu Yanjun
2021-09-28 9:41 ` Shoaib Rao
2021-09-28 9:58 ` Zhu Yanjun
2021-09-29 4:01 ` Shoaib Rao
2021-09-29 5:23 ` Zhu Yanjun
2021-09-29 5:53 ` Shoaib Rao
2021-10-04 18:57 ` Jason Gunthorpe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).