[dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Gagandeep Singh]

DPDK currently is supporting maximum 3 hugepage,
sizes whereas system can support more than this e.g.
64K, 2M, 32M and 1G.

Having these four hugepage sizes available to use by DPDK,
which is valid in case of '--in-memory' EAL option or
using 4 separate mount points for each hugepage size;
hugepage_info_init() API reports an error.

This change increases the maximum supported mount points
to 4.

Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
---

On ARM platform when the translation granule is 4KB,
there are 4 hugepage sizes supported.
On kernel 5.2, we can see below directories in
/sys/kernel/mm/hugepages:

hugepages-1048576kB
hugepages-2048kB
hugepages-32768kB
hugepages-64kB

 lib/librte_eal/common/eal_internal_cfg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index edff09d07..abb7ec913 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -15,7 +15,7 @@
 
 #include "eal_thread.h"
 
-#define MAX_HUGEPAGE_SIZES 3  /**< support up to 3 page sizes */
+#define MAX_HUGEPAGE_SIZES 4  /**< support up to 4 page sizes */
 
 /*
  * internal configuration structure for the number, size and
-- 
2.17.1

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[David Marchand]

On Wed, Aug 7, 2019 at 12:26 PM Gagandeep Singh <g.singh@nxp.com> wrote:
>
> DPDK currently is supporting maximum 3 hugepage,
> sizes whereas system can support more than this e.g.
> 64K, 2M, 32M and 1G.

You can mention ARM platform here, and that this issue starts with
kernel 5.2 (and I would try to mention this in the title as well).
This is better than an annotation that will be lost.


> Having these four hugepage sizes available to use by DPDK,
> which is valid in case of '--in-memory' EAL option or
> using 4 separate mount points for each hugepage size;
> hugepage_info_init() API reports an error.

Can you describe what is the impact from a user point of view rather
than mentioning this internal function?


> This change increases the maximum supported mount points
> to 4.

I suppose this fix does the trick for you.
However, we are in internal structures and I can't think of an impact
on datapath.
So we might as well use dynamic allocations rather than just enlarge this array.

Did you consider this?


-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Thomas Monjalon]

07/08/2019 14:00, David Marchand:
> On Wed, Aug 7, 2019 at 12:26 PM Gagandeep Singh <g.singh@nxp.com> wrote:
> >
> > DPDK currently is supporting maximum 3 hugepage,
> > sizes whereas system can support more than this e.g.
> > 64K, 2M, 32M and 1G.
> 
> You can mention ARM platform here, and that this issue starts with
> kernel 5.2 (and I would try to mention this in the title as well).
> This is better than an annotation that will be lost.
> 
> 
> > Having these four hugepage sizes available to use by DPDK,
> > which is valid in case of '--in-memory' EAL option or
> > using 4 separate mount points for each hugepage size;
> > hugepage_info_init() API reports an error.
> 
> Can you describe what is the impact from a user point of view rather
> than mentioning this internal function?

Yes please, we need to understand how much it is critical.
Should we Cc stable@dpdk.org for backport?
Should it be merged at the last minute in 19.08?


> > This change increases the maximum supported mount points
> > to 4.
> 
> I suppose this fix does the trick for you.
> However, we are in internal structures and I can't think of an impact
> on datapath.
> So we might as well use dynamic allocations rather than just enlarge this array.
> 
> Did you consider this?

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Hemant Agrawal]

HI Thomas,

> > > DPDK currently is supporting maximum 3 hugepage, sizes whereas
> > > system can support more than this e.g.
> > > 64K, 2M, 32M and 1G.
> >
> > You can mention ARM platform here, and that this issue starts with
> > kernel 5.2 (and I would try to mention this in the title as well).
> > This is better than an annotation that will be lost.
> >
> >
> > > Having these four hugepage sizes available to use by DPDK, which is
> > > valid in case of '--in-memory' EAL option or using 4 separate mount
> > > points for each hugepage size;
> > > hugepage_info_init() API reports an error.
> >
> > Can you describe what is the impact from a user point of view rather
> > than mentioning this internal function?
> 
> Yes please, we need to understand how much it is critical.
> Should we Cc stable@dpdk.org for backport?
> Should it be merged at the last minute in 19.08?
> 

VPP usages in-memory option. So, VPP on ARM with kernel 5.2 wont' work without this patch. 

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Stephen Hemminger]

On Wed,  7 Aug 2019 15:42:04 +0530
Gagandeep Singh <g.singh@nxp.com> wrote:

> DPDK currently is supporting maximum 3 hugepage,
> sizes whereas system can support more than this e.g.
> 64K, 2M, 32M and 1G.
> 
> Having these four hugepage sizes available to use by DPDK,
> which is valid in case of '--in-memory' EAL option or
> using 4 separate mount points for each hugepage size;
> hugepage_info_init() API reports an error.
> 
> This change increases the maximum supported mount points
> to 4.
> 
> Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
> 
> On ARM platform when the translation granule is 4KB,
> there are 4 hugepage sizes supported.
> On kernel 5.2, we can see below directories in
> /sys/kernel/mm/hugepages:
> 
> hugepages-1048576kB
> hugepages-2048kB
> hugepages-32768kB
> hugepages-64kB
> 
>  lib/librte_eal/common/eal_internal_cfg.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> index edff09d07..abb7ec913 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -15,7 +15,7 @@
>  
>  #include "eal_thread.h"
>  
> -#define MAX_HUGEPAGE_SIZES 3  /**< support up to 3 page sizes */
> +#define MAX_HUGEPAGE_SIZES 4  /**< support up to 4 page sizes */

