From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9F0CD43391; Tue, 21 Nov 2023 18:45:15 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7728840DCD; Tue, 21 Nov 2023 18:45:15 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 140CB40633 for ; Tue, 21 Nov 2023 18:45:14 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6022F1042; Tue, 21 Nov 2023 09:46:00 -0800 (PST) Received: from [10.1.34.179] (e125442.arm.com [10.1.34.179]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CA1BC3F6C4; Tue, 21 Nov 2023 09:45:11 -0800 (PST) Message-ID: <927d9e84-5583-4516-8a0e-bff52470406a@foss.arm.com> Date: Tue, 21 Nov 2023 17:45:10 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v7 14/21] dts: cpu docstring update Content-Language: en-US To: =?UTF-8?Q?Juraj_Linke=C5=A1?= , thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com Cc: dev@dpdk.org References: <20231108125324.191005-23-juraj.linkes@pantheon.tech> <20231115130959.39420-1-juraj.linkes@pantheon.tech> <20231115130959.39420-15-juraj.linkes@pantheon.tech> From: Yoan Picchi In-Reply-To: <20231115130959.39420-15-juraj.linkes@pantheon.tech> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 11/15/23 13:09, Juraj Linkeš wrote: > Format according to the Google format and PEP257, with slight > deviations. > > Signed-off-by: Juraj Linkeš > --- > dts/framework/testbed_model/cpu.py | 196 +++++++++++++++++++++-------- > 1 file changed, 144 insertions(+), 52 deletions(-) > > diff --git a/dts/framework/testbed_model/cpu.py b/dts/framework/testbed_model/cpu.py > index 8fe785dfe4..4edeb4a7c2 100644 > --- a/dts/framework/testbed_model/cpu.py > +++ b/dts/framework/testbed_model/cpu.py > @@ -1,6 +1,22 @@ > # SPDX-License-Identifier: BSD-3-Clause > # Copyright(c) 2023 PANTHEON.tech s.r.o. > > +"""CPU core representation and filtering. > + > +This module provides a unified representation of logical CPU cores along > +with filtering capabilities. > + > +When symmetric multiprocessing (SMP or multithreading) is enabled on a server, > +the physical CPU cores are split into logical CPU cores with different IDs. > + > +:class:`LogicalCoreCountFilter` filters by the number of logical cores. It's possible to specify > +the socket from which to filter the number of logical cores. It's also possible to not use all > +logical CPU cores from each physical core (e.g. only the first logical core of each physical core). > + > +:class:`LogicalCoreListFilter` filters by logical core IDs. This mostly checks that > +the logical cores are actually present on the server. > +""" > + > import dataclasses > from abc import ABC, abstractmethod > from collections.abc import Iterable, ValuesView > @@ -11,9 +27,17 @@ > > @dataclass(slots=True, frozen=True) > class LogicalCore(object): > - """ > - Representation of a CPU core. A physical core is represented in OS > - by multiple logical cores (lcores) if CPU multithreading is enabled. > + """Representation of a logical CPU core. > + > + A physical core is represented in OS by multiple logical cores (lcores) > + if CPU multithreading is enabled. When multithreading is disabled, their IDs are the same. > + > + Attributes: > + lcore: The logical core ID of a CPU core. It's the same as `core` with > + disabled multithreading. > + core: The physical core ID of a CPU core. > + socket: The physical socket ID where the CPU resides. > + node: The NUMA node ID where the CPU resides. > """ > > lcore: int > @@ -22,27 +46,36 @@ class LogicalCore(object): > node: int > > def __int__(self) -> int: > + """The CPU is best represented by the logical core, as that's what we configure in EAL.""" > return self.lcore > > > class LogicalCoreList(object): > - """ > - Convert these options into a list of logical core ids. > - lcore_list=[LogicalCore1, LogicalCore2] - a list of LogicalCores > - lcore_list=[0,1,2,3] - a list of int indices > - lcore_list=['0','1','2-3'] - a list of str indices; ranges are supported > - lcore_list='0,1,2-3' - a comma delimited str of indices; ranges are supported > - > - The class creates a unified format used across the framework and allows > - the user to use either a str representation (using str(instance) or directly > - in f-strings) or a list representation (by accessing instance.lcore_list). > - Empty lcore_list is allowed. > + r"""A unified way to store :class:`LogicalCore`\s. > + > + Create a unified format used across the framework and allow the user to use > + either a :class:`str` representation (using ``str(instance)`` or directly in f-strings) > + or a :class:`list` representation (by accessing the `lcore_list` property, > + which stores logical core IDs). > """ > > _lcore_list: list[int] > _lcore_str: str > > def __init__(self, lcore_list: list[int] | list[str] | list[LogicalCore] | str): > + """Process `lcore_list`, then sort. > + > + There are four supported logical core list formats:: > + > + lcore_list=[LogicalCore1, LogicalCore2] # a list of LogicalCores > + lcore_list=[0,1,2,3] # a list of int indices > + lcore_list=['0','1','2-3'] # a list of str indices; ranges are supported > + lcore_list='0,1,2-3' # a comma delimited str of indices; ranges are supported > + > + Args: > + lcore_list: Various ways to represent multiple logical cores. > + Empty `lcore_list` is allowed. > + """ > self._lcore_list = [] > if isinstance(lcore_list, str): > lcore_list = lcore_list.split(",") > @@ -60,6 +93,7 @@ def __init__(self, lcore_list: list[int] | list[str] | list[LogicalCore] | str): > > @property > def lcore_list(self) -> list[int]: > + """The logical core IDs.""" > return self._lcore_list > > def _get_consecutive_lcores_range(self, lcore_ids_list: list[int]) -> list[str]: > @@ -89,28 +123,30 @@ def _get_consecutive_lcores_range(self, lcore_ids_list: list[int]) -> list[str]: > return formatted_core_list > > def __str__(self) -> str: > + """The consecutive ranges of logical core IDs.""" > return self._lcore_str > > > @dataclasses.dataclass(slots=True, frozen=True) > class LogicalCoreCount(object): > - """ > - Define the number of logical cores to use. > - If sockets is not None, socket_count is ignored. > - """ > + """Define the number of logical cores per physical cores per sockets.""" > > + #: Use this many logical cores per each physical core. > lcores_per_core: int = 1 > + #: Use this many physical cores per each socket. > cores_per_socket: int = 2 > + #: Use this many sockets. > socket_count: int = 1 > + #: Use exactly these sockets. This takes precedence over `socket_count`, > + #: so when `sockets` is not :data:`None`, `socket_count` is ignored. > sockets: list[int] | None = None > > > class LogicalCoreFilter(ABC): > - """ > - Filter according to the input filter specifier. Each filter needs to be > - implemented in a derived class. > - This class only implements operations common to all filters, such as sorting > - the list to be filtered beforehand. > + """Common filtering class. > + > + Each filter needs to be implemented in a subclass. This base class sorts the list of cores > + and defines the filtering method, which must be implemented by subclasses. > """ > > _filter_specifier: LogicalCoreCount | LogicalCoreList > @@ -122,6 +158,17 @@ def __init__( > filter_specifier: LogicalCoreCount | LogicalCoreList, > ascending: bool = True, > ): > + """Filter according to the input filter specifier. > + > + The input `lcore_list` is copied and sorted by physical core before filtering. > + The list is copied so that the original is left intact. > + > + Args: > + lcore_list: The logical CPU cores to filter. > + filter_specifier: Filter cores from `lcore_list` according to this filter. > + ascending: Sort cores in ascending order (lowest to highest IDs). If data:`False`, > + sort in descending order. > + """ > self._filter_specifier = filter_specifier > > # sorting by core is needed in case hyperthreading is enabled > @@ -132,31 +179,45 @@ def __init__( > > @abstractmethod > def filter(self) -> list[LogicalCore]: > - """ > - Use self._filter_specifier to filter self._lcores_to_filter > - and return the list of filtered LogicalCores. > - self._lcores_to_filter is a sorted copy of the original list, > - so it may be modified. > + r"""Filter the cores. > + > + Use `self._filter_specifier` to filter `self._lcores_to_filter` and return > + the filtered :class:`LogicalCore`\s. > + `self._lcores_to_filter` is a sorted copy of the original list, so it may be modified. > + > + Returns: > + The filtered cores. > """ > > > class LogicalCoreCountFilter(LogicalCoreFilter): > - """ > + """Filter cores by specified counts. > + > Filter the input list of LogicalCores according to specified rules: > - Use cores from the specified number of sockets or from the specified socket ids. > - If sockets is specified, it takes precedence over socket_count. > - From each of those sockets, use only cores_per_socket of cores. > - And for each core, use lcores_per_core of logical cores. Hypertheading > - must be enabled for this to take effect. > - If ascending is True, use cores with the lowest numerical id first > - and continue in ascending order. If False, start with the highest > - id and continue in descending order. This ordering affects which > - sockets to consider first as well. > + > + * The input `filter_specifier` is :class:`LogicalCoreCount`, > + * Use cores from the specified number of sockets or from the specified socket ids, > + * If `sockets` is specified, it takes precedence over `socket_count`, > + * From each of those sockets, use only `cores_per_socket` of cores, > + * And for each core, use `lcores_per_core` of logical cores. Hypertheading > + must be enabled for this to take effect. > """ > > _filter_specifier: LogicalCoreCount > > def filter(self) -> list[LogicalCore]: > + """Filter the cores according to :class:`LogicalCoreCount`. > + > + Start by filtering the allowed sockets. The cores matching the allowed socket are returned. allowed socket*s* > + The cores of each socket are stored in separate lists. > + > + Then filter the allowed physical cores from those lists of cores per socket. When filtering > + physical cores, store the desired number of logical cores per physical core which then > + together constitute the final filtered list. > + > + Returns: > + The filtered cores. > + """ > sockets_to_filter = self._filter_sockets(self._lcores_to_filter) > filtered_lcores = [] > for socket_to_filter in sockets_to_filter: > @@ -166,24 +227,37 @@ def filter(self) -> list[LogicalCore]: > def _filter_sockets( > self, lcores_to_filter: Iterable[LogicalCore] > ) -> ValuesView[list[LogicalCore]]: > - """ > - Remove all lcores that don't match the specified socket(s). > - If self._filter_specifier.sockets is not None, keep lcores from those sockets, > - otherwise keep lcores from the first > - self._filter_specifier.socket_count sockets. > + """Filter a list of cores per each allowed socket. > + > + The sockets may be specified in two ways, either a number or a specific list of sockets. > + In case of a specific list, we just need to return the cores from those sockets. > + If filtering a number of cores, we need to go through all cores and note which sockets > + appear and only filter from the first n that appear. > + > + Args: > + lcores_to_filter: The cores to filter. These must be sorted by the physical core. > + > + Returns: > + A list of lists of logical CPU cores. Each list contains cores from one socket. > """ > allowed_sockets: set[int] = set() > socket_count = self._filter_specifier.socket_count > if self._filter_specifier.sockets: > + # when sockets in filter is specified, the sockets are already set > socket_count = len(self._filter_specifier.sockets) > allowed_sockets = set(self._filter_specifier.sockets) > > + # filter socket_count sockets from all sockets by checking the socket of each CPU > filtered_lcores: dict[int, list[LogicalCore]] = {} > for lcore in lcores_to_filter: > if not self._filter_specifier.sockets: > + # this is when sockets is not set, so we do the actual filtering > + # when it is set, allowed_sockets is already defined and can't be changed > if len(allowed_sockets) < socket_count: > + # allowed_sockets is a set, so adding an existing socket won't re-add it > allowed_sockets.add(lcore.socket) > if lcore.socket in allowed_sockets: > + # separate sockets per socket; this makes it easier in further processing socket*s* per socket ? > if lcore.socket in filtered_lcores: > filtered_lcores[lcore.socket].append(lcore) > else: > @@ -200,12 +274,13 @@ def _filter_sockets( > def _filter_cores_from_socket( > self, lcores_to_filter: Iterable[LogicalCore] > ) -> list[LogicalCore]: > - """ > - Keep only the first self._filter_specifier.cores_per_socket cores. > - In multithreaded environments, keep only > - the first self._filter_specifier.lcores_per_core lcores of those cores. > - """ > + """Filter a list of cores from the given socket. > + > + Go through the cores and note how many logical cores per physical core have been filtered. > > + Returns: > + The filtered logical CPU cores. > + """ > # no need to use ordered dict, from Python3.7 the dict > # insertion order is preserved (LIFO). > lcore_count_per_core_map: dict[int, int] = {} > @@ -248,15 +323,21 @@ def _filter_cores_from_socket( > > > class LogicalCoreListFilter(LogicalCoreFilter): > - """ > - Filter the input list of Logical Cores according to the input list of > - lcore indices. > - An empty LogicalCoreList won't filter anything. > + """Filter the logical CPU cores by logical CPU core IDs. > + > + This is a simple filter that looks at logical CPU IDs and only filter those that match. > + > + The input filter is :class:`LogicalCoreList`. An empty LogicalCoreList won't filter anything. > """ > > _filter_specifier: LogicalCoreList > > def filter(self) -> list[LogicalCore]: > + """Filter based on logical CPU core ID. > + > + Return: > + The filtered logical CPU cores. > + """ > if not len(self._filter_specifier.lcore_list): > return self._lcores_to_filter > > @@ -279,6 +360,17 @@ def lcore_filter( > filter_specifier: LogicalCoreCount | LogicalCoreList, > ascending: bool, > ) -> LogicalCoreFilter: > + """Factory for using the right filter with `filter_specifier`. > + > + Args: > + core_list: The logical CPU cores to filter. > + filter_specifier: The filter to use. > + ascending: Sort cores in ascending order (lowest to highest IDs). If :data:`False`, > + sort in descending order. > + > + Returns: > + The filter matching `filter_specifier`. > + """ > if isinstance(filter_specifier, LogicalCoreList): > return LogicalCoreListFilter(core_list, filter_specifier, ascending) > elif isinstance(filter_specifier, LogicalCoreCount):