reduce iSERT Max IO size

reduce iSERT Max IO size

[Krishnamraju Eraparaju]

Hi,

Please reduce the Max IO size to 1MiB(256 pages), at iSER Target.
The PBL memory consumption has increased significantly after increasing
the Max IO size to 16MiB(with commit:317000b926b07c).
Due to the large MR pool, the max no.of iSER connections(On one variant
of Chelsio cards) came down to 9, before it was 250.
NVMe-RDMA target also uses 1MiB max IO size.

Thanks,
Krishnam Raju.

Re: reduce iSERT Max IO size

[Sagi Grimberg]

> Hi,
> 
> Please reduce the Max IO size to 1MiB(256 pages), at iSER Target.
> The PBL memory consumption has increased significantly after increasing
> the Max IO size to 16MiB(with commit:317000b926b07c).
> Due to the large MR pool, the max no.of iSER connections(On one variant
> of Chelsio cards) came down to 9, before it was 250.
> NVMe-RDMA target also uses 1MiB max IO size.

Max, remind me what was the point to support 16M? Did this resolve
an issue?

Re: reduce iSERT Max IO size

[Krishnamraju Eraparaju]

Hi Sagi & Max,

Any update on this?
Please change the max IO size to 1MiB(256 pages). 


Thanks,
Krishnam Raju.
On Wednesday, September 09/23/20, 2020 at 01:57:47 -0700, Sagi Grimberg wrote:
> 
> >Hi,
> >
> >Please reduce the Max IO size to 1MiB(256 pages), at iSER Target.
> >The PBL memory consumption has increased significantly after increasing
> >the Max IO size to 16MiB(with commit:317000b926b07c).
> >Due to the large MR pool, the max no.of iSER connections(On one variant
> >of Chelsio cards) came down to 9, before it was 250.
> >NVMe-RDMA target also uses 1MiB max IO size.
> 
> Max, remind me what was the point to support 16M? Did this resolve
> an issue?

Re: reduce iSERT Max IO size

[Sagi Grimberg]

> Hi Sagi & Max,
> 
> Any update on this?
> Please change the max IO size to 1MiB(256 pages).

I think that the reason why this was changed to handle the worst case
was in case there are different capabilities on the initiator and the
target with respect to number of pages per MR. There is no handshake
that aligns expectations.

If we revert that it would restore the issue that you reported in the
first place:

--
IB/isert: allocate RW ctxs according to max IO size

Current iSER target code allocates MR pool budget based on queue size.
Since there is no handshake between iSER initiator and target on max IO
size, we'll set the iSER target to support upto 16MiB IO operations and
allocate the correct number of RDMA ctxs according to the factor of MR's
per IO operation. This would guaranty sufficient size of the MR pool for
the required IO queue depth and IO size.

Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
Tested-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
--

> 
> 
> Thanks,
> Krishnam Raju.
> On Wednesday, September 09/23/20, 2020 at 01:57:47 -0700, Sagi Grimberg wrote:
>>
>>> Hi,
>>>
>>> Please reduce the Max IO size to 1MiB(256 pages), at iSER Target.
>>> The PBL memory consumption has increased significantly after increasing
>>> the Max IO size to 16MiB(with commit:317000b926b07c).
>>> Due to the large MR pool, the max no.of iSER connections(On one variant
>>> of Chelsio cards) came down to 9, before it was 250.
>>> NVMe-RDMA target also uses 1MiB max IO size.
>>
>> Max, remind me what was the point to support 16M? Did this resolve
>> an issue?

Re: reduce iSERT Max IO size

[Krishnamraju Eraparaju]

On Friday, October 10/02/20, 2020 at 13:29:30 -0700, Sagi Grimberg wrote:
> 
> >Hi Sagi & Max,
> >
> >Any update on this?
> >Please change the max IO size to 1MiB(256 pages).
> 
> I think that the reason why this was changed to handle the worst case
> was in case there are different capabilities on the initiator and the
> target with respect to number of pages per MR. There is no handshake
> that aligns expectations.
But, the max pages per MR supported by most adapters is around 256 pages
only.
And I think only those iSER initiators, whose max pages per MR is 4096,
could send 16MiB sized IOs, am I correct?

> 
> If we revert that it would restore the issue that you reported in the
> first place:
> 
> --
> IB/isert: allocate RW ctxs according to max IO size
I don't see the reported issue after reducing the IO size to 256
pages(keeping all other changes of this patch intact).
That is, "attr.cap.max_rdma_ctxs" is now getting filled properly with
"rdma_rw_mr_factor()" related changes, I think.

Before this change "attr.cap.max_rdma_ctxs" was hardcoded with
128(ISCSI_DEF_XMIT_CMDS_MAX) pages, which is very low for single target
and muli-luns case.

So reverting only ISCSI_ISER_MAX_SG_TABLESIZE macro to 256 doesn't cause the
reported issue.

Thanks,
Krishnam Raju.
> 
> Current iSER target code allocates MR pool budget based on queue size.
> Since there is no handshake between iSER initiator and target on max IO
> size, we'll set the iSER target to support upto 16MiB IO operations and
> allocate the correct number of RDMA ctxs according to the factor of MR's
> per IO operation. This would guaranty sufficient size of the MR pool for
> the required IO queue depth and IO size.
> 
> Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
> Tested-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> --
> 
> >
> >
> >Thanks,
> >Krishnam Raju.
> >On Wednesday, September 09/23/20, 2020 at 01:57:47 -0700, Sagi Grimberg wrote:
> >>
> >>>Hi,
> >>>
> >>>Please reduce the Max IO size to 1MiB(256 pages), at iSER Target.
> >>>The PBL memory consumption has increased significantly after increasing
> >>>the Max IO size to 16MiB(with commit:317000b926b07c).
> >>>Due to the large MR pool, the max no.of iSER connections(On one variant
> >>>of Chelsio cards) came down to 9, before it was 250.
> >>>NVMe-RDMA target also uses 1MiB max IO size.
> >>
> >>Max, remind me what was the point to support 16M? Did this resolve
> >>an issue?