At least this is hidden in internal config. 

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Thomas Monjalon]

07/08/2019 15:28, Hemant Agrawal:
> HI Thomas,
> 
> > > > DPDK currently is supporting maximum 3 hugepage, sizes whereas
> > > > system can support more than this e.g.
> > > > 64K, 2M, 32M and 1G.
> > >
> > > You can mention ARM platform here, and that this issue starts with
> > > kernel 5.2 (and I would try to mention this in the title as well).
> > > This is better than an annotation that will be lost.
> > >
> > >
> > > > Having these four hugepage sizes available to use by DPDK, which is
> > > > valid in case of '--in-memory' EAL option or using 4 separate mount
> > > > points for each hugepage size;
> > > > hugepage_info_init() API reports an error.
> > >
> > > Can you describe what is the impact from a user point of view rather
> > > than mentioning this internal function?
> > 
> > Yes please, we need to understand how much it is critical.
> > Should we Cc stable@dpdk.org for backport?
> > Should it be merged at the last minute in 19.08?
> 
> VPP usages in-memory option. So, VPP on ARM with kernel 5.2 wont' work without this patch. 

Do you want to send a v2 with a better explanation?

I would suggest to restrict the change to Arm only with an ifdef,
in order to limit the risk for this release.
We can think about a dynamic hugepage scan in the next release.

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[David Marchand]

On Wed, Aug 7, 2019 at 3:28 PM Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
>
> HI Thomas,
>
> > > > DPDK currently is supporting maximum 3 hugepage, sizes whereas
> > > > system can support more than this e.g.
> > > > 64K, 2M, 32M and 1G.
> > >
> > > You can mention ARM platform here, and that this issue starts with
> > > kernel 5.2 (and I would try to mention this in the title as well).
> > > This is better than an annotation that will be lost.
> > >
> > >
> > > > Having these four hugepage sizes available to use by DPDK, which is
> > > > valid in case of '--in-memory' EAL option or using 4 separate mount
> > > > points for each hugepage size;
> > > > hugepage_info_init() API reports an error.
> > >
> > > Can you describe what is the impact from a user point of view rather
> > > than mentioning this internal function?
> >
> > Yes please, we need to understand how much it is critical.
> > Should we Cc stable@dpdk.org for backport?
> > Should it be merged at the last minute in 19.08?
> >
>
> VPP usages in-memory option. So, VPP on ARM with kernel 5.2 wont' work without this patch.
>

I have been looking at the changes in the linux kernel.
Can you pinpoint at the commit that changed this in 5.2?

