[dpdk-dev] [PATCH v3 1/2] eventdev: add device stop flush callback
Jerin Jacob
jerin.jacob at caviumnetworks.com
Tue Mar 20 08:44:05 CET 2018
-----Original Message-----
> Date: Wed, 14 Mar 2018 23:12:09 -0500
> From: Gage Eads <gage.eads at intel.com>
> To: dev at dpdk.org
> CC: jerin.jacob at caviumnetworks.com, harry.van.haaren at intel.com,
> hemant.agrawal at nxp.com, bruce.richardson at intel.com,
> santosh.shukla at caviumnetworks.com, nipun.gupta at 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 <gage.eads at intel.com>
> ---
> 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.
More information about the dev
mailing list