Re: reduce iSERT Max IO size

[Max Gurtovoy]

On 10/3/2020 6:36 AM, Krishnamraju Eraparaju wrote:
> On Friday, October 10/02/20, 2020 at 13:29:30 -0700, Sagi Grimberg wrote:
>>> Hi Sagi & Max,
>>>
>>> Any update on this?
>>> Please change the max IO size to 1MiB(256 pages).
>> I think that the reason why this was changed to handle the worst case
>> was in case there are different capabilities on the initiator and the
>> target with respect to number of pages per MR. There is no handshake
>> that aligns expectations.
> But, the max pages per MR supported by most adapters is around 256 pages
> only.

Not sure you're right.

> And I think only those iSER initiators, whose max pages per MR is 4096,
> could send 16MiB sized IOs, am I correct?

If the initiator can send 16MiB, we must make sure the target is capable 
to receive it.

>
>> If we revert that it would restore the issue that you reported in the
>> first place:
>>
>> --
>> IB/isert: allocate RW ctxs according to max IO size
> I don't see the reported issue after reducing the IO size to 256
> pages(keeping all other changes of this patch intact).
> That is, "attr.cap.max_rdma_ctxs" is now getting filled properly with
> "rdma_rw_mr_factor()" related changes, I think.
>
> Before this change "attr.cap.max_rdma_ctxs" was hardcoded with
> 128(ISCSI_DEF_XMIT_CMDS_MAX) pages, which is very low for single target
> and muli-luns case.
>
> So reverting only ISCSI_ISER_MAX_SG_TABLESIZE macro to 256 doesn't cause the
> reported issue.
>
> Thanks,
> Krishnam Raju.
>> Current iSER target code allocates MR pool budget based on queue size.
>> Since there is no handshake between iSER initiator and target on max IO
>> size, we'll set the iSER target to support upto 16MiB IO operations and
>> allocate the correct number of RDMA ctxs according to the factor of MR's
>> per IO operation. This would guaranty sufficient size of the MR pool for
>> the required IO queue depth and IO size.
>>
>> Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
>> Tested-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
>> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
>> --
>>
>>>
>>> Thanks,
>>> Krishnam Raju.
>>> On Wednesday, September 09/23/20, 2020 at 01:57:47 -0700, Sagi Grimberg wrote:
>>>>> Hi,
>>>>>
>>>>> Please reduce the Max IO size to 1MiB(256 pages), at iSER Target.
>>>>> The PBL memory consumption has increased significantly after increasing
>>>>> the Max IO size to 16MiB(with commit:317000b926b07c).
>>>>> Due to the large MR pool, the max no.of iSER connections(On one variant
>>>>> of Chelsio cards) came down to 9, before it was 250.
>>>>> NVMe-RDMA target also uses 1MiB max IO size.
>>>> Max, remind me what was the point to support 16M? Did this resolve
>>>> an issue?

Re: reduce iSERT Max IO size

[Max Gurtovoy]

On 10/3/2020 6:36 AM, Krishnamraju Eraparaju wrote:
> On Friday, October 10/02/20, 2020 at 13:29:30 -0700, Sagi Grimberg wrote:
>>> Hi Sagi & Max,
>>>
>>> Any update on this?
>>> Please change the max IO size to 1MiB(256 pages).
>> I think that the reason why this was changed to handle the worst case
>> was in case there are different capabilities on the initiator and the
>> target with respect to number of pages per MR. There is no handshake
>> that aligns expectations.
> But, the max pages per MR supported by most adapters is around 256 pages
> only.
> And I think only those iSER initiators, whose max pages per MR is 4096,
> could send 16MiB sized IOs, am I correct?

If the initiator can send 16MiB, we must make sure the target is capable 
to receive it.

>
>> If we revert that it would restore the issue that you reported in the
>> first place:
>>
>> --
>> IB/isert: allocate RW ctxs according to max IO size
> I don't see the reported issue after reducing the IO size to 256
> pages(keeping all other changes of this patch intact).
> That is, "attr.cap.max_rdma_ctxs" is now getting filled properly with
> "rdma_rw_mr_factor()" related changes, I think.
>
> Before this change "attr.cap.max_rdma_ctxs" was hardcoded with
> 128(ISCSI_DEF_XMIT_CMDS_MAX) pages, which is very low for single target
> and muli-luns case.
>
> So reverting only ISCSI_ISER_MAX_SG_TABLESIZE macro to 256 doesn't cause the
> reported issue.
>
> Thanks,
> Krishnam Raju.
>> Current iSER target code allocates MR pool budget based on queue size.
>> Since there is no handshake between iSER initiator and target on max IO
>> size, we'll set the iSER target to support upto 16MiB IO operations and
>> allocate the correct number of RDMA ctxs according to the factor of MR's
>> per IO operation. This would guaranty sufficient size of the MR pool for
>> the required IO queue depth and IO size.
>>
>> Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
>> Tested-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
>> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
>> --
>>
>>>
>>> Thanks,
>>> Krishnam Raju.
>>> On Wednesday, September 09/23/20, 2020 at 01:57:47 -0700, Sagi Grimberg wrote:
>>>>> Hi,
>>>>>
>>>>> Please reduce the Max IO size to 1MiB(256 pages), at iSER Target.
>>>>> The PBL memory consumption has increased significantly after increasing
>>>>> the Max IO size to 16MiB(with commit:317000b926b07c).
>>>>> Due to the large MR pool, the max no.of iSER connections(On one variant
>>>>> of Chelsio cards) came down to 9, before it was 250.
>>>>> NVMe-RDMA target also uses 1MiB max IO size.
>>>> Max, remind me what was the point to support 16M? Did this resolve
>>>> an issue?

