dts: Change hugepage runtime config to 2MB Exclusively

  The previous implementation configures and allocates hugepage sizes
based on a system default. This can lead to two problems: overallocation of
hugepages (which may crash the remote host), and configuration of hugepage
sizes that are not recommended during runtime. This new implementation
allows only 2MB hugepage allocation during runtime; any other unique
hugepage size must be configured by the end-user for initializing DTS.

If the amount of 2MB hugepages requested exceeds the amount of 2MB
hugepages already configured on the system, then the system will remount
hugepages to cover the difference. If the amount of hugepages requested is
either less than or equal to the amount already configured on the system,
then nothing is done.

Bugzilla ID: 1370
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/conf.yaml                                |  8 ++++----
 dts/framework/config/__init__.py             |  4 ++--
 dts/framework/config/conf_yaml_schema.json   |  6 +++---
 dts/framework/config/types.py                |  2 +-
 dts/framework/testbed_model/linux_session.py | 21 +++++++++-----------
 5 files changed, 19 insertions(+), 22 deletions(-)
  

Recheck-request: iol-intel-Functional

checking for lab infra failure

On Thu, Apr 4, 2024 at 11:31 AM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> The previous implementation configures and allocates hugepage sizes
> based on a system default. This can lead to two problems: overallocation of
> hugepages (which may crash the remote host), and configuration of hugepage
> sizes that are not recommended during runtime. This new implementation
> allows only 2MB hugepage allocation during runtime; any other unique
> hugepage size must be configured by the end-user for initializing DTS.
>
> If the amount of 2MB hugepages requested exceeds the amount of 2MB
> hugepages already configured on the system, then the system will remount
> hugepages to cover the difference. If the amount of hugepages requested is
> either less than or equal to the amount already configured on the system,
> then nothing is done.
>
> Bugzilla ID: 1370
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>  dts/conf.yaml                                |  8 ++++----
>  dts/framework/config/__init__.py             |  4 ++--
>  dts/framework/config/conf_yaml_schema.json   |  6 +++---
>  dts/framework/config/types.py                |  2 +-
>  dts/framework/testbed_model/linux_session.py | 21 +++++++++-----------
>  5 files changed, 19 insertions(+), 22 deletions(-)
>
> diff --git a/dts/conf.yaml b/dts/conf.yaml
> index 8068345dd5..3690e77ee5 100644
> --- a/dts/conf.yaml
> +++ b/dts/conf.yaml
> @@ -35,8 +35,8 @@ nodes:
>      lcores: "" # use all the available logical cores
>      use_first_core: false # tells DPDK to use any physical core
>      memory_channels: 4 # tells DPDK to use 4 memory channels
> -    hugepages:  # optional; if removed, will use system hugepage configuration
> -        amount: 256
> +    hugepages_2mb: # optional; if removed, will use system hugepage configuration
> +        amount: 2560
>          force_first_numa: false
>      ports:
>        # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
> @@ -71,8 +71,8 @@ nodes:
>          os_driver: rdma
>          peer_node: "SUT 1"
>          peer_pci: "0000:00:08.1"
> -    hugepages:  # optional; if removed, will use system hugepage configuration
> -        amount: 256
> +    hugepages_2mb: # optional; if removed, will use system hugepage configuration
> +        amount: 2560
>          force_first_numa: false
>      traffic_generator:
>          type: SCAPY
> diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
> index 4cb5c74059..b6f820e39e 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -255,8 +255,8 @@ def from_dict(
>              Either an SUT or TG configuration instance.
>          """
>          hugepage_config = None
> -        if "hugepages" in d:
> -            hugepage_config_dict = d["hugepages"]
> +        if "hugepages_2mb" in d:
> +            hugepage_config_dict = d["hugepages_2mb"]
>              if "force_first_numa" not in hugepage_config_dict:
>                  hugepage_config_dict["force_first_numa"] = False
>              hugepage_config = HugepageConfiguration(**hugepage_config_dict)
> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index 4731f4511d..f4d7199523 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -146,7 +146,7 @@
>          "compiler"
>        ]
>      },
> -    "hugepages": {
> +    "hugepages_2mb": {
>        "type": "object",
>        "description": "Optional hugepage configuration. If not specified, hugepages won't be configured and DTS will use system configuration.",
>        "properties": {
> @@ -253,8 +253,8 @@
>              "type": "integer",
>              "description": "How many memory channels to use. Optional, defaults to 1."
>            },
> -          "hugepages": {
> -            "$ref": "#/definitions/hugepages"
> +          "hugepages_2mb": {
> +            "$ref": "#/definitions/hugepages_2mb"
>            },
>            "ports": {
>              "type": "array",
> diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
> index 1927910d88..016e0c3dbd 100644
> --- a/dts/framework/config/types.py
> +++ b/dts/framework/config/types.py
> @@ -46,7 +46,7 @@ class NodeConfigDict(TypedDict):
>      """Allowed keys and values."""
>
>      #:
> -    hugepages: HugepageConfigurationDict
> +    hugepages_2mb: HugepageConfigurationDict
>      #:
>      name: str
>      #:
> diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
> index 5d24030c3d..ad3c811301 100644
> --- a/dts/framework/testbed_model/linux_session.py
> +++ b/dts/framework/testbed_model/linux_session.py
> @@ -15,7 +15,7 @@
>
>  from typing_extensions import NotRequired
>
> -from framework.exception import RemoteCommandExecutionError
> +from framework.exception import ConfigurationError, RemoteCommandExecutionError
>  from framework.utils import expand_range
>
>  from .cpu import LogicalCore
> @@ -87,25 +87,22 @@ def get_dpdk_file_prefix(self, dpdk_prefix: str) -> str:
>      def setup_hugepages(self, hugepage_count: int, force_first_numa: bool) -> None:
>          """Overrides :meth:`~.os_session.OSSession.setup_hugepages`."""
>          self._logger.info("Getting Hugepage information.")
> -        hugepage_size = self._get_hugepage_size()
> +        if "hugepages-2048kB" not in self.send_command("ls /sys/kernel/mm/hugepages").stdout:
> +            raise ConfigurationError("2MB hugepages not supported by operating system")
>          hugepages_total = self._get_hugepages_total()
>          self._numa_nodes = self._get_numa_nodes()
>
> -        if force_first_numa or hugepages_total != hugepage_count:
> +        if force_first_numa or hugepages_total < hugepage_count:
>              # when forcing numa, we need to clear existing hugepages regardless
>              # of size, so they can be moved to the first numa node
> -            self._configure_huge_pages(hugepage_count, hugepage_size, force_first_numa)
> +            self._configure_huge_pages(hugepage_count, force_first_numa)
>          else:
>              self._logger.info("Hugepages already configured.")
>          self._mount_huge_pages()
>
> -    def _get_hugepage_size(self) -> int:
> -        hugepage_size = self.send_command("awk '/Hugepagesize/ {print $2}' /proc/meminfo").stdout
> -        return int(hugepage_size)
> -
>      def _get_hugepages_total(self) -> int:
>          hugepages_total = self.send_command(
> -            "awk '/HugePages_Total/ { print $2 }' /proc/meminfo"
> +            "cat /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages"
>          ).stdout
>          return int(hugepages_total)
>
> @@ -136,15 +133,15 @@ def _supports_numa(self) -> bool:
>          # there's no reason to do any numa specific configuration)
>          return len(self._numa_nodes) > 1
>
> -    def _configure_huge_pages(self, amount: int, size: int, force_first_numa: bool) -> None:
> +    def _configure_huge_pages(self, amount: int, force_first_numa: bool) -> None:
>          self._logger.info("Configuring Hugepages.")
> -        hugepage_config_path = f"/sys/kernel/mm/hugepages/hugepages-{size}kB/nr_hugepages"
> +        hugepage_config_path = "/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages"
>          if force_first_numa and self._supports_numa():
>              # clear non-numa hugepages
>              self.send_command(f"echo 0 | tee {hugepage_config_path}", privileged=True)
>              hugepage_config_path = (
>                  f"/sys/devices/system/node/node{self._numa_nodes[0]}/hugepages"
> -                f"/hugepages-{size}kB/nr_hugepages"
> +                f"/hugepages-2048kB/nr_hugepages"
>              )
>
>          self.send_command(f"echo {amount} | tee {hugepage_config_path}", privileged=True)
> --
> 2.44.0
>
  

> From: Nicholas Pratte [mailto:npratte@iol.unh.edu]
> Sent: Thursday, 4 April 2024 17.31
> 
> The previous implementation configures and allocates hugepage sizes
> based on a system default. This can lead to two problems: overallocation
> of
> hugepages (which may crash the remote host), and configuration of
> hugepage
> sizes that are not recommended during runtime. This new implementation
> allows only 2MB hugepage allocation during runtime; any other unique
> hugepage size must be configured by the end-user for initializing DTS.
> 
> If the amount of 2MB hugepages requested exceeds the amount of 2MB
> hugepages already configured on the system, then the system will remount
> hugepages to cover the difference. If the amount of hugepages requested
> is
> either less than or equal to the amount already configured on the
> system,
> then nothing is done.
> 
> Bugzilla ID: 1370
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---

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.
  

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).

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.
  

> 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.
  

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.

/Bruce
  

> 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.
  

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.
>