From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id A89A54339B;
	Wed, 22 Nov 2023 12:18:38 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 482FD4028C;
	Wed, 22 Nov 2023 12:18:38 +0100 (CET)
Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com
 [209.85.208.42]) by mails.dpdk.org (Postfix) with ESMTP id 81A4540273
 for <dev@dpdk.org>; Wed, 22 Nov 2023 12:18:37 +0100 (CET)
Received: by mail-ed1-f42.google.com with SMTP id
 4fb4d7f45d1cf-549070a04baso2199761a12.3
 for <dev@dpdk.org>; Wed, 22 Nov 2023 03:18:37 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=pantheon.tech; s=google; t=1700651917; x=1701256717; darn=dpdk.org;
 h=content-transfer-encoding:cc:to:subject:message-id:date:from
 :in-reply-to:references:mime-version:from:to:cc:subject:date
 :message-id:reply-to;
 bh=H89pdyIl/ImlrIQRm4Bs/hnV3skMIJYae9X9cglb4tU=;
 b=Lgf0TodKSvvSf5LURNseSxvlmN41kr4aqjNJjqqP7NOSNDpXtTONVB8y/htxA0KSaP
 TKRT/NQRCkieZOyD44D3jdrJrvIRrAtGoIPnVUvIG7caf2Qnou1b+V3bhxb6QP6XZTQg
 r5ubs4gN78x/But6ZsqfUm/Ewmcu6afAEgz2DmDTp9hiJBVVz/YAMA/CwtMFBi01ed8y
 IwFGicwXiUoZK2AOI9WajjLNzXIlWRd/u7vTdcPN06iKKAsZ8qKzl+V9VpjK19OGU8iK
 GBHBBmacQ+vjdcwHOWrUgiDViUnEf7AaaCdHvwB8jGLp1/3VCQ9TwqNDeQonCVNAHr0w
 tP9g==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1700651917; x=1701256717;
 h=content-transfer-encoding:cc:to:subject:message-id:date:from
 :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc
 :subject:date:message-id:reply-to;
 bh=H89pdyIl/ImlrIQRm4Bs/hnV3skMIJYae9X9cglb4tU=;
 b=gk6jRHb7ltJDcCK1+VN0PB1OLWGOsokt4EctoBBsC9vJ5ouUL0IWkye3uWyUr3rNol
 y9hPVCDNu+DuY8HyVMbnJIvDd12l1i/VCLrONdNE5TsXMPsbkbIfthuKn2l/uGGc3GrS
 fs51arDj0lAw8v+7lbBXwH4H978h2CdONlhYFXCOLjay8GwMPKFp5JPGx+MN3YzfRCNf
 rUfku8cV5dJ3jwyE8Pudu7RJuas4POJiXsYmlpwcpZdTdik5lGLXxw2OUcFWHUWpquZE
 O0JTH3z6BQHVER3gH3H6YTluOpNthDpZ3a1XrRrRibB15XEO0OLExN3NgVmJTBbuD7II
 stdw==
X-Gm-Message-State: AOJu0YwcA0vkDgYYannQ8UaOJ2qv/4remhPm4pEJ/fgb6DQXWROL6EeI
 2llMWPvvAMSG0rEIfvy53BMqHO5MAc3lkPkBny6N/g==
X-Google-Smtp-Source: AGHT+IE+k+XJbtsse3IvtWksGE0O0pIRIGZULxLM3H2vAqpUUiLoEiVuJt5pmtuET6EimTM/TBELr6O1b6IzdWZHcKw=
X-Received: by 2002:a17:906:f6d3:b0:a01:bc90:736d with SMTP id
 jo19-20020a170906f6d300b00a01bc90736dmr1358825ejb.40.1700651917055; Wed, 22
 Nov 2023 03:18:37 -0800 (PST)
MIME-Version: 1.0
References: <20231108125324.191005-23-juraj.linkes@pantheon.tech>
 <20231115130959.39420-1-juraj.linkes@pantheon.tech>
 <20231115130959.39420-15-juraj.linkes@pantheon.tech>
 <927d9e84-5583-4516-8a0e-bff52470406a@foss.arm.com>
In-Reply-To: <927d9e84-5583-4516-8a0e-bff52470406a@foss.arm.com>
From: =?UTF-8?Q?Juraj_Linke=C5=A1?= <juraj.linkes@pantheon.tech>
Date: Wed, 22 Nov 2023 12:18:26 +0100
Message-ID: <CAOb5WZZqJ0MaufsC8bh_ZhehR5DZ4OHS_FpVhiDgjW9Y+jQBuQ@mail.gmail.com>
Subject: Re: [PATCH v7 14/21] dts: cpu docstring update
To: Yoan Picchi <yoan.picchi@foss.arm.com>
Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, 
 probb@iol.unh.edu, paul.szczepanek@arm.com, dev@dpdk.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