Re: reduce iSERT Max IO size

[Krishnamraju Eraparaju]

On Sunday, October 10/04/20, 2020 at 00:45:26 +0300, Max Gurtovoy wrote:
> 
> On 10/3/2020 6:36 AM, Krishnamraju Eraparaju wrote:
> >On Friday, October 10/02/20, 2020 at 13:29:30 -0700, Sagi Grimberg wrote:
> >>>Hi Sagi & Max,
> >>>
> >>>Any update on this?
> >>>Please change the max IO size to 1MiB(256 pages).
> >>I think that the reason why this was changed to handle the worst case
> >>was in case there are different capabilities on the initiator and the
> >>target with respect to number of pages per MR. There is no handshake
> >>that aligns expectations.
> >But, the max pages per MR supported by most adapters is around 256 pages
> >only.
> >And I think only those iSER initiators, whose max pages per MR is 4096,
> >could send 16MiB sized IOs, am I correct?
> 
> If the initiator can send 16MiB, we must make sure the target is
> capable to receive it.
I think max IO size, at iSER initiator, depends on
"max_fast_reg_page_list_len".
currently, below are the supported "max_fast_reg_page_list_len" of
various iwarp drivers:

iw_cxgb4: 128 pages
Softiwarp: 256 pages
i40iw: 512 pages
qedr: couldn't find.

For iwarp case, if 512 is the max pages supported by all iwarp drivers,
then provisioning a gigantic MR pool at target(to accommodate never used
16MiB IO) wouldn't be a overkill?
> 
> >
> >>If we revert that it would restore the issue that you reported in the
> >>first place:
> >>
> >>--
> >>IB/isert: allocate RW ctxs according to max IO size
> >I don't see the reported issue after reducing the IO size to 256
> >pages(keeping all other changes of this patch intact).
> >That is, "attr.cap.max_rdma_ctxs" is now getting filled properly with
> >"rdma_rw_mr_factor()" related changes, I think.
> >
> >Before this change "attr.cap.max_rdma_ctxs" was hardcoded with
> >128(ISCSI_DEF_XMIT_CMDS_MAX) pages, which is very low for single target
> >and muli-luns case.
> >
> >So reverting only ISCSI_ISER_MAX_SG_TABLESIZE macro to 256 doesn't cause the
> >reported issue.
> >
> >Thanks,
> >Krishnam Raju.
> >>Current iSER target code allocates MR pool budget based on queue size.
> >>Since there is no handshake between iSER initiator and target on max IO
> >>size, we'll set the iSER target to support upto 16MiB IO operations and
> >>allocate the correct number of RDMA ctxs according to the factor of MR's
> >>per IO operation. This would guaranty sufficient size of the MR pool for
> >>the required IO queue depth and IO size.
> >>
> >>Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
> >>Tested-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
> >>Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> >>--
> >>
> >>>
> >>>Thanks,
> >>>Krishnam Raju.
> >>>On Wednesday, September 09/23/20, 2020 at 01:57:47 -0700, Sagi Grimberg wrote:
> >>>>>Hi,
> >>>>>
> >>>>>Please reduce the Max IO size to 1MiB(256 pages), at iSER Target.
> >>>>>The PBL memory consumption has increased significantly after increasing
> >>>>>the Max IO size to 16MiB(with commit:317000b926b07c).
> >>>>>Due to the large MR pool, the max no.of iSER connections(On one variant
> >>>>>of Chelsio cards) came down to 9, before it was 250.
> >>>>>NVMe-RDMA target also uses 1MiB max IO size.
> >>>>Max, remind me what was the point to support 16M? Did this resolve
> >>>>an issue?

Re: reduce iSERT Max IO size

[Max Gurtovoy]

On 10/7/2020 6:36 AM, Krishnamraju Eraparaju wrote:
> On Sunday, October 10/04/20, 2020 at 00:45:26 +0300, Max Gurtovoy wrote:
>> On 10/3/2020 6:36 AM, Krishnamraju Eraparaju wrote:
>>> On Friday, October 10/02/20, 2020 at 13:29:30 -0700, Sagi Grimberg wrote:
>>>>> Hi Sagi & Max,
>>>>>
>>>>> Any update on this?
>>>>> Please change the max IO size to 1MiB(256 pages).
>>>> I think that the reason why this was changed to handle the worst case
>>>> was in case there are different capabilities on the initiator and the
>>>> target with respect to number of pages per MR. There is no handshake
>>>> that aligns expectations.
>>> But, the max pages per MR supported by most adapters is around 256 pages
>>> only.
>>> And I think only those iSER initiators, whose max pages per MR is 4096,
>>> could send 16MiB sized IOs, am I correct?
>> If the initiator can send 16MiB, we must make sure the target is
>> capable to receive it.
> I think max IO size, at iSER initiator, depends on
> "max_fast_reg_page_list_len".
> currently, below are the supported "max_fast_reg_page_list_len" of
> various iwarp drivers:
>
> iw_cxgb4: 128 pages
> Softiwarp: 256 pages
> i40iw: 512 pages
> qedr: couldn't find.
>
> For iwarp case, if 512 is the max pages supported by all iwarp drivers,
> then provisioning a gigantic MR pool at target(to accommodate never used
> 16MiB IO) wouldn't be a overkill?

For RoCE/IB Mellanox HCAs we support 16MiB IO size and even more. We 
limited to 16MiB in iSER/iSERT.

Sagi,

what about adding a module parameter for this as we did in iSER initiator ?

