From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH v3 1/2] eventdev: add device stop flush callback Date: Tue, 20 Mar 2018 13:14:05 +0530 Message-ID: <20180320074404.GA2022@jerin> References: <1520550631-13403-1-git-send-email-gage.eads@intel.com> <1521087130-20244-1-git-send-email-gage.eads@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, harry.van.haaren@intel.com, hemant.agrawal@nxp.com, bruce.richardson@intel.com, santosh.shukla@caviumnetworks.com, nipun.gupta@nxp.com To: Gage Eads Return-path: Received: from NAM02-SN1-obe.outbound.protection.outlook.com (mail-sn1nam02on0057.outbound.protection.outlook.com [104.47.36.57]) by dpdk.org (Postfix) with ESMTP id D8BAA27D for ; Tue, 20 Mar 2018 08:44:24 +0100 (CET) Content-Disposition: inline In-Reply-To: <1521087130-20244-1-git-send-email-gage.eads@intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" -----Original Message----- > Date: Wed, 14 Mar 2018 23:12:09 -0500 > From: Gage Eads > To: dev@dpdk.org > CC: jerin.jacob@caviumnetworks.com, harry.van.haaren@intel.com, > hemant.agrawal@nxp.com, bruce.richardson@intel.com, > santosh.shukla@caviumnetworks.com, nipun.gupta@nxp.com > Subject: [PATCH v3 1/2] eventdev: add device stop flush callback > X-Mailer: git-send-email 2.7.4 > > When an event device is stopped, it drains all event queues. These events > may contain pointers, so to prevent memory leaks eventdev now supports a > user-provided flush callback that is called during the queue drain process. > This callback is stored in process memory, so the callback must be > registered by any process that may call rte_event_dev_stop(). > > This commit also clarifies the behavior of rte_event_dev_stop(). > > This follows this mailing list discussion: > http://dpdk.org/ml/archives/dev/2018-January/087484.html > > Signed-off-by: Gage Eads > --- > v2: allow a NULL callback pointer to unregister the callback > v3: move the callback pointer to struct rte_eventdev_ops and > the user argument to struct rte_eventdev_data. > > drivers/event/dpaa/dpaa_eventdev.c | 3 +- > drivers/event/dpaa2/dpaa2_eventdev.c | 3 +- > drivers/event/octeontx/ssovf_evdev.c | 6 ++- > drivers/event/opdl/opdl_evdev.c | 4 +- > drivers/event/skeleton/skeleton_eventdev.c | 5 ++- > drivers/event/sw/sw_evdev.c | 4 +- > lib/librte_eventdev/rte_eventdev.c | 17 +++++++++ > lib/librte_eventdev/rte_eventdev.h | 55 ++++++++++++++++++++++++++-- > lib/librte_eventdev/rte_eventdev_pmd.h | 3 ++ > lib/librte_eventdev/rte_eventdev_version.map | 6 +++ > 10 files changed, 95 insertions(+), 11 deletions(-) > > diff --git a/drivers/event/dpaa/dpaa_eventdev.c b/drivers/event/dpaa/dpaa_eventdev.c > index 0006801..4bf1918 100644 > --- a/drivers/event/dpaa/dpaa_eventdev.c > +++ b/drivers/event/dpaa/dpaa_eventdev.c > @@ -571,7 +571,7 @@ dpaa_event_eth_rx_adapter_stop(const struct rte_eventdev *dev, > return 0; > } > > -static const struct rte_eventdev_ops dpaa_eventdev_ops = { > +static struct rte_eventdev_ops dpaa_eventdev_ops = { > .dev_infos_get = dpaa_event_dev_info_get, > .dev_configure = dpaa_event_dev_configure, > .dev_start = dpaa_event_dev_start, > @@ -591,6 +591,7 @@ static const struct rte_eventdev_ops dpaa_eventdev_ops = { > .eth_rx_adapter_queue_del = dpaa_event_eth_rx_adapter_queue_del, > .eth_rx_adapter_start = dpaa_event_eth_rx_adapter_start, > .eth_rx_adapter_stop = dpaa_event_eth_rx_adapter_stop, > + .dev_stop_flush = NULL, Do we need explicit NULL assignment here. It will be NULL by default. > }; > > static int > diff --git a/drivers/event/dpaa2/dpaa2_eventdev.c b/drivers/event/dpaa2/dpaa2_eventdev.c > index c3e6fbf..363837a 100644 > --- a/drivers/event/dpaa2/dpaa2_eventdev.c > +++ b/drivers/event/dpaa2/dpaa2_eventdev.c > @@ -693,7 +693,7 @@ dpaa2_eventdev_eth_stop(const struct rte_eventdev *dev, > return 0; > } > > -static const struct rte_eventdev_ops dpaa2_eventdev_ops = { > +static struct rte_eventdev_ops dpaa2_eventdev_ops = { > .dev_infos_get = dpaa2_eventdev_info_get, > .dev_configure = dpaa2_eventdev_configure, > .dev_start = dpaa2_eventdev_start, > @@ -714,6 +714,7 @@ static const struct rte_eventdev_ops dpaa2_eventdev_ops = { > .eth_rx_adapter_queue_del = dpaa2_eventdev_eth_queue_del, > .eth_rx_adapter_start = dpaa2_eventdev_eth_start, > .eth_rx_adapter_stop = dpaa2_eventdev_eth_stop, > + .dev_stop_flush = NULL, Same as above comment. > }; > > static int > diff --git a/drivers/event/octeontx/ssovf_evdev.c b/drivers/event/octeontx/ssovf_evdev.c > index a108607..713983b 100644 > --- a/drivers/event/octeontx/ssovf_evdev.c > +++ b/drivers/event/octeontx/ssovf_evdev.c > @@ -591,7 +591,7 @@ ssovf_selftest(const char *key __rte_unused, const char *value, > } > > /* Initialize and register event driver with DPDK Application */ > -static const struct rte_eventdev_ops ssovf_ops = { > +static struct rte_eventdev_ops ssovf_ops = { > .dev_infos_get = ssovf_info_get, > .dev_configure = ssovf_configure, > .queue_def_conf = ssovf_queue_def_conf, > @@ -615,7 +615,9 @@ static const struct rte_eventdev_ops ssovf_ops = { > .dump = ssovf_dump, > .dev_start = ssovf_start, > .dev_stop = ssovf_stop, > - .dev_close = ssovf_close > + .dev_close = ssovf_close, > + > + .dev_stop_flush = NULL Same as above comment. > }; > > static int > diff --git a/drivers/event/opdl/opdl_evdev.c b/drivers/event/opdl/opdl_evdev.c > index 7708369..477e102 100644 > --- a/drivers/event/opdl/opdl_evdev.c > +++ b/drivers/event/opdl/opdl_evdev.c > @@ -607,7 +607,7 @@ set_do_test(const char *key __rte_unused, const char *value, void *opaque) > static int > opdl_probe(struct rte_vdev_device *vdev) > { > - static const struct rte_eventdev_ops evdev_opdl_ops = { > + static struct rte_eventdev_ops evdev_opdl_ops = { > .dev_configure = opdl_dev_configure, > .dev_infos_get = opdl_info_get, > .dev_close = opdl_close, > @@ -629,6 +629,8 @@ opdl_probe(struct rte_vdev_device *vdev) > .xstats_get_names = opdl_xstats_get_names, > .xstats_get_by_name = opdl_xstats_get_by_name, > .xstats_reset = opdl_xstats_reset, > + > + .dev_stop_flush = NULL, Same as above comment. > }; > > static const char *const args[] = { > diff --git a/drivers/event/skeleton/skeleton_eventdev.c b/drivers/event/skeleton/skeleton_eventdev.c > index 7f46756..c627f01 100644 > --- a/drivers/event/skeleton/skeleton_eventdev.c > +++ b/drivers/event/skeleton/skeleton_eventdev.c > @@ -319,7 +319,7 @@ skeleton_eventdev_dump(struct rte_eventdev *dev, FILE *f) > > > /* Initialize and register event driver with DPDK Application */ > -static const struct rte_eventdev_ops skeleton_eventdev_ops = { > +static struct rte_eventdev_ops skeleton_eventdev_ops = { > .dev_infos_get = skeleton_eventdev_info_get, > .dev_configure = skeleton_eventdev_configure, > .dev_start = skeleton_eventdev_start, > @@ -334,7 +334,8 @@ static const struct rte_eventdev_ops skeleton_eventdev_ops = { > .port_link = skeleton_eventdev_port_link, > .port_unlink = skeleton_eventdev_port_unlink, > .timeout_ticks = skeleton_eventdev_timeout_ticks, > - .dump = skeleton_eventdev_dump > + .dump = skeleton_eventdev_dump, > + .dev_stop_flush = NULL Same as above comment. > }; > > static int > diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c > index 6672fd8..bda2e21 100644 > --- a/drivers/event/sw/sw_evdev.c > +++ b/drivers/event/sw/sw_evdev.c > @@ -772,7 +772,7 @@ static int32_t sw_sched_service_func(void *args) > static int > sw_probe(struct rte_vdev_device *vdev) > { > - static const struct rte_eventdev_ops evdev_sw_ops = { > + static struct rte_eventdev_ops evdev_sw_ops = { > .dev_configure = sw_dev_configure, > .dev_infos_get = sw_info_get, > .dev_close = sw_close, > @@ -797,6 +797,8 @@ sw_probe(struct rte_vdev_device *vdev) > .xstats_reset = sw_xstats_reset, > > .dev_selftest = test_sw_eventdev, > + > + .dev_stop_flush = NULL, Same as above comment. > }; > > static const char *const args[] = { > diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c > index 851a119..2de8d9a 100644 > --- a/lib/librte_eventdev/rte_eventdev.c > +++ b/lib/librte_eventdev/rte_eventdev.c > @@ -1123,6 +1123,23 @@ rte_event_dev_start(uint8_t dev_id) > return 0; > } > > +int > +rte_event_dev_stop_flush_callback_register(uint8_t dev_id, > + eventdev_stop_flush_t callback, void *userdata) > +{ > + struct rte_eventdev *dev; > + > + RTE_EDEV_LOG_DEBUG("Stop flush register dev_id=%" PRIu8, dev_id); > + > + RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL); > + dev = &rte_eventdevs[dev_id]; > + > + dev->dev_ops->dev_stop_flush = callback; > + dev->data->dev_stop_flush_arg = userdata; > + > + return 0; > +} Other than above minor comments, everything else looks good to me.