On Tue, Nov 21, 2023 at 6:45=E2=80=AFPM Yoan Picchi <yoan.picchi@foss.arm.c=
om> wrote:
>
> On 11/15/23 13:09, Juraj Linke=C5=A1 wrote:
> > Format according to the Google format and PEP257, with slight
> > deviations.
> >
> > Signed-off-by: Juraj Linke=C5=A1 <juraj.linkes@pantheon.tech>
> > ---
> >   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 alo=
ng
> > +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 mostl=
y 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=3DTrue, frozen=3DTrue)
> >   class LogicalCore(object):
> > -    """
> > -    Representation of a CPU core. A physical core is represented in OS
> > -    by multiple logical cores (lcores) if CPU multithreading is enable=
d.
> > +    """Representation of a logical CPU core.
> > +
> > +    A physical core is represented in OS by multiple logical cores (lc=
ores)
> > +    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 `co=
re` 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=3D[LogicalCore1, LogicalCore2] - a list of LogicalCores
> > -    lcore_list=3D[0,1,2,3] - a list of int indices
> > -    lcore_list=3D['0','1','2-3'] - a list of str indices; ranges are s=
upported
> > -    lcore_list=3D'0,1,2-3' - a comma delimited str of indices; ranges =
are supported
> > -
> > -    The class creates a unified format used across the framework and a=
llows
> > -    the user to use either a str representation (using str(instance) o=
r directly
> > -    in f-strings) or a list representation (by accessing instance.lcor=
e_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 us=
er to use
> > +    either a :class:`str` representation (using ``str(instance)`` or d=
irectly in f-strings)
> > +    or a :class:`list` representation (by accessing the `lcore_list` p=
roperty,
> > +    which stores logical core IDs).
> >       """
> >
> >       _lcore_list: list[int]
> >       _lcore_str: str
> >
> >       def __init__(self, lcore_list: list[int] | list[str] | list[Logic=
alCore] | str):
> > +        """Process `lcore_list`, then sort.
> > +
> > +        There are four supported logical core list formats::
> > +
> > +            lcore_list=3D[LogicalCore1, LogicalCore2]  # a list of Log=
icalCores
> > +            lcore_list=3D[0,1,2,3]        # a list of int indices
> > +            lcore_list=3D['0','1','2-3']  # a list of str indices; ran=
ges are supported
> > +            lcore_list=3D'0,1,2-3'        # a comma delimited str of i=
ndices; ranges are supported
> > +
> > +        Args:
> > +            lcore_list: Various ways to represent multiple logical cor=
es.
> > +                Empty `lcore_list` is allowed.
> > +        """
> >           self._lcore_list =3D []
> >           if isinstance(lcore_list, str):
> >               lcore_list =3D 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=3DTrue, frozen=3DTrue)
> >   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 socke=
ts."""
> >
> > +    #: Use this many logical cores per each physical core.
> >       lcores_per_core: int =3D 1
> > +    #: Use this many physical cores per each socket.
> >       cores_per_socket: int =3D 2
> > +    #: Use this many sockets.
> >       socket_count: int =3D 1
> > +    #: Use exactly these sockets. This takes precedence over `socket_c=
ount`,
> > +    #: so when `sockets` is not :data:`None`, `socket_count` is ignore=
d.
> >       sockets: list[int] | None =3D 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 sub=
classes.
> >       """
> >
> >       _filter_specifier: LogicalCoreCount | LogicalCoreList
> > @@ -122,6 +158,17 @@ def __init__(
> >           filter_specifier: LogicalCoreCount | LogicalCoreList,
> >           ascending: bool =3D True,
> >       ):
> > +        """Filter according to the input filter specifier.
> > +
> > +        The input `lcore_list` is copied and sorted by physical core b=
efore 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 highes=
t IDs). If data:`False`,
> > +                sort in descending order.
> > +        """
> >           self._filter_specifier =3D 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 rule=
s:
> > -    Use cores from the specified number of sockets or from the specifi=
ed 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. Hyperthea=
ding
> > -    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 s=
pecified socket ids,
> > +        * If `sockets` is specified, it takes precedence over `socket_=
count`,
> > +        * From each of those sockets, use only `cores_per_socket` of c=
ores,
> > +        * And for each core, use `lcores_per_core` of logical cores. H=
ypertheading
> > +          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*
>

Ack.

> > +        The cores of each socket are stored in separate lists.
> > +
> > +        Then filter the allowed physical cores from those lists of cor=
es 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 =3D self._filter_sockets(self._lcores_to_fi=
lter)
> >           filtered_lcores =3D []
> >           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 fro=
m 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 f=
rom those sockets.
> > +        If filtering a number of cores, we need to go through all core=
s 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 sorte=
d by the physical core.
> > +
> > +        Returns:
> > +            A list of lists of logical CPU cores. Each list contains c=
ores from one socket.
> >           """
> >           allowed_sockets: set[int] =3D set()
> >           socket_count =3D self._filter_specifier.socket_count
> >           if self._filter_specifier.sockets:
> > +            # when sockets in filter is specified, the sockets are alr=
eady set
> >               socket_count =3D len(self._filter_specifier.sockets)
> >               allowed_sockets =3D 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]] =3D {}
> >           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 a=
nd 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 ?
>

Good catch, this should be "separate lcores into sockets".

> >                   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 co=
res.
> > -        In multithreaded environments, keep only
> > -        the first self._filter_specifier.lcores_per_core lcores of tho=
se cores.
> > -        """
> > +        """Filter a list of cores from the given socket.
> > +
> > +        Go through the cores and note how many logical cores per physi=
cal 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] =3D {}
> > @@ -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 fil=
ter those that match.
> > +
> > +    The input filter is :class:`LogicalCoreList`. An empty LogicalCore=
List 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 ID=
s). If :data:`False`,
> > +            sort in descending order.
> > +
> > +    Returns:
> > +        The filter matching `filter_specifier`.
> > +    """
> >       if isinstance(filter_specifier, LogicalCoreList):
> >           return LogicalCoreListFilter(core_list, filter_specifier, asc=
ending)
> >       elif isinstance(filter_specifier, LogicalCoreCount):
>