>>>> If we revert that it would restore the issue that you reported in the
>>>> first place:
>>>>
>>>> --
>>>> IB/isert: allocate RW ctxs according to max IO size
>>> I don't see the reported issue after reducing the IO size to 256
>>> pages(keeping all other changes of this patch intact).
>>> That is, "attr.cap.max_rdma_ctxs" is now getting filled properly with
>>> "rdma_rw_mr_factor()" related changes, I think.
>>>
>>> Before this change "attr.cap.max_rdma_ctxs" was hardcoded with
>>> 128(ISCSI_DEF_XMIT_CMDS_MAX) pages, which is very low for single target
>>> and muli-luns case.
>>>
>>> So reverting only ISCSI_ISER_MAX_SG_TABLESIZE macro to 256 doesn't cause the
>>> reported issue.
>>>
>>> Thanks,
>>> Krishnam Raju.
>>>> Current iSER target code allocates MR pool budget based on queue size.
>>>> Since there is no handshake between iSER initiator and target on max IO
>>>> size, we'll set the iSER target to support upto 16MiB IO operations and
>>>> allocate the correct number of RDMA ctxs according to the factor of MR's
>>>> per IO operation. This would guaranty sufficient size of the MR pool for
>>>> the required IO queue depth and IO size.
>>>>
>>>> Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
>>>> Tested-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
>>>> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
>>>> --
>>>>
>>>>> Thanks,
>>>>> Krishnam Raju.
>>>>> On Wednesday, September 09/23/20, 2020 at 01:57:47 -0700, Sagi Grimberg wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Please reduce the Max IO size to 1MiB(256 pages), at iSER Target.
>>>>>>> The PBL memory consumption has increased significantly after increasing
>>>>>>> the Max IO size to 16MiB(with commit:317000b926b07c).
>>>>>>> Due to the large MR pool, the max no.of iSER connections(On one variant
>>>>>>> of Chelsio cards) came down to 9, before it was 250.
>>>>>>> NVMe-RDMA target also uses 1MiB max IO size.
>>>>>> Max, remind me what was the point to support 16M? Did this resolve
>>>>>> an issue?

Re: reduce iSERT Max IO size

[Sagi Grimberg]

>> I think max IO size, at iSER initiator, depends on
>> "max_fast_reg_page_list_len".
>> currently, below are the supported "max_fast_reg_page_list_len" of
>> various iwarp drivers:
>>
>> iw_cxgb4: 128 pages
>> Softiwarp: 256 pages
>> i40iw: 512 pages
>> qedr: couldn't find.
>>
>> For iwarp case, if 512 is the max pages supported by all iwarp drivers,
>> then provisioning a gigantic MR pool at target(to accommodate never used
>> 16MiB IO) wouldn't be a overkill?
> 
> For RoCE/IB Mellanox HCAs we support 16MiB IO size and even more. We 
> limited to 16MiB in iSER/iSERT.
> 
> Sagi,
> 
> what about adding a module parameter for this as we did in iSER initiator ?

I don't think we have any other choice...

Re: reduce iSERT Max IO size

[Leon Romanovsky]

On Wed, Oct 07, 2020 at 04:50:27PM -0700, Sagi Grimberg wrote:
>
> > > I think max IO size, at iSER initiator, depends on
> > > "max_fast_reg_page_list_len".
> > > currently, below are the supported "max_fast_reg_page_list_len" of
> > > various iwarp drivers:
> > >
> > > iw_cxgb4: 128 pages
> > > Softiwarp: 256 pages
> > > i40iw: 512 pages
> > > qedr: couldn't find.
> > >
> > > For iwarp case, if 512 is the max pages supported by all iwarp drivers,
> > > then provisioning a gigantic MR pool at target(to accommodate never used
> > > 16MiB IO) wouldn't be a overkill?
> >
> > For RoCE/IB Mellanox HCAs we support 16MiB IO size and even more. We
> > limited to 16MiB in iSER/iSERT.
> >
> > Sagi,
> >
> > what about adding a module parameter for this as we did in iSER initiator ?
>
> I don't think we have any other choice...

Sagi,

I didn't read whole thread and know little about ULPs, but wonder if isn't
it possible to check device type (iWARP/RoCE) during iSERT initialization
and create MR pool only after device is recognized?

Thanks

Re: Re: reduce iSERT Max IO size

[Bernard Metzler]

-----"Krishnamraju Eraparaju" <krishna2@chelsio.com> wrote: -----

>To: "Max Gurtovoy" <mgurtovoy@nvidia.com>
>From: "Krishnamraju Eraparaju" <krishna2@chelsio.com>
>Date: 10/07/2020 05:36AM
>Cc: "Sagi Grimberg" <sagi@grimberg.me>, linux-rdma@vger.kernel.org,
>"Potnuri Bharat Teja" <bharat@chelsio.com>, "Max Gurtovoy"
><maxg@mellanox.com>
>Subject: [EXTERNAL] Re: reduce iSERT Max IO size
>
>On Sunday, October 10/04/20, 2020 at 00:45:26 +0300, Max Gurtovoy
>wrote:
>> 
>> On 10/3/2020 6:36 AM, Krishnamraju Eraparaju wrote:
>> >On Friday, October 10/02/20, 2020 at 13:29:30 -0700, Sagi Grimberg
>wrote:
>> >>>Hi Sagi & Max,
>> >>>
>> >>>Any update on this?
>> >>>Please change the max IO size to 1MiB(256 pages).
>> >>I think that the reason why this was changed to handle the worst
>case
>> >>was in case there are different capabilities on the initiator and
>the
>> >>target with respect to number of pages per MR. There is no
>handshake
>> >>that aligns expectations.
>> >But, the max pages per MR supported by most adapters is around 256
>pages
>> >only.
>> >And I think only those iSER initiators, whose max pages per MR is
>4096,
>> >could send 16MiB sized IOs, am I correct?
>> 
>> If the initiator can send 16MiB, we must make sure the target is
>> capable to receive it.
>I think max IO size, at iSER initiator, depends on
>"max_fast_reg_page_list_len".
>currently, below are the supported "max_fast_reg_page_list_len" of
>various iwarp drivers:
>
>iw_cxgb4: 128 pages
>Softiwarp: 256 pages
>i40iw: 512 pages
>qedr: couldn't find.
>

