From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: Nicholas Pratte <npratte@iol.unh.edu>,
probb@iol.unh.edu, dmarx@iol.unh.edu, jspewock@iol.unh.edu,
luca.vizzarro@arm.com, yoan.picchi@foss.arm.com,
Honnappa.Nagarahalli@arm.com, paul.szczepanek@arm.com
Cc: dev@dpdk.org
Subject: Re: [PATCH v2 2/6] dts: Use First Core Logic Change
Date: Tue, 10 Sep 2024 15:34:09 +0200 [thread overview]
Message-ID: <9b8d2e3c-0008-4ba3-b93e-0a606822757c@pantheon.tech> (raw)
In-Reply-To: <20240705171341.23894-6-npratte@iol.unh.edu>
On 5. 7. 2024 19:13, Nicholas Pratte wrote:
> Removed use_first_core from the conf.yaml in favor of determining this
> within the framework. use_first_core continue to serve a purpose in that
> it is only enabled when core 0 is explicitly provided in the
> configuration. Any other configuration, including "" or "any," will
> omit core 0.
>
> Documentation reworks are included to reflect the changes made.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
> dts/conf.yaml | 3 +--
> dts/framework/config/__init__.py | 11 +++++++----
> dts/framework/config/conf_yaml_schema.json | 6 +-----
> dts/framework/testbed_model/node.py | 9 +++++++++
> 4 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/dts/conf.yaml b/dts/conf.yaml
> index 56cc08ced2..53192e0761 100644
> --- a/dts/conf.yaml
> +++ b/dts/conf.yaml
> @@ -29,8 +29,7 @@ nodes:
> user: dtsuser
> arch: x86_64
> os: linux
> - lcores: "" # use all the available logical cores
> - use_first_core: false # tells DPDK to use any physical core
> + lcores: "" # use all available logical cores (Skips first core)
The message in parentheses could be confusing, let's make it explicit
that an empty string skips first core.
And the comments in this file are all lowercase so let's do that in
parentheses as well.
> memory_channels: 4 # tells DPDK to use 4 memory channels
> hugepages_2mb: # optional; if removed, will use system hugepage configuration
> number_of: 256
> diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
> index 456a8a83ab..4c05373ef3 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -246,6 +246,9 @@ def from_dict(
> hugepage_config_dict["force_first_numa"] = False
> hugepage_config = HugepageConfiguration(**hugepage_config_dict)
>
> + lcores = "1" if "lcores" not in d else d["lcores"] if "any" not in d["lcores"] else ""
> + use_first_core = "0" in lcores
> +
I'm wondering whether we want to do this in config. The logic seems to
be better placed elsewhere. My thinking is this:
1. Removing use_first_core from Node._get_remote_cpus() (and the
downstream OSSession methods) makes sense since the method really looks
like it should be getting all remote CPUs. A benefit here is the first
core filtering doesn't happen in OSSession where it currently is - it
just doesn't make sense there.
2. Putting use_first_core to LogicalCoreList also doesn't make much
sense, it's supposed to be a list and not supposed to do any filtering.
3. The two above are used to compose the final list of usable cores on a
node. We could either filter the first core from 1. after getting the
cores, we could filter it from 2. after getting the core list or filter
it after using 1. and 2. with LogicalCoreListFilter().
Or in other words:
remote_lcores = self.main_session.get_remote_cpus()
lcore_list_config = LogicalCoreList(self.config.lcores)
self.lcores = LogicalCoreListFilter(remote_lcores,
lcore_list_config).filter()
We can either remove the first core from remote_lcores,
lcore_list_config or from self.lcores. The result is the same if we
remove it from any of these, so maybe we just work with self.lcores as
that is also what you've used in the patch to issue the warning.
The logic of whether to remove the core or not would be in Node (if
self.config.lcores is either "" or "any").
> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index 3f7bc2acae..01a6afdc72 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -163,13 +163,9 @@
> },
> "lcores": {
> "type": "string",
> - "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$",
> + "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
This should be added to the description.
> "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
> },
> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
> index 12a40170ac..9b3f01f1e9 100644
> --- a/dts/framework/testbed_model/node.py
> +++ b/dts/framework/testbed_model/node.py
> @@ -86,6 +86,15 @@ def __init__(self, node_config: NodeConfiguration):
> self.lcores, LogicalCoreList(self.config.lcores)
> ).filter()
>
> + if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
> + self._logger.info(
If this is a warning, then we should call self._logger.warning()
> + """
> + WARNING: First core being used;
> + using the first core is considered risky and should only
> + be done by advanced users.
> + """
> + )
> +
> self._other_sessions = []
> self._init_ports()
>
next prev parent reply other threads:[~2024-09-10 13:34 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
2024-06-13 20:18 ` [PATCH 1/4] dts: Remove build target config and list of devices Nicholas Pratte
2024-06-14 18:07 ` Jeremy Spewock
2024-06-13 20:18 ` [PATCH 2/4] dts: Use First Core Logic Change Nicholas Pratte
2024-06-14 18:09 ` Jeremy Spewock
2024-06-20 13:41 ` Nicholas Pratte
2024-06-13 20:18 ` [PATCH 3/4] dts: Self-Discovering Architecture Change Nicholas Pratte
2024-06-14 18:09 ` Jeremy Spewock
2024-06-13 20:18 ` [PATCH 4/4] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
2024-06-14 18:11 ` Jeremy Spewock
2024-07-05 17:13 ` [PATCH v2 0/6] dts: Remove Excess Attributes From User Config Nicholas Pratte
2024-07-05 18:29 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
2024-07-05 18:31 ` [PATCH v2 2/6] dts: Use First Core Logic Change Nicholas Pratte
2024-07-05 18:32 ` [PATCH v2 3/6] dts: Self-Discovering Architecture Change Nicholas Pratte
2024-07-05 18:32 ` [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
2024-07-05 18:33 ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
2024-07-05 18:33 ` [PATCH v2 6/6] doc: dpdk documentation changes for new dts config Nicholas Pratte
2024-07-05 17:13 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
2024-07-16 15:07 ` Jeremy Spewock
2024-09-12 20:33 ` Nicholas Pratte
2024-09-10 11:30 ` Juraj Linkeš
2024-09-12 20:31 ` Nicholas Pratte
2024-07-05 17:13 ` [PATCH v2 2/6] dts: Use First Core Logic Change Nicholas Pratte
2024-09-10 13:34 ` Juraj Linkeš [this message]
2024-07-05 17:13 ` [PATCH v2 3/6] dts: Self-Discovering Architecture Change Nicholas Pratte
2024-09-10 13:41 ` Juraj Linkeš
2024-07-05 17:13 ` [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
2024-09-10 14:04 ` Juraj Linkeš
2024-07-05 17:13 ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
2024-07-16 14:59 ` Jeremy Spewock
2024-09-10 14:12 ` Juraj Linkeš
2024-07-05 17:13 ` [PATCH v2 6/6] doc: dpdk documentation changes for new dts config Nicholas Pratte
2024-09-10 14:17 ` Juraj Linkeš
2024-07-05 18:24 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9b8d2e3c-0008-4ba3-b93e-0a606822757c@pantheon.tech \
--to=juraj.linkes@pantheon.tech \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=dev@dpdk.org \
--cc=dmarx@iol.unh.edu \
--cc=jspewock@iol.unh.edu \
--cc=luca.vizzarro@arm.com \
--cc=npratte@iol.unh.edu \
--cc=paul.szczepanek@arm.com \
--cc=probb@iol.unh.edu \
--cc=yoan.picchi@foss.arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).