[dpdk-dev,v3,1/2] eventdev: add device stop flush callback
Checks
Commit Message
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@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(-)
Comments
-----Original Message-----
> Date: Wed, 14 Mar 2018 23:12:09 -0500
> From: Gage Eads <gage.eads@intel.com>
> 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 <gage.eads@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.
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Tuesday, March 20, 2018 2:44 AM
> To: Eads, Gage <gage.eads@intel.com>
> Cc: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>;
> hemant.agrawal@nxp.com; Richardson, Bruce <bruce.richardson@intel.com>;
> santosh.shukla@caviumnetworks.com; nipun.gupta@nxp.com
> Subject: Re: [PATCH v3 1/2] eventdev: add device stop flush callback
>
> -----Original Message-----
> > Date: Wed, 14 Mar 2018 23:12:09 -0500
> > From: Gage Eads <gage.eads@intel.com>
> > 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 <gage.eads@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.
>
Good point. I will fix and resubmit a v4.
@@ -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,
};
static int
@@ -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,
};
static int
@@ -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
};
static int
@@ -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,
};
static const char *const args[] = {
@@ -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
};
static int
@@ -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,
};
static const char *const args[] = {
@@ -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;
+}
+
void
rte_event_dev_stop(uint8_t dev_id)
{
@@ -244,6 +244,7 @@ extern "C" {
#include <rte_errno.h>
struct rte_mbuf; /* we just use mbuf pointers; no need to include rte_mbuf.h */
+struct rte_event;
/* Event device capability bitmap flags */
#define RTE_EVENT_DEV_CAP_QUEUE_QOS (1ULL << 0)
@@ -835,15 +836,61 @@ int
rte_event_dev_start(uint8_t dev_id);
/**
- * Stop an event device. The device can be restarted with a call to
- * rte_event_dev_start()
+ * Stop an event device.
+ *
+ * This function causes all queued events to be drained. While draining events
+ * out of the device, this function calls the user-provided flush callback
+ * (if one was registered) once per event.
+ *
+ * This function does not drain events from event ports; the application is
+ * responsible for flushing events from all ports before stopping the device.
+ *
+ * The device can be restarted with a call to rte_event_dev_start(). Threads
+ * that continue to enqueue/dequeue while the device is stopped, or being
+ * stopped, will result in undefined behavior.
*
* @param dev_id
* Event device identifier.
+ *
+ * @see rte_event_dev_stop_flush_callback_register()
*/
void
rte_event_dev_stop(uint8_t dev_id);
+typedef void (*eventdev_stop_flush_t)(uint8_t dev_id, struct rte_event event,
+ void *arg);
+/**< Callback function called during rte_event_dev_stop(), invoked once per
+ * flushed event.
+ */
+
+/**
+ * Registers a callback function to be invoked during rte_event_dev_stop() for
+ * each flushed event. This function can be used to properly dispose of queued
+ * events, for example events containing memory pointers.
+ *
+ * The callback function is only registered for the calling process. The
+ * callback function must be registered in every process that can call
+ * rte_event_dev_stop().
+ *
+ * To unregister a callback, call this function with a NULL callback pointer.
+ *
+ * @param dev_id
+ * The identifier of the device.
+ * @param callback
+ * Callback function invoked once per flushed event.
+ * @param userdata
+ * Argument supplied to callback.
+ *
+ * @return
+ * - 0 on success.
+ * - -EINVAL if *dev_id* is invalid
+ *
+ * @see rte_event_dev_stop()
+ */
+int
+rte_event_dev_stop_flush_callback_register(uint8_t dev_id,
+ eventdev_stop_flush_t callback, void *userdata);
+
/**
* Close an event device. The device cannot be restarted!
*
@@ -1152,6 +1199,8 @@ struct rte_eventdev_data {
/* Service initialization state */
uint32_t service_id;
/* Service ID*/
+ void *dev_stop_flush_arg;
+ /**< User-provided argument for event flush function */
RTE_STD_C11
uint8_t dev_started : 1;
@@ -1178,7 +1227,7 @@ struct rte_eventdev {
struct rte_eventdev_data *data;
/**< Pointer to device data */
- const struct rte_eventdev_ops *dev_ops;
+ struct rte_eventdev_ops *dev_ops;
/**< Functions exported by PMD */
struct rte_device *dev;
/**< Device info. supplied by probing */
@@ -642,6 +642,9 @@ struct rte_eventdev_ops {
eventdev_selftest dev_selftest;
/**< Start eventdev Selftest */
+
+ eventdev_stop_flush_t dev_stop_flush;
+ /**< User-provided event flush function */
};
/**
@@ -74,3 +74,9 @@ DPDK_18.02 {
rte_event_dev_selftest;
} DPDK_17.11;
+
+DPDK_18.05 {
+ global:
+
+ rte_event_dev_stop_flush_callback_register;
+} DPDK_18.02;