For siw, this limit is not determined by resource constraints.
We could bump it up to 512, or higher. What is a reasonable
maximum, from iSER view?


>For iwarp case, if 512 is the max pages supported by all iwarp
>drivers,
>then provisioning a gigantic MR pool at target(to accommodate never
>used
>16MiB IO) wouldn't be a overkill?
>> 
>> >
>> >>If we revert that it would restore the issue that you reported in
>the
>> >>first place:
>> >>
>> >>--
>> >>IB/isert: allocate RW ctxs according to max IO size
>> >I don't see the reported issue after reducing the IO size to 256
>> >pages(keeping all other changes of this patch intact).
>> >That is, "attr.cap.max_rdma_ctxs" is now getting filled properly
>with
>> >"rdma_rw_mr_factor()" related changes, I think.
>> >
>> >Before this change "attr.cap.max_rdma_ctxs" was hardcoded with
>> >128(ISCSI_DEF_XMIT_CMDS_MAX) pages, which is very low for single
>target
>> >and muli-luns case.
>> >
>> >So reverting only ISCSI_ISER_MAX_SG_TABLESIZE macro to 256 doesn't
>cause the
>> >reported issue.
>> >
>> >Thanks,
>> >Krishnam Raju.
>> >>Current iSER target code allocates MR pool budget based on queue
>size.
>> >>Since there is no handshake between iSER initiator and target on
>max IO
>> >>size, we'll set the iSER target to support upto 16MiB IO
>operations and
>> >>allocate the correct number of RDMA ctxs according to the factor
>of MR's
>> >>per IO operation. This would guaranty sufficient size of the MR
>pool for
>> >>the required IO queue depth and IO size.
>> >>
>> >>Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
>> >>Tested-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
>> >>Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
>> >>--
>> >>
>> >>>
>> >>>Thanks,
>> >>>Krishnam Raju.
>> >>>On Wednesday, September 09/23/20, 2020 at 01:57:47 -0700, Sagi
>Grimberg wrote:
>> >>>>>Hi,
>> >>>>>
>> >>>>>Please reduce the Max IO size to 1MiB(256 pages), at iSER
>Target.
>> >>>>>The PBL memory consumption has increased significantly after
>increasing
>> >>>>>the Max IO size to 16MiB(with commit:317000b926b07c).
>> >>>>>Due to the large MR pool, the max no.of iSER connections(On
>one variant
>> >>>>>of Chelsio cards) came down to 9, before it was 250.
>> >>>>>NVMe-RDMA target also uses 1MiB max IO size.
>> >>>>Max, remind me what was the point to support 16M? Did this
>resolve
>> >>>>an issue?
>

Re: reduce iSERT Max IO size

[Sagi Grimberg]

>>>> I think max IO size, at iSER initiator, depends on
>>>> "max_fast_reg_page_list_len".
>>>> currently, below are the supported "max_fast_reg_page_list_len" of
>>>> various iwarp drivers:
>>>>
>>>> iw_cxgb4: 128 pages
>>>> Softiwarp: 256 pages
>>>> i40iw: 512 pages
>>>> qedr: couldn't find.
>>>>
>>>> For iwarp case, if 512 is the max pages supported by all iwarp drivers,
>>>> then provisioning a gigantic MR pool at target(to accommodate never used
>>>> 16MiB IO) wouldn't be a overkill?
>>>
>>> For RoCE/IB Mellanox HCAs we support 16MiB IO size and even more. We
>>> limited to 16MiB in iSER/iSERT.
>>>
>>> Sagi,
>>>
>>> what about adding a module parameter for this as we did in iSER initiator ?
>>
>> I don't think we have any other choice...
> 
> Sagi,
> 
> I didn't read whole thread and know little about ULPs, but wonder if isn't
> it possible to check device type (iWARP/RoCE) during iSERT initialization
> and create MR pool only after device is recognized?

Its already done this way. The problem is that there is no handshake
procedure in iSER between the host and the target for what the maximum
transfer size would be, so currently isert sets up to a worse case of
16MB. This has implications on memory requirements.

In the absence of a handshake we should have the user choose with a 
param... Ideally that would be a configfs parameter, but a modparam can 
work as well here.

Re: Re: reduce iSERT Max IO size

[Krishnamraju Eraparaju]

On Thursday, October 10/08/20, 2020 at 13:12:39 +0000, Bernard Metzler wrote:
> -----"Krishnamraju Eraparaju" <krishna2@chelsio.com> wrote: -----
> 
> >To: "Max Gurtovoy" <mgurtovoy@nvidia.com>
> >From: "Krishnamraju Eraparaju" <krishna2@chelsio.com>
> >Date: 10/07/2020 05:36AM
> >Cc: "Sagi Grimberg" <sagi@grimberg.me>, linux-rdma@vger.kernel.org,
> >"Potnuri Bharat Teja" <bharat@chelsio.com>, "Max Gurtovoy"
> ><maxg@mellanox.com>
> >Subject: [EXTERNAL] Re: reduce iSERT Max IO size
> >
> >On Sunday, October 10/04/20, 2020 at 00:45:26 +0300, Max Gurtovoy
> >wrote:
> >> 
> >> On 10/3/2020 6:36 AM, Krishnamraju Eraparaju wrote:
> >> >On Friday, October 10/02/20, 2020 at 13:29:30 -0700, Sagi Grimberg
> >wrote:
> >> >>>Hi Sagi & Max,
> >> >>>
> >> >>>Any update on this?
> >> >>>Please change the max IO size to 1MiB(256 pages).
> >> >>I think that the reason why this was changed to handle the worst
> >case
> >> >>was in case there are different capabilities on the initiator and
> >the
> >> >>target with respect to number of pages per MR. There is no
> >handshake
> >> >>that aligns expectations.
> >> >But, the max pages per MR supported by most adapters is around 256
> >pages
> >> >only.
> >> >And I think only those iSER initiators, whose max pages per MR is
> >4096,
> >> >could send 16MiB sized IOs, am I correct?
> >> 
> >> If the initiator can send 16MiB, we must make sure the target is
> >> capable to receive it.
> >I think max IO size, at iSER initiator, depends on
> >"max_fast_reg_page_list_len".
> >currently, below are the supported "max_fast_reg_page_list_len" of
> >various iwarp drivers:
> >
> >iw_cxgb4: 128 pages
> >Softiwarp: 256 pages
> >i40iw: 512 pages
> >qedr: couldn't find.
> >
> 
> For siw, this limit is not determined by resource constraints.
> We could bump it up to 512, or higher. What is a reasonable
> maximum, from iSER view?