I can see a change in the code, but in 5.0, or maybe something changed
in the configuration.

The patch you propose is not that risky (x86 supports two pagesizes,
and max hugepage is already at 3, so we know the code works fine with
less than the max).
Yet, I want to understand why this is urgent now.

CCing other architecture maintainers.


-- 
David Marchand

Re: [dpdk-dev] [EXT] Re: [PATCH] eal: change max hugepage sizes to 4

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, August 8, 2019 1:03 PM
> To: Hemant Agrawal <hemant.agrawal@nxp.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; Gagandeep Singh
> <G.Singh@nxp.com>; dev <dev@dpdk.org>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Olivier Matz <olivier.matz@6wind.com>;
> Andrew Rybchenko <arybchenko@solarflare.com>; Nipun Gupta
> <nipun.gupta@nxp.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Gavin Hu <gavin.hu@arm.com>; Bruce Richardson
> <bruce.richardson@intel.com>
> Subject: [EXT] Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4
> On Wed, Aug 7, 2019 at 3:28 PM Hemant Agrawal <hemant.agrawal@nxp.com>
> wrote:
> >
> > HI Thomas,
> >
> > > > > DPDK currently is supporting maximum 3 hugepage, sizes whereas
> > > > > system can support more than this e.g.
> > > > > 64K, 2M, 32M and 1G.
> > > >
> > > > You can mention ARM platform here, and that this issue starts with
> > > > kernel 5.2 (and I would try to mention this in the title as well).
> > > > This is better than an annotation that will be lost.
> > > >
> > > >
> > > > > Having these four hugepage sizes available to use by DPDK, which
> > > > > is valid in case of '--in-memory' EAL option or using 4 separate
> > > > > mount points for each hugepage size;
> > > > > hugepage_info_init() API reports an error.
> > > >
> > > > Can you describe what is the impact from a user point of view
> > > > rather than mentioning this internal function?
> > >
> > > Yes please, we need to understand how much it is critical.
> > > Should we Cc stable@dpdk.org for backport?
> > > Should it be merged at the last minute in 19.08?
> > >
> >
> > VPP usages in-memory option. So, VPP on ARM with kernel 5.2 wont' work
> without this patch.
> >
> 
> I have been looking at the changes in the linux kernel.
> Can you pinpoint at the commit that changed this in 5.2?
> 
> I can see a change in the code, but in 5.0, or maybe something changed in the
> configuration.
> 
> The patch you propose is not that risky (x86 supports two pagesizes, and max
> hugepage is already at 3, so we know the code works fine with less than the
> max).
> Yet, I want to understand why this is urgent now.
> 
> CCing other architecture maintainers.

Tested this change with an arm64 machine + 4.18 kernel. Looks OK.
Tested-by: Jerin Jacob <jerinj@marvell.com>  

> 
> 
> --
> David Marchand

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Hemant Agrawal]

Hi David,

> > VPP usages in-memory option. So, VPP on ARM with kernel 5.2 wont' work
> without this patch.
> >
> 
> I have been looking at the changes in the linux kernel.
> Can you pinpoint at the commit that changed this in 5.2?
> 
> I can see a change in the code, but in 5.0, or maybe something changed in the
> configuration.

[Hemant] I am not aware of exact patch, which changed the behavior. May be Honnappa/Steve can help here.

Regards,
Hemant

> 
> The patch you propose is not that risky (x86 supports two pagesizes, and max
> hugepage is already at 3, so we know the code works fine with less than the
> max).
> Yet, I want to understand why this is urgent now.
> 
> CCing other architecture maintainers.
> 
> 
> --
> David Marchand

[dpdk-dev] [PATCH v2] eal: change max hugepage sizes to 4 for ARM platforms

[Gagandeep Singh]

ARM is supporting maximum 4 hugepage sizes (64K, 2M, 32M
and 1G) when granule is 4KB since very long and DPDK
support maximum 3 hugepage sizes.

With all 4 hugepage sizes enabled, applications and some
stacks like VPP which are working over DPDK and using
"in-memory" eal option, or using separate mount points
on ARM based platform, fails at huge page initialization,
reporting error messages from eal:

EAL: FATAL: Cannot get hugepage information.
EAL: Cannot get hugepage information.
EAL: Error - exiting with code: 1

This issue is visible on kernel 5.2 (stable rel), where
kernel is by default creating directories for each supported
hugepage size in /sys/kernel/mm/hugepages/

On earlier Stable Kernel LTR's, the directories visible in
/sys/kernel/mm/hugepages/ were dependent upon what hugepage
sizes are configured at boot time.

This change increases the maximum supported mount points
to 4 for ARM based platforms.

Cc: stable@dpdk.org

Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
---

Changes in V2:
 - restricted this change specific to ARM
 - commit message updated
 - stable added to cc

 lib/librte_eal/common/eal_internal_cfg.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index edff09d07..a42f34923 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -15,7 +15,11 @@
 
 #include "eal_thread.h"
 
+#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+#define MAX_HUGEPAGE_SIZES 4  /**< support up to 4 page sizes */
+#else
 #define MAX_HUGEPAGE_SIZES 3  /**< support up to 3 page sizes */
+#endif
 
 /*
  * internal configuration structure for the number, size and
-- 
2.17.1

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Steve Capper]

Hello,

On Thu, Aug 08, 2019 at 10:37:20AM +0000, Hemant Agrawal wrote:
> Hi David,
> 
> > > VPP usages in-memory option. So, VPP on ARM with kernel 5.2 wont' work
> > without this patch.
> > >
> > 
> > I have been looking at the changes in the linux kernel.
> > Can you pinpoint at the commit that changed this in 5.2?
> > 
> > I can see a change in the code, but in 5.0, or maybe something changed in the
> > configuration.
> 
> [Hemant] I am not aware of exact patch, which changed the behavior. May be Honnappa/Steve can help here.
> 

All the possible hugetlb sizes being advertised was added in v5.0 by:
a21b0b78eaf7 arm64: hugetlb: Register hugepages during arch init

(Some more recent patches changed the constants used but the logic
remained the same).

Cheers,
-- 
Steve

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] eal: change max hugepage sizes to 4 for ARM platforms

[David Marchand]

On Thu, Aug 8, 2019 at 2:27 PM Gagandeep Singh <g.singh@nxp.com> wrote:
>
> ARM is supporting maximum 4 hugepage sizes (64K, 2M, 32M
> and 1G) when granule is 4KB since very long and DPDK
> support maximum 3 hugepage sizes.
>
> With all 4 hugepage sizes enabled, applications and some
> stacks like VPP which are working over DPDK and using
> "in-memory" eal option, or using separate mount points
> on ARM based platform, fails at huge page initialization,
> reporting error messages from eal:
>
> EAL: FATAL: Cannot get hugepage information.
> EAL: Cannot get hugepage information.
> EAL: Error - exiting with code: 1
>
> This issue is visible on kernel 5.2 (stable rel), where
> kernel is by default creating directories for each supported
> hugepage size in /sys/kernel/mm/hugepages/

Yes, any supported hugepage size is announced in sysfs.

> On earlier Stable Kernel LTR's, the directories visible in
> /sys/kernel/mm/hugepages/ were dependent upon what hugepage
> sizes are configured at boot time.

The change is in kernel 5.0 with:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a21b0b78

where all supported sizes are now registered.

>
> This change increases the maximum supported mount points
> to 4 for ARM based platforms.
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
>
> Changes in V2:
>  - restricted this change specific to ARM
>  - commit message updated
>  - stable added to cc


Please keep me Cc:.


-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[David Marchand]

On Thu, Aug 8, 2019 at 2:29 PM Steve Capper <Steve.Capper@arm.com> wrote:
>
> Hello,
>
> On Thu, Aug 08, 2019 at 10:37:20AM +0000, Hemant Agrawal wrote:
> > Hi David,
> >
> > > > VPP usages in-memory option. So, VPP on ARM with kernel 5.2 wont' work
> > > without this patch.
> > > >
> > >
> > > I have been looking at the changes in the linux kernel.
> > > Can you pinpoint at the commit that changed this in 5.2?
> > >
> > > I can see a change in the code, but in 5.0, or maybe something changed in the
> > > configuration.
> >
> > [Hemant] I am not aware of exact patch, which changed the behavior. May be Honnappa/Steve can help here.
> >
>
> All the possible hugetlb sizes being advertised was added in v5.0 by:
> a21b0b78eaf7 arm64: hugetlb: Register hugepages during arch init
>
> (Some more recent patches changed the constants used but the logic
> remained the same).

Ah, thanks for confirming :-).


-- 
David Marchand

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] eal: change max hugepage sizes to 4 for ARM platforms

[Thomas Monjalon]

08/08/2019 14:37, David Marchand:
> On Thu, Aug 8, 2019 at 2:27 PM Gagandeep Singh <g.singh@nxp.com> wrote:
> >
> > ARM is supporting maximum 4 hugepage sizes (64K, 2M, 32M
> > and 1G) when granule is 4KB since very long and DPDK
> > support maximum 3 hugepage sizes.
> >
> > With all 4 hugepage sizes enabled, applications and some
> > stacks like VPP which are working over DPDK and using
> > "in-memory" eal option, or using separate mount points
> > on ARM based platform, fails at huge page initialization,
> > reporting error messages from eal:
> >
> > EAL: FATAL: Cannot get hugepage information.
> > EAL: Cannot get hugepage information.
> > EAL: Error - exiting with code: 1
> >
> > This issue is visible on kernel 5.2 (stable rel), where
> > kernel is by default creating directories for each supported
> > hugepage size in /sys/kernel/mm/hugepages/
> 
> Yes, any supported hugepage size is announced in sysfs.
> 
> > On earlier Stable Kernel LTR's, the directories visible in
> > /sys/kernel/mm/hugepages/ were dependent upon what hugepage
> > sizes are configured at boot time.
> 
> The change is in kernel 5.0 with:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a21b0b78
> 
> where all supported sizes are now registered.
> 
> >
> > This change increases the maximum supported mount points
> > to 4 for ARM based platforms.
> >
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>

Applied with small updates in commit log (especially commit origin in 5.0), thanks.

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Burakov, Anatoly]

On 07-Aug-19 2:28 PM, Hemant Agrawal wrote:
> HI Thomas,
> 
>>>> DPDK currently is supporting maximum 3 hugepage, sizes whereas
>>>> system can support more than this e.g.
>>>> 64K, 2M, 32M and 1G.
>>>
>>> You can mention ARM platform here, and that this issue starts with
>>> kernel 5.2 (and I would try to mention this in the title as well).
>>> This is better than an annotation that will be lost.
>>>
>>>
>>>> Having these four hugepage sizes available to use by DPDK, which is
>>>> valid in case of '--in-memory' EAL option or using 4 separate mount
>>>> points for each hugepage size;
>>>> hugepage_info_init() API reports an error.
>>>
>>> Can you describe what is the impact from a user point of view rather
>>> than mentioning this internal function?
>>
>> Yes please, we need to understand how much it is critical.
>> Should we Cc stable@dpdk.org for backport?
>> Should it be merged at the last minute in 19.08?
>>
> 
> VPP usages in-memory option. So, VPP on ARM with kernel 5.2 wont' work without this patch.
> 

Off-topic, but it's nice to see real-world usage for this option!

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Burakov, Anatoly]

On 08-Aug-19 8:31 AM, Thomas Monjalon wrote:
> 07/08/2019 15:28, Hemant Agrawal:
>> HI Thomas,
>>
>>>>> DPDK currently is supporting maximum 3 hugepage, sizes whereas
>>>>> system can support more than this e.g.
>>>>> 64K, 2M, 32M and 1G.
>>>>
>>>> You can mention ARM platform here, and that this issue starts with
>>>> kernel 5.2 (and I would try to mention this in the title as well).
>>>> This is better than an annotation that will be lost.
>>>>
>>>>
>>>>> Having these four hugepage sizes available to use by DPDK, which is
>>>>> valid in case of '--in-memory' EAL option or using 4 separate mount
>>>>> points for each hugepage size;
>>>>> hugepage_info_init() API reports an error.
>>>>
>>>> Can you describe what is the impact from a user point of view rather
>>>> than mentioning this internal function?
>>>
>>> Yes please, we need to understand how much it is critical.
>>> Should we Cc stable@dpdk.org for backport?
>>> Should it be merged at the last minute in 19.08?
>>
>> VPP usages in-memory option. So, VPP on ARM with kernel 5.2 wont' work without this patch.
> 
> Do you want to send a v2 with a better explanation?
> 
> I would suggest to restrict the change to Arm only with an ifdef,
> in order to limit the risk for this release.
> We can think about a dynamic hugepage scan in the next release.
> 

I don't see how this is necessary. The 3 is an arbitrary number here, 
and the ABI isn't broken as this is an internal structure. We could 
increase it to 16 for all i care, and it wouldn't make any difference to 
the rest of the code - we never populate more than we can find anyway.

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[David Marchand]

On Mon, Aug 12, 2019 at 11:43 AM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
> On 08-Aug-19 8:31 AM, Thomas Monjalon wrote:
> > I would suggest to restrict the change to Arm only with an ifdef,
> > in order to limit the risk for this release.
> > We can think about a dynamic hugepage scan in the next release.
> >
>
> I don't see how this is necessary. The 3 is an arbitrary number here,
> and the ABI isn't broken as this is an internal structure. We could
> increase it to 16 for all i care, and it wouldn't make any difference to
> the rest of the code - we never populate more than we can find anyway.

I agree on the principle.
But at the time this popped up, we were really close to the release.
It seemed a way to mitigate any unforeseen issue by limiting to the
platform that was affected.


-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Thomas Monjalon]

12/08/2019 11:49, David Marchand:
> On Mon, Aug 12, 2019 at 11:43 AM Burakov, Anatoly
> <anatoly.burakov@intel.com> wrote:
> > On 08-Aug-19 8:31 AM, Thomas Monjalon wrote:
> > > I would suggest to restrict the change to Arm only with an ifdef,
> > > in order to limit the risk for this release.
> > > We can think about a dynamic hugepage scan in the next release.
> >
> > I don't see how this is necessary. The 3 is an arbitrary number here,
> > and the ABI isn't broken as this is an internal structure. We could
> > increase it to 16 for all i care, and it wouldn't make any difference to
> > the rest of the code - we never populate more than we can find anyway.
> 
> I agree on the principle.
> But at the time this popped up, we were really close to the release.
> It seemed a way to mitigate any unforeseen issue by limiting to the
> platform that was affected.

Exactly, we were extra cautious.

Please increase the value for everybody, thanks.

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Burakov, Anatoly]

On 12-Aug-19 10:49 AM, David Marchand wrote:
> On Mon, Aug 12, 2019 at 11:43 AM Burakov, Anatoly
> <anatoly.burakov@intel.com> wrote:
>> On 08-Aug-19 8:31 AM, Thomas Monjalon wrote:
>>> I would suggest to restrict the change to Arm only with an ifdef,
>>> in order to limit the risk for this release.
>>> We can think about a dynamic hugepage scan in the next release.
>>>
>>
>> I don't see how this is necessary. The 3 is an arbitrary number here,
>> and the ABI isn't broken as this is an internal structure. We could
>> increase it to 16 for all i care, and it wouldn't make any difference to
>> the rest of the code - we never populate more than we can find anyway.
> 
> I agree on the principle.
> But at the time this popped up, we were really close to the release.
> It seemed a way to mitigate any unforeseen issue by limiting to the
> platform that was affected.
> 

Fair enough. A follow up is needed so. Frankly, i don't see the need to 
complicate things with "dynamic" stuff here - a static array of 8 or 16 
page sizes should be enough for everyone (TM).

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[David Marchand]

On Thu, Aug 8, 2019 at 11:00 AM Gagandeep Singh <G.Singh@nxp.com> wrote:
>
> > >HI Thomas,
> > > > > > DPDK currently is supporting maximum 3 hugepage, sizes whereas
> > > > > > system can support more than this e.g.
> > > > > > 64K, 2M, 32M and 1G.
> > > > >
> > > > > You can mention ARM platform here, and that this issue starts with
> > > > > kernel 5.2 (and I would try to mention this in the title as well).
> > > > > This is better than an annotation that will be lost.
> > > > >
> > > > >
> > > > > > Having these four hugepage sizes available to use by DPDK, which is
> > > > > > valid in case of '--in-memory' EAL option or using 4 separate mount
> > > > > > points for each hugepage size;
> > > > > > hugepage_info_init() API reports an error.
> > > > >
> > > > > Can you describe what is the impact from a user point of view rather
> > > > > than mentioning this internal function?
> > > >
> > > > Yes please, we need to understand how much it is critical.
> > > > Should we Cc stable@dpdk.org for backport?
> > > > Should it be merged at the last minute in 19.08?
> > >
> > > VPP usages in-memory option. So, VPP on ARM with kernel 5.2 wont' work
> > without this patch.
> >
> > Do you want to send a v2 with a better explanation?
> >
> > I would suggest to restrict the change to Arm only with an ifdef,
> > in order to limit the risk for this release.
> > We can think about a dynamic hugepage scan in the next release.
> >
> Ok, I will send a v2 with better explanation and will also add an if check to make this change for ARM specific only.

I hope this explanation covers the question I had about when the
change happened in the kernel :-).
Thanks.


-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Gagandeep Singh]

> >HI Thomas,
> > > > > DPDK currently is supporting maximum 3 hugepage, sizes whereas
> > > > > system can support more than this e.g.
> > > > > 64K, 2M, 32M and 1G.
> > > >
> > > > You can mention ARM platform here, and that this issue starts with
> > > > kernel 5.2 (and I would try to mention this in the title as well).
> > > > This is better than an annotation that will be lost.
> > > >
> > > >
> > > > > Having these four hugepage sizes available to use by DPDK, which is
> > > > > valid in case of '--in-memory' EAL option or using 4 separate mount
> > > > > points for each hugepage size;
> > > > > hugepage_info_init() API reports an error.
> > > >
> > > > Can you describe what is the impact from a user point of view rather
> > > > than mentioning this internal function?
> > >
> > > Yes please, we need to understand how much it is critical.
> > > Should we Cc stable@dpdk.org for backport?
> > > Should it be merged at the last minute in 19.08?
> >
> > VPP usages in-memory option. So, VPP on ARM with kernel 5.2 wont' work
> without this patch.
> 
> Do you want to send a v2 with a better explanation?
> 
> I would suggest to restrict the change to Arm only with an ifdef,
> in order to limit the risk for this release.
> We can think about a dynamic hugepage scan in the next release.
> 
Ok, I will send a v2 with better explanation and will also add an if check to make this change for ARM specific only.

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Honnappa Nagarahalli]

<snip>
 
> Hi Thomas,
> 
> > 07/08/2019 14:00, David Marchand:
> > > On Wed, Aug 7, 2019 at 12:26 PM Gagandeep Singh <g.singh@nxp.com>
> > wrote:
> > > >
> > > > DPDK currently is supporting maximum 3 hugepage, sizes whereas
> > > > system can support more than this e.g.
> > > > 64K, 2M, 32M and 1G.
> > >
> > > You can mention ARM platform here, and that this issue starts with
> > > kernel 5.2 (and I would try to mention this in the title as well).
> > > This is better than an annotation that will be lost.
> > >
> > >
> > > > Having these four hugepage sizes available to use by DPDK, which
> > > > is valid in case of '--in-memory' EAL option or using 4 separate
> > > > mount points for each hugepage size;
> > > > hugepage_info_init() API reports an error.
> > >
> > > Can you describe what is the impact from a user point of view rather
> > > than mentioning this internal function?
> >
> > Yes please, we need to understand how much it is critical.
> 
> It is critical for stacks like VPP which uses by default in-memory.
> There stack would be broken on kernel 5.2 and above.
> I will change the description to make it clear.
> 
> > Should we Cc stable@dpdk.org for backport?
> yes
> 
> > Should it be merged at the last minute in 19.08?
> If this is the only change, that we need to do (i.e. no dynamic allocation),
> then yes it can go.
Would it make sense to keep this change to Arm platforms only as other platforms are not affected?

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Gagandeep Singh]

Hi Thomas,

> 07/08/2019 14:00, David Marchand:
> > On Wed, Aug 7, 2019 at 12:26 PM Gagandeep Singh <g.singh@nxp.com>
> wrote:
> > >
> > > DPDK currently is supporting maximum 3 hugepage,
> > > sizes whereas system can support more than this e.g.
> > > 64K, 2M, 32M and 1G.
> >
> > You can mention ARM platform here, and that this issue starts with
> > kernel 5.2 (and I would try to mention this in the title as well).
> > This is better than an annotation that will be lost.
> >
> >
> > > Having these four hugepage sizes available to use by DPDK,
> > > which is valid in case of '--in-memory' EAL option or
> > > using 4 separate mount points for each hugepage size;
> > > hugepage_info_init() API reports an error.
> >
> > Can you describe what is the impact from a user point of view rather
> > than mentioning this internal function?
> 
> Yes please, we need to understand how much it is critical.

It is critical for stacks like VPP which uses by default in-memory.
There stack would be broken on kernel 5.2 and above.
I will change the description to make it clear. 

> Should we Cc stable@dpdk.org for backport?
yes

> Should it be merged at the last minute in 19.08?
If this is the only change, that we need to do (i.e. no dynamic allocation), then yes it can go.

Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4

[Gagandeep Singh]

> 
> On Wed, Aug 7, 2019 at 12:26 PM Gagandeep Singh <g.singh@nxp.com> wrote:
> >
> > DPDK currently is supporting maximum 3 hugepage,
> > sizes whereas system can support more than this e.g.
> > 64K, 2M, 32M and 1G.
> 
> You can mention ARM platform here, and that this issue starts with
> kernel 5.2 (and I would try to mention this in the title as well).
> This is better than an annotation that will be lost.
> 
> 
> > Having these four hugepage sizes available to use by DPDK,
> > which is valid in case of '--in-memory' EAL option or
> > using 4 separate mount points for each hugepage size;
> > hugepage_info_init() API reports an error.
> 
> Can you describe what is the impact from a user point of view rather
> than mentioning this internal function?
> 
> 
> > This change increases the maximum supported mount points
> > to 4.
> 
> I suppose this fix does the trick for you.
> However, we are in internal structures and I can't think of an impact
> on datapath.
> So we might as well use dynamic allocations rather than just enlarge this array.
> 
> Did you consider this?
Yes, we have thought about it, but that would mean a lot more testing is required for all supported kernel or may be on some stacks as well.
MAX_HUGEPAGE_SIZES is set as a static value 3 since beginning while ARM (or may be some other platforms) is supporting 4 sizes since very long.

The value of this macro has not changed from long. It is just a mismatch between what DPDK is supporting and what underneath hardware is supporting.
This issue is coming now because in kernel 5.2, kernel is making the directories by default (and not taking from the bootargs) for each hugepage sizes.
Here are the possible cases that we are aware of

For 64KB granule, the kernel supports the following huge page sizes:
        2MB     using 32 x 64KB pages which are contiguous
        512MB   using a level 2 block mapping (a pmd_t)
        16GB    using 32 x 512MB block mappings

For a 16KB granule, we have:
        2MB     using 128 x 16KB pages
        32MB    using a level 2 block mapping (a pmd_t)
        1GB     using 32 x 32MB block mappings

For 4KB granule, we have:
        64KB    using 16 x 4KB pages
        2MB     using a level 2 block mapping (a pmd_t)
        32MB    using 16 x level 2 block mappings
        1GB     using a level 1 block mapping (a pud_t)

And using the static value of 4, it should cover all cases.

> 
> 
> --
> David Marchand