Re: [PATCH] dts: Change hugepage runtime config to 2MB Exclusively

[Nicholas Pratte <npratte@iol.unh.edu>]

I will change the default hugepage total back to 256. Much of the
justification for changing the default amount was based on guidelines
in the DPDK documentation. In the system requirements section, an
example is provided demonstrating how to allocate 2GB of 2MB
hugepages. We used this reasoning when setting the default amount as
2560 creates 5GB, or 2GB for each NUMA node, and some additional
wiggle room. Much of this was also based on the Juraj's initial
suggestion of 256 hugepages largely being arbitrary, but he had also
referenced the aforementioned DPDK documentation during discussion of
how many hugepages should be configured by default if nothing is set.
Regardless, our suggestion of changing the default hugepage size was
more of a light suggestion than anything else, so I will just remove
this.

I will also make the changes you suggested for linux_session.py. The
plan I have is to add the parameter back in and hardcode the
2048 hugepage size to continue our 2MB logic, with the understanding
that this can be changed in the future.

On Mon, Apr 8, 2024 at 5:35 AM Morten Brørup <mb@smartsharesystems.com> wrote:
>
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Monday, 8 April 2024 10.11
> >
> > On Sun, Apr 07, 2024 at 12:04:24AM +0200, Morten Brørup wrote:
> > > > From: Patrick Robb [mailto:probb@iol.unh.edu]
> > > > Sent: Saturday, 6 April 2024 21.21
> > > >
> > > > On Sat, Apr 6, 2024 at 4:47 AM Morten Brørup <mb@smartsharesystems.com>
> > > > wrote:
> > > > >
> > > > >
> > > > > This change seems very CPU specific.
> > > > >
> > > > > E.g. in x86 32-bit mode, the hugepage size is 4 MB, not 2 MB.
> > > > >
> > > > > I don't know the recommended hugepage size on other architectures.
> > > > >
> > > >
> > > > Thanks Morten, that's an important insight which we weren't aware of
> > > > when we initially discussed this ticket.
> > > >
> > > > We read on some dpdk docs that 1gb hugepages should be set at boot (I
> > > > think the reason is because that's when you can guarantee there is gbs
> > > > of contiguous available memory), and that after boot, only 2gb
> > > > hugepages should be set. I assume that even for other arches which
> > > > don't support the 2mb pages specifically, we still want to allocate
> > > > hugepages using the smallest page size possible per arch (for the same
> > > > reason).
> > >
> > > Correct; for very large hugepages, they need to be set at boot.
> > > I don't remember why, but that's the way the Linux kernel works.
> > > 2 MB is way below that threshold, and 1 GB is above.
> > >
> > > You can also not set nr_overcommit_hugepages for those very large hugepages,
> > only nr_hugepages.
> > >
> > > >
> > > > So I think we can add some dict which stores the smallest valid
> > > > hugepage size per arch. Then during config init, use the config's arch
> > > > value to determine that size, and set the total hugepages allocation
> > > > based on that size and the hugepages count set in the conf.yaml. Or
> > > > maybe we can store the list of all valid hugepgage sizes per arch
> > > > (which are also valid to be set post boot), allow for a new
> > > > hugepage_size value on the conf.yaml, validate the input at config
> > > > init, and then set according to those values. I prefer the former
> > > > option though as I don't think the added flexibility offered by the
> > > > latter seems important.
> > >
> > > I agree; the former option suffices.
> > >
> > > A tiny detail...
> > > ARM supports multiple (4, I think) different hugepage sizes, where the
> > smallest size is 64 KB.
> > > So you might want to choose another hugepage size than the smallest; but I
> > still agree with your proposed concept of using one specific hugepage size per
> > arch.
> > >
> > > Like x86_64, ARM also supports 2 MB and 1 GB hugepage sizes, and 2 MB
> > hugepages is also the typical default on Linux.
> > >
> > > I don't know which hugepage sizes are supported by other architectures.
> > > It might only be 32-bit x86 that needs a different hugepage size than 2 MB.
> > >
> >
> > Even for 32-bit x86, I think most distros now use PAE mode to allow physical
> > addresses >4GB, so even then 32-bit hugepages are 2MB rather than 4MB.
>
> In order to save you from more work on this patch, we could assume/hope that all architectures support 2 MB hugepages, and drop the discussed per-architecture size dict.
> At least for until the assumption doesn't hold true anymore.
>
> Then I only have two further comments to the patch:
>
> 1. Don't increase from 256 to 2560 hugepages (in dts/conf.yaml), unless there is a reason for it. If there is, please mention the increase and reason in the patch description.
> 2. Don't remove the size parameter from _configure_huge_pages() (in /dts/framework/testbed_model/linux_session.py); it might be useful later.
>