* Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4
2019-08-07 13:28 ` Hemant Agrawal
@ 2019-08-08 7:31 ` Thomas Monjalon
2019-08-12 9:43 ` Burakov, Anatoly
2019-08-08 7:33 ` David Marchand
2019-08-12 9:42 ` Burakov, Anatoly
2 siblings, 1 reply; 19+ messages in thread
From: Thomas Monjalon @ 2019-08-08 7:31 UTC (permalink / raw)
To: Hemant Agrawal, Gagandeep Singh
Cc: dev, David Marchand, Burakov, Anatoly, Olivier Matz,
Andrew Rybchenko, Nipun Gupta, honnappa.nagarahalli,
Steve Capper, jerinj, bruce.richardson, gavin.hu,
konstantin.ananyev, drc
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4
2019-08-08 7:31 ` Thomas Monjalon
@ 2019-08-12 9:43 ` Burakov, Anatoly
2019-08-12 9:49 ` David Marchand
0 siblings, 1 reply; 19+ messages in thread
From: Burakov, Anatoly @ 2019-08-12 9:43 UTC (permalink / raw)
To: Thomas Monjalon, Hemant Agrawal, Gagandeep Singh
Cc: dev, David Marchand, Olivier Matz, Andrew Rybchenko, Nipun Gupta,
honnappa.nagarahalli, Steve Capper, jerinj, bruce.richardson,
gavin.hu, konstantin.ananyev, drc
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4
2019-08-12 9:43 ` Burakov, Anatoly
@ 2019-08-12 9:49 ` David Marchand
2019-08-12 10:01 ` Thomas Monjalon
2019-08-12 10:38 ` Burakov, Anatoly
0 siblings, 2 replies; 19+ messages in thread
From: David Marchand @ 2019-08-12 9:49 UTC (permalink / raw)
To: Burakov, Anatoly
Cc: Thomas Monjalon, Hemant Agrawal, Gagandeep Singh, dev,
Olivier Matz, Andrew Rybchenko, Nipun Gupta,
Honnappa Nagarahalli, Steve Capper, Jerin Jacob Kollanukkaran,
Bruce Richardson, Gavin Hu, Ananyev, Konstantin,
David Christensen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4
2019-08-12 9:49 ` David Marchand
@ 2019-08-12 10:01 ` Thomas Monjalon
2019-08-12 10:38 ` Burakov, Anatoly
1 sibling, 0 replies; 19+ messages in thread
From: Thomas Monjalon @ 2019-08-12 10:01 UTC (permalink / raw)
To: Burakov, Anatoly
Cc: David Marchand, Hemant Agrawal, Gagandeep Singh, dev,
Olivier Matz, Andrew Rybchenko, Nipun Gupta,
Honnappa Nagarahalli, Steve Capper, Jerin Jacob Kollanukkaran,
Bruce Richardson, Gavin Hu, Ananyev, Konstantin,
David Christensen
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4
2019-08-12 9:49 ` David Marchand
2019-08-12 10:01 ` Thomas Monjalon
@ 2019-08-12 10:38 ` Burakov, Anatoly
1 sibling, 0 replies; 19+ messages in thread
From: Burakov, Anatoly @ 2019-08-12 10:38 UTC (permalink / raw)
To: David Marchand
Cc: Thomas Monjalon, Hemant Agrawal, Gagandeep Singh, dev,
Olivier Matz, Andrew Rybchenko, Nipun Gupta,
Honnappa Nagarahalli, Steve Capper, Jerin Jacob Kollanukkaran,
Bruce Richardson, Gavin Hu, Ananyev, Konstantin,
David Christensen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4
2019-08-07 13:28 ` Hemant Agrawal
2019-08-08 7:31 ` Thomas Monjalon
@ 2019-08-08 7:33 ` David Marchand
2019-08-08 9:00 ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
2019-08-08 10:37 ` [dpdk-dev] " Hemant Agrawal
2019-08-12 9:42 ` Burakov, Anatoly
2 siblings, 2 replies; 19+ messages in thread
From: David Marchand @ 2019-08-08 7:33 UTC (permalink / raw)
To: Hemant Agrawal
Cc: Thomas Monjalon, Gagandeep Singh, dev, Burakov, Anatoly,
Olivier Matz, Andrew Rybchenko, Nipun Gupta,
Jerin Jacob Kollanukkaran, Gavin Hu, Bruce Richardson
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [EXT] Re: [PATCH] eal: change max hugepage sizes to 4
2019-08-08 7:33 ` David Marchand
@ 2019-08-08 9:00 ` Jerin Jacob Kollanukkaran
2019-08-08 10:37 ` [dpdk-dev] " Hemant Agrawal
1 sibling, 0 replies; 19+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2019-08-08 9:00 UTC (permalink / raw)
To: David Marchand, Hemant Agrawal
Cc: Thomas Monjalon, Gagandeep Singh, dev, Burakov, Anatoly,
Olivier Matz, Andrew Rybchenko, Nipun Gupta, Gavin Hu,
Bruce Richardson
> -----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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4
2019-08-08 7:33 ` David Marchand
2019-08-08 9:00 ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
@ 2019-08-08 10:37 ` Hemant Agrawal
2019-08-08 12:29 ` Steve Capper
1 sibling, 1 reply; 19+ messages in thread
From: Hemant Agrawal @ 2019-08-08 10:37 UTC (permalink / raw)
To: David Marchand, Steve Capper, Honnappa Nagarahalli
Cc: Thomas Monjalon, Gagandeep Singh, dev, Burakov, Anatoly,
Olivier Matz, Andrew Rybchenko, Nipun Gupta,
Jerin Jacob Kollanukkaran, Gavin Hu, Bruce Richardson
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4
2019-08-08 10:37 ` [dpdk-dev] " Hemant Agrawal
@ 2019-08-08 12:29 ` Steve Capper
2019-08-08 12:39 ` David Marchand
0 siblings, 1 reply; 19+ messages in thread
From: Steve Capper @ 2019-08-08 12:29 UTC (permalink / raw)
To: hemant.agrawal
Cc: David Marchand, Honnappa Nagarahalli, thomas, Gagandeep Singh,
dev, Burakov, Anatoly, Olivier Matz, Andrew Rybchenko,
Nipun.gupta@nxp.com, jerinj, Gavin Hu (Arm Technology China),
Bruce Richardson, nd
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4
2019-08-08 12:29 ` Steve Capper
@ 2019-08-08 12:39 ` David Marchand
0 siblings, 0 replies; 19+ messages in thread
From: David Marchand @ 2019-08-08 12:39 UTC (permalink / raw)
To: Steve Capper
Cc: hemant.agrawal, Honnappa Nagarahalli, thomas, Gagandeep Singh,
dev, Burakov, Anatoly, Olivier Matz, Andrew Rybchenko,
Nipun.gupta@nxp.com, jerinj, Gavin Hu (Arm Technology China),
Bruce Richardson, nd
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: change max hugepage sizes to 4
2019-08-07 13:28 ` Hemant Agrawal
2019-08-08 7:31 ` Thomas Monjalon
2019-08-08 7:33 ` David Marchand
@ 2019-08-12 9:42 ` Burakov, Anatoly
2 siblings, 0 replies; 19+ messages in thread
From: Burakov, Anatoly @ 2019-08-12 9:42 UTC (permalink / raw)
To: Hemant Agrawal, Thomas Monjalon, Gagandeep Singh
Cc: David Marchand, dev, Olivier Matz, Andrew Rybchenko, Nipun Gupta
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
^ permalink raw reply [flat|nested] 19+ messages in thread