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 954C5A034C; Mon, 9 May 2022 19:29:38 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 419E7410EE; Mon, 9 May 2022 19:29:38 +0200 (CEST) Received: from mail-io1-f53.google.com (mail-io1-f53.google.com [209.85.166.53]) by mails.dpdk.org (Postfix) with ESMTP id 3CCB0407FF for ; Mon, 9 May 2022 19:29:37 +0200 (CEST) Received: by mail-io1-f53.google.com with SMTP id f2so16082132ioh.7 for ; Mon, 09 May 2022 10:29:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=RyhmfHAQ1UQSQiIoGPMGIBlLRgA/K5vvWt8t7vZzilA=; b=XGDBMsOzl7+H1O5OwXSgs6A/XMDp/iuUeL/mBFHujV3unJxyvYfMMXUM9LyO8FqprR WjVwpR8jABuwGdjZC5w2wiiUUIpu8Fct3qRjyNJpwoksZFyPeV5P8UbxY/TLycXikM7X 2oQ3i8yKhBqf7uWL308vyxKf7YVeDFZy9AUBLZX4Z8RhOL8la6cU9BJV2tXb90cz8DkE 58Bl6UHvtyaXDAMMOGs2BIrtw95KQYi+MDgiv9Ho6h1AaSZ4XD5r8WspU4rl2ccEpzFz i18b0U8V5OiNHhvw+aTqDbWHsuWBtFOuJKUCF/MXMpI40OVo1oAWHId5GzoErePYGubn MBug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=RyhmfHAQ1UQSQiIoGPMGIBlLRgA/K5vvWt8t7vZzilA=; b=GQza+6mNdmrlgnU8RBhUg47EiIcDAkcUNmB3JrKBgXzkB/VlHY3Y23fiv3Ku9sp2Cn UAJfNiFsJ3dQy31+Y0V2PCJLJy3o/syzQmaxHkugN89vpxAZCbqBRn60bad57CZBX55r INlZn0mPFS6I0PAc6S4MRZjgx/3s4f9+v2PalcrN80AbUzi597+1qjfKSKs7dY+SWdJk OrfdhtUg3/KV3bOITN2Ct5z+xrbv/k63BWU/O6cZ78i3/gr1wX7GWcfgf7ga435OwIgD 3MnpRDmETDPNc/geWpeB3XV2mjQGC7TE2y8qP2Rcl8i/H37HnvUpON7/G7xF+AoeQxdB TRzw== X-Gm-Message-State: AOAM5339qiInEZ8do+DFduOSYUZlcs92HOWQJuHH36H9qHN4ySLo6QY1 E+j1yMdEsN1qGZj7gcj+I795R0XwSFKh4nFCowY= X-Google-Smtp-Source: ABdhPJws2HmZSDCw7eVMeTPV7YQBw3m1J36DAfcBFzfl3d47nPCq9mMZqHoK+p3Paz8e95OFZy0ICHYXSoW3up1DD0k= X-Received: by 2002:a05:6602:1815:b0:658:c186:c1cc with SMTP id t21-20020a056602181500b00658c186c1ccmr6955865ioh.25.1652117376505; Mon, 09 May 2022 10:29:36 -0700 (PDT) MIME-Version: 1.0 References: <20220427113223.13948-1-pbhagavatula@marvell.com> In-Reply-To: <20220427113223.13948-1-pbhagavatula@marvell.com> From: Jerin Jacob Date: Mon, 9 May 2022 22:59:10 +0530 Message-ID: Subject: Re: [PATCH 1/3] eventdev: add function to quiesce an event port To: Pavan Nikhilesh , "Jayatheerthan, Jay" , Erik Gabriel Carrillo , "Gujjar, Abhinandan S" , "McDaniel, Timothy" , Hemant Agrawal , Nipun Gupta , "Van Haaren, Harry" , =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , Liang Ma , Peter Mccarthy Cc: Jerin Jacob , Ray Kinsella , dpdk-dev , Shijith Thotton Content-Type: text/plain; charset="UTF-8" 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, Apr 27, 2022 at 5:02 PM Pavan Nikhilesh wrote: > > Add function to quiesce any core specific resources consumed by > the event port. > > When the application decides to migrate the event port to another lcore > or teardown the current lcore it may to call `rte_event_port_quiesce` > to make sure that all the data associated with the event port are released > from the lcore, this might also include any prefetched events. > > While releasing the event port from the lcore, this function calls the > user-provided flush callback once per event. > > Signed-off-by: Pavan Nikhilesh + eventdev stake holder @jay.jayatheerthan@intel.com @erik.g.carrillo@intel.com @abhinandan.gujjar@intel.com timothy.mcdaniel@intel.com sthotton@marvell.com hemant.agrawal@nxp.com nipun.gupta@nxp.com harry.van.haaren@intel.com mattias.ronnblom@ericsson.com liangma@liangbit.com peter.mccarthy@intel.com Since it is in a slow path and allows port teardown on migration for the implementations where core has some state for the port. The new API addition looks good to me. Any objection or alternative thought from eventdev stake holders? Some comments below. > --- > lib/eventdev/eventdev_pmd.h | 19 +++++++++++++++++++ > lib/eventdev/rte_eventdev.c | 19 +++++++++++++++++++ > lib/eventdev/rte_eventdev.h | 33 +++++++++++++++++++++++++++++++++ > lib/eventdev/version.map | 3 +++ > 4 files changed, 74 insertions(+) > > diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h > index ce469d47a6..cf9f2146a1 100644 > --- a/lib/eventdev/eventdev_pmd.h > +++ b/lib/eventdev/eventdev_pmd.h > @@ -381,6 +381,23 @@ typedef int (*eventdev_port_setup_t)(struct rte_eventdev *dev, > */ > typedef void (*eventdev_port_release_t)(void *port); > > +/** > + * Quiesce any core specific resources consumed by the event port > + * > + * @param dev > + * Event device pointer. > + * @param port > + * Event port pointer. > + * @param flush_cb > + * User-provided event flush function. > + * @param args > + * Arguments to be passed to the user-provided event flush function. > + * > + */ > +typedef void (*eventdev_port_quiesce_t)(struct rte_eventdev *dev, void *port, Please prefix rte_ for public symbols. i.e rte_event_port_quiesce_t. I know we missed for existing eventdev_stop_flush_t, which we can fix in the next ABI. I will send a patch for same. > + eventdev_port_flush_t flush_cb, > + void *args); > + > /** > * Link multiple source event queues to destination event port. > * > @@ -1218,6 +1235,8 @@ struct eventdev_ops { > /**< Set up an event port. */ > eventdev_port_release_t port_release; > /**< Release an event port. */ > + eventdev_port_quiesce_t port_quiesce; > + /**< Quiesce an event port. */ > > eventdev_port_link_t port_link; > /**< Link event queues to an event port. */ > diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c > index 532a253553..541fa5dc61 100644 > --- a/lib/eventdev/rte_eventdev.c > +++ b/lib/eventdev/rte_eventdev.c > @@ -730,6 +730,25 @@ rte_event_port_setup(uint8_t dev_id, uint8_t port_id, > return 0; > } > > +void > +rte_event_port_quiesce(uint8_t dev_id, uint8_t port_id, > + eventdev_port_flush_t release_cb, void *args) > +{ > + struct rte_eventdev *dev; > + > + RTE_EVENTDEV_VALID_DEVID_OR_RET(dev_id); > + dev = &rte_eventdevs[dev_id]; > + > + if (!is_valid_port(dev, port_id)) { > + RTE_EDEV_LOG_ERR("Invalid port_id=%" PRIu8, port_id); > + return; > + } > + > + if (dev->dev_ops->port_quiesce) > + (*dev->dev_ops->port_quiesce)(dev, dev->data->ports[port_id], > + release_cb, args); > +} > + > int > rte_event_dev_attr_get(uint8_t dev_id, uint32_t attr_id, > uint32_t *attr_value) > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h > index 42a5660169..c86d8a5576 100644 > --- a/lib/eventdev/rte_eventdev.h > +++ b/lib/eventdev/rte_eventdev.h > @@ -830,6 +830,39 @@ int > rte_event_port_setup(uint8_t dev_id, uint8_t port_id, > const struct rte_event_port_conf *port_conf); > > +typedef void (*eventdev_port_flush_t)(uint8_t dev_id, struct rte_event event, > + void *arg); > +/**< Callback function prototype that can be passed during > + * rte_event_port_release(), invoked once per a released event. > + */ > + > +/** > + * Quiesce any core specific resources consumed by the event port. > + * > + * Event ports are generally coupled with lcores, and a given Hardware > + * implementation might require the PMD to store port specific data in the > + * lcore. > + * When the application decides to migrate the event port to an other lcore an other -> another > + * or teardown the current lcore it may to call `rte_event_port_quiesce` > + * to make sure that all the data associated with the event port are released > + * from the lcore, this might also include any prefetched events. > + * While releasing the event port from the lcore, this function calls the > + * user-provided flush callback once per event. > + * > + * The event port specific config is not reset. Make this as @note The event port-specific config shall not reset on this API call or similar. > + * > + * @param dev_id > + * The identifier of the device. > + * @param port_id > + * The index of the event port to setup. The value must be in the range > + * [0, nb_event_ports - 1] previously supplied to rte_event_dev_configure(). > + * @param release_cb > + * Callback function invoked once per flushed event. > + */ > +__rte_experimental > +void rte_event_port_quiesce(uint8_t dev_id, uint8_t port_id, > + eventdev_port_flush_t release_cb, void *args); > Please update doc/guides/prog_guide/eventdev.rst and add section for teardown and mention existing rte_event_dev_stop_flush_callback_register() and this new API.