If the most common IO sizes are 4K & 8K, then the reasonable max IO size of
256 pages(1 MiB) would be appropriate, by default. currently, NVMet-rdma
also limits max IO size to 1MiB.
> 
> 
> >For iwarp case, if 512 is the max pages supported by all iwarp
> >drivers,
> >then provisioning a gigantic MR pool at target(to accommodate never
> >used
> >16MiB IO) wouldn't be a overkill?
> >> 
> >> >
> >> >>If we revert that it would restore the issue that you reported in
> >the
> >> >>first place:
> >> >>
> >> >>--
> >> >>IB/isert: allocate RW ctxs according to max IO size
> >> >I don't see the reported issue after reducing the IO size to 256
> >> >pages(keeping all other changes of this patch intact).
> >> >That is, "attr.cap.max_rdma_ctxs" is now getting filled properly
> >with
> >> >"rdma_rw_mr_factor()" related changes, I think.
> >> >
> >> >Before this change "attr.cap.max_rdma_ctxs" was hardcoded with
> >> >128(ISCSI_DEF_XMIT_CMDS_MAX) pages, which is very low for single
> >target
> >> >and muli-luns case.
> >> >
> >> >So reverting only ISCSI_ISER_MAX_SG_TABLESIZE macro to 256 doesn't
> >cause the
> >> >reported issue.
> >> >
> >> >Thanks,
> >> >Krishnam Raju.
> >> >>Current iSER target code allocates MR pool budget based on queue
> >size.
> >> >>Since there is no handshake between iSER initiator and target on
> >max IO
> >> >>size, we'll set the iSER target to support upto 16MiB IO
> >operations and
> >> >>allocate the correct number of RDMA ctxs according to the factor
> >of MR's
> >> >>per IO operation. This would guaranty sufficient size of the MR
> >pool for
> >> >>the required IO queue depth and IO size.
> >> >>
> >> >>Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
> >> >>Tested-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
> >> >>Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> >> >>--
> >> >>
> >> >>>
> >> >>>Thanks,
> >> >>>Krishnam Raju.
> >> >>>On Wednesday, September 09/23/20, 2020 at 01:57:47 -0700, Sagi
> >Grimberg wrote:
> >> >>>>>Hi,
> >> >>>>>
> >> >>>>>Please reduce the Max IO size to 1MiB(256 pages), at iSER
> >Target.
> >> >>>>>The PBL memory consumption has increased significantly after
> >increasing
> >> >>>>>the Max IO size to 16MiB(with commit:317000b926b07c).
> >> >>>>>Due to the large MR pool, the max no.of iSER connections(On
> >one variant
> >> >>>>>of Chelsio cards) came down to 9, before it was 250.
> >> >>>>>NVMe-RDMA target also uses 1MiB max IO size.
> >> >>>>Max, remind me what was the point to support 16M? Did this
> >resolve
> >> >>>>an issue?
> >
> 

Re: reduce iSERT Max IO size

[Max Gurtovoy]

On 10/8/2020 9:59 PM, Krishnamraju Eraparaju wrote:
> On Thursday, October 10/08/20, 2020 at 13:12:39 +0000, Bernard Metzler wrote:
>> -----"Krishnamraju Eraparaju" <krishna2@chelsio.com> wrote: -----
>>
>>> To: "Max Gurtovoy" <mgurtovoy@nvidia.com>
>>> From: "Krishnamraju Eraparaju" <krishna2@chelsio.com>
>>> Date: 10/07/2020 05:36AM
>>> Cc: "Sagi Grimberg" <sagi@grimberg.me>, linux-rdma@vger.kernel.org,
>>> "Potnuri Bharat Teja" <bharat@chelsio.com>, "Max Gurtovoy"
>>> <maxg@mellanox.com>
>>> Subject: [EXTERNAL] Re: reduce iSERT Max IO size
>>>
>>> On Sunday, October 10/04/20, 2020 at 00:45:26 +0300, Max Gurtovoy
>>> wrote:
>>>> On 10/3/2020 6:36 AM, Krishnamraju Eraparaju wrote:
>>>>> On Friday, October 10/02/20, 2020 at 13:29:30 -0700, Sagi Grimberg
>>> wrote:
>>>>>>> Hi Sagi & Max,
>>>>>>>
>>>>>>> Any update on this?
>>>>>>> Please change the max IO size to 1MiB(256 pages).
>>>>>> I think that the reason why this was changed to handle the worst
>>> case
>>>>>> was in case there are different capabilities on the initiator and
>>> the
>>>>>> target with respect to number of pages per MR. There is no
>>> handshake
>>>>>> that aligns expectations.
>>>>> But, the max pages per MR supported by most adapters is around 256
>>> pages
>>>>> only.
>>>>> And I think only those iSER initiators, whose max pages per MR is
>>> 4096,
>>>>> could send 16MiB sized IOs, am I correct?
>>>> If the initiator can send 16MiB, we must make sure the target is
>>>> capable to receive it.
>>> I think max IO size, at iSER initiator, depends on
>>> "max_fast_reg_page_list_len".
>>> currently, below are the supported "max_fast_reg_page_list_len" of
>>> various iwarp drivers:
>>>
>>> iw_cxgb4: 128 pages
>>> Softiwarp: 256 pages
>>> i40iw: 512 pages
>>> qedr: couldn't find.
>>>
>> For siw, this limit is not determined by resource constraints.
>> We could bump it up to 512, or higher. What is a reasonable
>> maximum, from iSER view?
> If the most common IO sizes are 4K & 8K, then the reasonable max IO size of
> 256 pages(1 MiB) would be appropriate, by default. currently, NVMet-rdma
> also limits max IO size to 1MiB.

