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 275E742A4D; Wed, 3 May 2023 10:27:02 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B32EF4114B; Wed, 3 May 2023 10:27:01 +0200 (CEST) Received: from mail-vk1-f176.google.com (mail-vk1-f176.google.com [209.85.221.176]) by mails.dpdk.org (Postfix) with ESMTP id 406E741144 for ; Wed, 3 May 2023 10:27:00 +0200 (CEST) Received: by mail-vk1-f176.google.com with SMTP id 71dfb90a1353d-44fb5a271d2so38004e0c.0 for ; Wed, 03 May 2023 01:27:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683102419; x=1685694419; 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=kOxDuEOUAjt9axqsibVj5oYp7tUh7U3XkKJ+3A5okec=; b=Am82n6v7hs6i1TmzYAbnJTfhMyf3VhBo1R6dyJLsmQyB+aYmM6UD+Rtspie4ytZY/u mS8pL/sRRcXVIuYfqCTUWUlcobckiCW6FcU7hCQXHiPmf8LPU7DXDL/qAmKFdL/Ggwj7 e7v/rke2utYZHVOLMKIy7xXsQiplb13oRDyEVHeqNYX6cXBFU4XzDXiMa8ieZZr46XDw iQTqV2LpBZvgKnhvg2x2Mi6FwEe/Fxl2jzgOnwbrCGt3g0BxTnpSViWVmycGzZZueQS9 /tUvZpMtbr/t7KEK6TrckCMnriTgq1Xp9ln47o5YyjWurLxeOK27dhc7T0EZ3UkZK6ap 9UGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683102419; x=1685694419; 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=kOxDuEOUAjt9axqsibVj5oYp7tUh7U3XkKJ+3A5okec=; b=QZJjOtEVYF/IlCci9drYFUUzHONFcpAWCNezn/IgD2hqV0c0UOoo339CaS+Ni7BMPW 4ncR9ZelT+FYfeamfI+u5n63mfvlNs+jC/38Ru1yZpdrRzUWZjRkLh68/QX06jEcI+7t fiC0eyLq8fYXh2HTzdFLPiOw3Ps1X3drbyHA7aQz6nGgHljFpNeuwOPhTgsrjh0ptW5T WO+nJ9zkgOYbR/4U64mh5yQ71Pjh8/WI0gX42NINE9YJI2nBpgKT3j7lx6xE6mRMN7Cg rZ2uVV27/4jtU4xO6UFHdKMyYRi3IjZDVj1wxLo1S7c+LuZWqAqednB/JU5DeRjgCkUB BThQ== X-Gm-Message-State: AC+VfDzBAEnRPW0XLlmI2iPQmy6b2f+MNdVz5T+rzw7WLC3CoVQxk0Lc +nLqy8Na7/O+5bvsWhWwAjtWugaawhaxUUM+cog= X-Google-Smtp-Source: ACHHUZ7bQuamj7NXOvwsfwBjr0JNbOy7H8WcIuwSyrbZw0cMkWc0pg3GeMz0O7GphH3K5zsIXz6JeLa5PlIqiYwHbPk= X-Received: by 2002:a67:ec11:0:b0:430:7548:c049 with SMTP id d17-20020a67ec11000000b004307548c049mr889465vso.7.1683102419398; Wed, 03 May 2023 01:26:59 -0700 (PDT) MIME-Version: 1.0 References: <20230419095427.563185-1-sivaprasad.tummala@amd.com> <324d5201-da95-f926-5580-f74ca5c09799@amd.com> In-Reply-To: From: Jerin Jacob Date: Wed, 3 May 2023 13:56:33 +0530 Message-ID: Subject: Re: [RFC PATCH 1/5] eventdev: add power monitoring API on event port To: Ferruh Yigit Cc: Sivaprasad Tummala , david.hunt@intel.com, jerinj@marvell.com, harry.van.haaren@intel.com, dev@dpdk.org, Pavan Nikhilesh , "McDaniel, Timothy" , Shijith Thotton , Hemant Agrawal , Sachin Saxena , =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , Peter Mccarthy , Liang Ma 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, May 3, 2023 at 1:44=E2=80=AFPM Ferruh Yigit = wrote: > > On 5/3/2023 8:58 AM, Jerin Jacob wrote: > > On Tue, May 2, 2023 at 4:49=E2=80=AFPM Ferruh Yigit wrote: > >> > >> On 4/25/2023 5:09 AM, Jerin Jacob wrote: > >>> On Mon, Apr 24, 2023 at 9:36=E2=80=AFPM Ferruh Yigit wrote: > >>>> > >>>> On 4/19/2023 11:15 AM, Jerin Jacob wrote: > >>>>> On Wed, Apr 19, 2023 at 3:24=E2=80=AFPM Sivaprasad Tummala > >>>>> wrote: > >>>>>> > >>>>>> A new API to allow power monitoring condition on event port to > >>>>>> optimize power when no events are arriving on an event port for > >>>>>> the worker core to process in an eventdev based pipelined applicat= ion. > >>>>>> > >>>>>> Signed-off-by: Sivaprasad Tummala > >>>>>> + * > >>>>>> + * @param dev_id > >>>>>> + * Eventdev id > >>>>>> + * @param port_id > >>>>>> + * Eventdev port id > >>>>>> + * @param pmc > >>>>>> + * The pointer to power-optimized monitoring condition structur= e. > >>>>>> + * > >>>>>> + * @return > >>>>>> + * - 0: Success. > >>>>>> + * -ENOTSUP: Operation not supported. > >>>>>> + * -EINVAL: Invalid parameters. > >>>>>> + * -ENODEV: Invalid device ID. > >>>>>> + */ > >>>>>> +__rte_experimental > >>>>>> +int > >>>>>> +rte_event_port_get_monitor_addr(uint8_t dev_id, uint8_t port_id, > >>>>>> + struct rte_power_monitor_cond *pmc); > >>>>> > >>>>> + eventdev driver maintainers > >>>>> > >>>>> I think, we don't need to expose this application due to applicatio= ns > >>>>> 1)To make applications to be transparent whether power saving is en= abled or not? > >>>>> 2)Some HW and Arch already supports power managent in driver and in= HW > >>>>> (Not using CPU architecture directly) > >>>>> > >>>>> If so, that will be translated to following, > >>>>> a) Add rte_event_port_power_saving_ena_dis(uint8_t dev_id, uint8_t > >>>>> port_id, bool ena) for controlling power saving in slowpath. > >>>>> b) Create reusable PMD private function based on the CPU architectu= re > >>>>> power saving primitive to cover the PMD don't have native power sav= ing > >>>>> support. > >>>>> c)Update rte_event_dequeue_burst() burst of PMD callback to use (b)= . > >>>>> > >>>>> > >>>> > >>>> Hi Jerin, > >>> > >>> Hi Ferruh, > >>> > >>>> > >>>> ethdev approach seems applied here. > >>> > >>> Understands that. But none of the NIC HW supports power management at > >>> HW level like eventdev, so that way > >>> for what we are doing for ethdev is a correct abstraction for ethdev. > >>> > >> > >> What I understand is there is HW based event manager and SW based ones= , > >> SW based ones can benefit more from CPU power optimizations, for HW > >> event managers if there is not enough benefit they can just ignore the > >> feature. > >> > >>>> > >>>> In ethdev, 'rte_event_port_get_monitor_addr()' equivalent is > >>>> 'rte_eth_get_monitor_addr()'. > >>>> > >>>> Although 'rte_eth_get_monitor_addr()' is public API, it is currently > >>>> only called from Rx/Tx callback functions implemented in the power l= ibrary. > >>>> But I assume intention to make it public is to enable users to imple= ment > >>>> their own callback functions that has custom algorithm for the power > >>>> management. > >>> > >>> If there is a use case for customizing with own callback, we can prov= ide that. > >>> Provided NULL is valid with default algorithm. > >>> > >>>> > >>>> And probably same is true for the 'rte_event_port_get_monitor_addr()= '. > >>>> > >>>> > >>>> Also instead of implementing power features for withing PMDs, isn't = it > >>>> better to have a common eventdev layer for it? > >>> > >>> We can have rte_evetdev_pmd_* APIs as non-public APIs. > >>> My only objection is to NOT introduce _monitor_ APIs at eventdev leve= l, > >>> Instead, _monitor_ is one way to do it in SW, So we need higher level > >>> of abstraction. > >>> > >> > >> I see, this seems a trade off between flexibility and usability. If > >> application has access to _monitor_ APIs, they can be more flexible to > >> implement their own logic. > > > > OK. > > > >> > >> Another option can be application provides the policy with an API and > >> monitor API used to realize the policy, but for this case it can be > >> challenge to find and implement correct policies. > > > > OK. If we can enumerate the policies, then it will be ideal. > > On plus side, there will not be any changes in needed in lib/power/ > > > > If we are talking about a power framework that user defines policies, I > expect parsing/defining policies will be in the power library and will > require changes in the power library anyway. OK > > But as mentioned above it is difficult to define a proper policy, this > is not really related to eventdev, more a power library issue. We can > continue to provide flexibility to user in eventdev and discuss the > policy if a wider forum. OK. > > > > >> > >>>> > >>>> For the PMDs benefit from HW event manager, just not implementing > >>>> .get_monitor_addr() dev_ops will make them free from power related A= PIs. > >>> > >>> But application fast path code gets diverged by exposing low level pr= imitives. > >>> > >> > >> I am not clear with concern above, but for application that use defaul= t > >> callbacks, 'rte_power_eventdev_pmgmt_port_enable()' needs to be called > >> to enable this feature, if not called datapath is not impacted. > >> And if not dequeue callback added at all, custom or default, data path > >> is not impacted at all. > > > > Concerns are around following code[1] when callback is not registered > > for this use case. > > In eventdev, we are using _one packet at a time_ for a lot of use case > > with latency critical workload like L1 processing. > > On such cases, the following code will add up. > > > > [1] > > cb =3D __atomic_load_n((void **)&fp_ops->ev_port.clbk[port_id], > > __ATOMIC_RELAXED); > > if (unlikely(cb !=3D NULL)) > > nb_rx =3D rte_event_dequeue_callbacks(dev_id, port_id, ev, nb_rx= , cb); > > > > I see two options, > > 1) Enumerate the power policy and let driver implement through > > non-public PMD helper functions > > OR > > 2)Move the power management callback to driver via non-public PMD > > helper functions to avoid cost of > > PMDs where power managment done in HW and to remove above extra check > > when NO callback is registered[1] > > > > Got it, yes there is an additional check with event callbacks, we can > add a compiler flag around it as done in ethdev to let it not compiled > when not needed, will it work? I would prefer to expose PMD helper function which can be called at end of the driver dequeue function so that other PMD can reuse as needed. This is to avoid compiler flag, cache line occupancy changes in struct rte_eventdev struct rte_event_fp_ops in generic code also we may not need full-fledged generic callbacks scheme for this. >