[v9,2/2] dts: Change hugepage 'amount' to a different term

Message ID 20240610152048.23617-6-npratte@iol.unh.edu (mailing list archive)
State Accepted
Delegated to: Thomas Monjalon
Headers
Series Methodology change for hugepage configuration |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/github-robot: build fail github build: failed
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS

Commit Message

Nicholas Pratte June 10, 2024, 3:20 p.m. UTC
  The term 'amount' is used for uncountable nouns. Since total hugepages
is a discrete value (i.e. countable), the declaration of the 'amount'
key value pair should be changes to a different term in both the config
and the rest of the code.

Bugzilla ID: 1370

Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/conf.yaml                                |  4 ++--
 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 | 12 +++++-------
 dts/framework/testbed_model/node.py          |  2 +-
 dts/framework/testbed_model/os_session.py    |  8 +++-----
 7 files changed, 17 insertions(+), 21 deletions(-)
  

Comments

Juraj Linkeš June 10, 2024, 3:26 p.m. UTC | #1
On 10. 6. 2024 17:20, Nicholas Pratte wrote:
> The term 'amount' is used for uncountable nouns. Since total hugepages
> is a discrete value (i.e. countable), the declaration of the 'amount'
> key value pair should be changes to a different term in both the config
> and the rest of the code.
> 
> Bugzilla ID: 1370
> 
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>

Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
  
Luca Vizzarro June 17, 2024, 3:08 p.m. UTC | #2
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
  

Patch

diff --git a/dts/conf.yaml b/dts/conf.yaml
index 56c3ae6f4c..ce475776a7 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -36,7 +36,7 @@  nodes:
     use_first_core: false # tells DPDK to use any physical core
     memory_channels: 4 # tells DPDK to use 4 memory channels
     hugepages_2mb: # optional; if removed, will use system hugepage configuration
-        amount: 256
+        number_of: 256
         force_first_numa: false
     ports:
       # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
@@ -72,7 +72,7 @@  nodes:
         peer_node: "SUT 1"
         peer_pci: "0000:00:08.1"
     hugepages_2mb: # optional; if removed, will use system hugepage configuration
-        amount: 256
+        number_of: 256
         force_first_numa: false
     traffic_generator:
         type: SCAPY
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index b6f820e39e..81dd321b18 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -127,11 +127,11 @@  class HugepageConfiguration:
     r"""The hugepage configuration of :class:`~framework.testbed_model.node.Node`\s.
 
     Attributes:
-        amount: The number of hugepages.
+        number_of: The number of hugepages to allocate.
         force_first_numa: If :data:`True`, the hugepages will be configured on the first NUMA node.
     """
 
-    amount: int
+    number_of: int
     force_first_numa: bool
 
 
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f4d7199523..71c893662a 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -150,9 +150,9 @@ 
       "type": "object",
       "description": "Optional hugepage configuration. If not specified, hugepages won't be configured and DTS will use system configuration.",
       "properties": {
-        "amount": {
+        "number_of": {
           "type": "integer",
-          "description": "The amount of hugepages to configure. Hugepage size will be the system default."
+          "description": "The number of hugepages to configure. Hugepage size will be the system default."
         },
         "force_first_numa": {
           "type": "boolean",
@@ -161,7 +161,7 @@ 
       },
       "additionalProperties": false,
       "required": [
-        "amount"
+        "number_of"
       ]
     },
     "mac_address": {
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index 016e0c3dbd..5c9d659901 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -37,7 +37,7 @@  class HugepageConfigurationDict(TypedDict):
     """Allowed keys and values."""
 
     #:
-    amount: int
+    number_of: int
     #:
     force_first_numa: bool
 
diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
index d0f7cfa77c..99abc21353 100644
--- a/dts/framework/testbed_model/linux_session.py
+++ b/dts/framework/testbed_model/linux_session.py
@@ -84,9 +84,7 @@  def get_dpdk_file_prefix(self, dpdk_prefix: str) -> str:
         """Overrides :meth:`~.os_session.OSSession.get_dpdk_file_prefix`."""
         return dpdk_prefix
 
-    def setup_hugepages(
-        self, hugepage_count: int, hugepage_size: int, force_first_numa: bool
-    ) -> None:
+    def setup_hugepages(self, number_of: int, hugepage_size: int, force_first_numa: bool) -> None:
         """Overrides :meth:`~.os_session.OSSession.setup_hugepages`."""
         self._logger.info("Getting Hugepage information.")
         hugepages_total = self._get_hugepages_total(hugepage_size)
@@ -97,10 +95,10 @@  def setup_hugepages(
             raise ConfigurationError("hugepage size not supported by operating system")
         self._numa_nodes = self._get_numa_nodes()
 
-        if force_first_numa or hugepages_total < hugepage_count:
+        if force_first_numa or hugepages_total < number_of:
             # 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(number_of, hugepage_size, force_first_numa)
         else:
             self._logger.info("Hugepages already configured.")
         self._mount_huge_pages()
@@ -138,7 +136,7 @@  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, number_of: int, size: int, force_first_numa: bool) -> None:
         self._logger.info("Configuring Hugepages.")
         hugepage_config_path = f"/sys/kernel/mm/hugepages/hugepages-{size}kB/nr_hugepages"
         if force_first_numa and self._supports_numa():
@@ -149,7 +147,7 @@  def _configure_huge_pages(self, amount: int, size: int, force_first_numa: bool)
                 f"/hugepages-{size}kB/nr_hugepages"
             )
 
-        self.send_command(f"echo {amount} | tee {hugepage_config_path}", privileged=True)
+        self.send_command(f"echo {number_of} | tee {hugepage_config_path}", privileged=True)
 
     def update_ports(self, ports: list[Port]) -> None:
         """Overrides :meth:`~.os_session.OSSession.update_ports`."""
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 3f2a727c3b..6818a057d0 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -266,7 +266,7 @@  def _setup_hugepages(self) -> None:
         """
         if self.config.hugepages:
             self.main_session.setup_hugepages(
-                self.config.hugepages.amount,
+                self.config.hugepages.number_of,
                 self.main_session.hugepage_size,
                 self.config.hugepages.force_first_numa,
             )
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index 19dcf5f963..bba4aca9ce 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -347,18 +347,16 @@  def get_dpdk_file_prefix(self, dpdk_prefix: str) -> str:
         """
 
     @abstractmethod
-    def setup_hugepages(
-        self, hugepage_count: int, hugepage_size: int, force_first_numa: bool
-    ) -> None:
+    def setup_hugepages(self, number_of: int, hugepage_size: int, force_first_numa: bool) -> None:
         """Configure hugepages on the node.
 
         Get the node's Hugepage Size, configure the specified count of hugepages
         if needed and mount the hugepages if needed.
 
         Args:
-            hugepage_count: Configure this many hugepages.
+            number_of: Configure this many hugepages.
             hugepage_size: Configure hugepages of this size.
-            force_first_numa:  If :data:`True`, configure hugepages just on the first numa node.
+            force_first_numa:  If :data:`True`, configure just on the first numa node.
         """
 
     @abstractmethod