We can set a default to 1MiB, and add module param that can increase 
this size to a max IO size of 16MiB.

I'll sent a patch early next week.


>>
>>> For iwarp case, if 512 is the max pages supported by all iwarp
>>> drivers,
>>> then provisioning a gigantic MR pool at target(to accommodate never
>>> used
>>> 16MiB IO) wouldn't be a overkill?
>>>>>> If we revert that it would restore the issue that you reported in
>>> the
>>>>>> first place:
>>>>>>
>>>>>> --
>>>>>> IB/isert: allocate RW ctxs according to max IO size
>>>>> I don't see the reported issue after reducing the IO size to 256
>>>>> pages(keeping all other changes of this patch intact).
>>>>> That is, "attr.cap.max_rdma_ctxs" is now getting filled properly
>>> with
>>>>> "rdma_rw_mr_factor()" related changes, I think.
>>>>>
>>>>> Before this change "attr.cap.max_rdma_ctxs" was hardcoded with
>>>>> 128(ISCSI_DEF_XMIT_CMDS_MAX) pages, which is very low for single
>>> target
>>>>> and muli-luns case.
>>>>>
>>>>> So reverting only ISCSI_ISER_MAX_SG_TABLESIZE macro to 256 doesn't
>>> cause the
>>>>> reported issue.
>>>>>
>>>>> Thanks,
>>>>> Krishnam Raju.
>>>>>> Current iSER target code allocates MR pool budget based on queue
>>> size.
>>>>>> Since there is no handshake between iSER initiator and target on
>>> max IO
>>>>>> size, we'll set the iSER target to support upto 16MiB IO
>>> operations and
>>>>>> allocate the correct number of RDMA ctxs according to the factor
>>> of MR's
>>>>>> per IO operation. This would guaranty sufficient size of the MR
>>> pool for
>>>>>> the required IO queue depth and IO size.
>>>>>>
>>>>>> Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
>>>>>> Tested-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
>>>>>> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
>>>>>> --
>>>>>>
>>>>>>> Thanks,
>>>>>>> Krishnam Raju.
>>>>>>> On Wednesday, September 09/23/20, 2020 at 01:57:47 -0700, Sagi
>>> Grimberg wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Please reduce the Max IO size to 1MiB(256 pages), at iSER
>>> Target.
>>>>>>>>> The PBL memory consumption has increased significantly after
>>> increasing
>>>>>>>>> the Max IO size to 16MiB(with commit:317000b926b07c).
>>>>>>>>> Due to the large MR pool, the max no.of iSER connections(On
>>> one variant
>>>>>>>>> of Chelsio cards) came down to 9, before it was 250.
>>>>>>>>> NVMe-RDMA target also uses 1MiB max IO size.
>>>>>>>> Max, remind me what was the point to support 16M? Did this
>>> resolve
>>>>>>>> an issue?

Re: reduce iSERT Max IO size

[Krishnamraju Eraparaju]

On Friday, October 10/09/20, 2020 at 01:47:04 +0300, Max Gurtovoy wrote:
> 
> On 10/8/2020 9:59 PM, Krishnamraju Eraparaju wrote:
> >On Thursday, October 10/08/20, 2020 at 13:12:39 +0000, Bernard Metzler wrote:
> >>-----"Krishnamraju Eraparaju" <krishna2@chelsio.com> wrote: -----
> >>
> >>>To: "Max Gurtovoy" <mgurtovoy@nvidia.com>
> >>>From: "Krishnamraju Eraparaju" <krishna2@chelsio.com>
> >>>Date: 10/07/2020 05:36AM
> >>>Cc: "Sagi Grimberg" <sagi@grimberg.me>, linux-rdma@vger.kernel.org,
> >>>"Potnuri Bharat Teja" <bharat@chelsio.com>, "Max Gurtovoy"
> >>><maxg@mellanox.com>
> >>>Subject: [EXTERNAL] Re: reduce iSERT Max IO size
> >>>
> >>>On Sunday, October 10/04/20, 2020 at 00:45:26 +0300, Max Gurtovoy
> >>>wrote:
> >>>>On 10/3/2020 6:36 AM, Krishnamraju Eraparaju wrote:
> >>>>>On Friday, October 10/02/20, 2020 at 13:29:30 -0700, Sagi Grimberg
> >>>wrote:
> >>>>>>>Hi Sagi & Max,
> >>>>>>>
> >>>>>>>Any update on this?
> >>>>>>>Please change the max IO size to 1MiB(256 pages).
> >>>>>>I think that the reason why this was changed to handle the worst
> >>>case
> >>>>>>was in case there are different capabilities on the initiator and
> >>>the
> >>>>>>target with respect to number of pages per MR. There is no
> >>>handshake
> >>>>>>that aligns expectations.
> >>>>>But, the max pages per MR supported by most adapters is around 256
> >>>pages
> >>>>>only.
> >>>>>And I think only those iSER initiators, whose max pages per MR is
> >>>4096,
> >>>>>could send 16MiB sized IOs, am I correct?
> >>>>If the initiator can send 16MiB, we must make sure the target is
> >>>>capable to receive it.
> >>>I think max IO size, at iSER initiator, depends on
> >>>"max_fast_reg_page_list_len".
> >>>currently, below are the supported "max_fast_reg_page_list_len" of
> >>>various iwarp drivers:
> >>>
> >>>iw_cxgb4: 128 pages
> >>>Softiwarp: 256 pages
> >>>i40iw: 512 pages
> >>>qedr: couldn't find.
> >>>
> >>For siw, this limit is not determined by resource constraints.
> >>We could bump it up to 512, or higher. What is a reasonable
> >>maximum, from iSER view?
> >If the most common IO sizes are 4K & 8K, then the reasonable max IO size of
> >256 pages(1 MiB) would be appropriate, by default. currently, NVMet-rdma
> >also limits max IO size to 1MiB.
> 
> We can set a default to 1MiB, and add module param that can increase
> this size to a max IO size of 16MiB.
> 
> I'll sent a patch early next week.

Thanks Max!

> 
> 
> >>
> >>>For iwarp case, if 512 is the max pages supported by all iwarp
> >>>drivers,
> >>>then provisioning a gigantic MR pool at target(to accommodate never
> >>>used
> >>>16MiB IO) wouldn't be a overkill?
> >>>>>>If we revert that it would restore the issue that you reported in
> >>>the
> >>>>>>first place:
> >>>>>>
> >>>>>>--
> >>>>>>IB/isert: allocate RW ctxs according to max IO size
> >>>>>I don't see the reported issue after reducing the IO size to 256
> >>>>>pages(keeping all other changes of this patch intact).
> >>>>>That is, "attr.cap.max_rdma_ctxs" is now getting filled properly
> >>>with
> >>>>>"rdma_rw_mr_factor()" related changes, I think.
> >>>>>
> >>>>>Before this change "attr.cap.max_rdma_ctxs" was hardcoded with
> >>>>>128(ISCSI_DEF_XMIT_CMDS_MAX) pages, which is very low for single
> >>>target
> >>>>>and muli-luns case.
> >>>>>
> >>>>>So reverting only ISCSI_ISER_MAX_SG_TABLESIZE macro to 256 doesn't
> >>>cause the
> >>>>>reported issue.
> >>>>>
> >>>>>Thanks,
> >>>>>Krishnam Raju.
> >>>>>>Current iSER target code allocates MR pool budget based on queue
> >>>size.
> >>>>>>Since there is no handshake between iSER initiator and target on
> >>>max IO
> >>>>>>size, we'll set the iSER target to support upto 16MiB IO
> >>>operations and
> >>>>>>allocate the correct number of RDMA ctxs according to the factor
> >>>of MR's
> >>>>>>per IO operation. This would guaranty sufficient size of the MR
> >>>pool for
> >>>>>>the required IO queue depth and IO size.
> >>>>>>
> >>>>>>Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
> >>>>>>Tested-by: Krishnamraju Eraparaju <krishna2@chelsio.com>
> >>>>>>Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> >>>>>>--
> >>>>>>
> >>>>>>>Thanks,
> >>>>>>>Krishnam Raju.
> >>>>>>>On Wednesday, September 09/23/20, 2020 at 01:57:47 -0700, Sagi
> >>>Grimberg wrote:
> >>>>>>>>>Hi,
> >>>>>>>>>
> >>>>>>>>>Please reduce the Max IO size to 1MiB(256 pages), at iSER
> >>>Target.
> >>>>>>>>>The PBL memory consumption has increased significantly after
> >>>increasing
> >>>>>>>>>the Max IO size to 16MiB(with commit:317000b926b07c).
> >>>>>>>>>Due to the large MR pool, the max no.of iSER connections(On
> >>>one variant
> >>>>>>>>>of Chelsio cards) came down to 9, before it was 250.
> >>>>>>>>>NVMe-RDMA target also uses 1MiB max IO size.
> >>>>>>>>Max, remind me what was the point to support 16M? Did this
> >>>resolve
> >>>>>>>>an issue?

Re: reduce iSERT Max IO size

[Leon Romanovsky]

On Thu, Oct 08, 2020 at 09:20:41AM -0700, Sagi Grimberg wrote:
>
> > > > > I think max IO size, at iSER initiator, depends on
> > > > > "max_fast_reg_page_list_len".
> > > > > currently, below are the supported "max_fast_reg_page_list_len" of
> > > > > various iwarp drivers:
> > > > >
> > > > > iw_cxgb4: 128 pages
> > > > > Softiwarp: 256 pages
> > > > > i40iw: 512 pages
> > > > > qedr: couldn't find.
> > > > >
> > > > > For iwarp case, if 512 is the max pages supported by all iwarp drivers,
> > > > > then provisioning a gigantic MR pool at target(to accommodate never used
> > > > > 16MiB IO) wouldn't be a overkill?
> > > >
> > > > For RoCE/IB Mellanox HCAs we support 16MiB IO size and even more. We
> > > > limited to 16MiB in iSER/iSERT.
> > > >
> > > > Sagi,
> > > >
> > > > what about adding a module parameter for this as we did in iSER initiator ?
> > >
> > > I don't think we have any other choice...
> >
> > Sagi,
> >
> > I didn't read whole thread and know little about ULPs, but wonder if isn't
> > it possible to check device type (iWARP/RoCE) during iSERT initialization
> > and create MR pool only after device is recognized?
>
> Its already done this way. The problem is that there is no handshake
> procedure in iSER between the host and the target for what the maximum
> transfer size would be, so currently isert sets up to a worse case of
> 16MB. This has implications on memory requirements.

Thanks for the explanation.