From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
To: Gage Eads <gage.eads@intel.com>
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
Subject: Re: [dpdk-dev] [PATCH v3 1/2] eventdev: add device stop flush callback
Date: Tue, 20 Mar 2018 13:14:05 +0530 [thread overview]
Message-ID: <20180320074404.GA2022@jerin> (raw)
In-Reply-To: <1521087130-20244-1-git-send-email-gage.eads@intel.com>
-----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.
next prev parent reply other threads:[~2018-03-20 7:44 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-05 23:01 [dpdk-dev] [PATCH " Gage Eads
2018-03-05 23:01 ` [dpdk-dev] [PATCH 2/2] event/sw: support " Gage Eads
2018-03-08 23:10 ` [dpdk-dev] [PATCH v2 1/2] eventdev: add " Gage Eads
2018-03-08 23:10 ` [dpdk-dev] [PATCH v2 2/2] event/sw: support " Gage Eads
2018-03-12 6:25 ` [dpdk-dev] [PATCH v2 1/2] eventdev: add " Jerin Jacob
2018-03-12 14:30 ` Eads, Gage
2018-03-12 14:38 ` Jerin Jacob
2018-03-15 4:12 ` [dpdk-dev] [PATCH v3 " Gage Eads
2018-03-15 4:12 ` [dpdk-dev] [PATCH v3 2/2] event/sw: support " Gage Eads
2018-03-20 7:44 ` Jerin Jacob [this message]
2018-03-20 14:11 ` [dpdk-dev] [PATCH v3 1/2] eventdev: add " Eads, Gage
2018-03-20 14:13 ` [dpdk-dev] [PATCH v4 " Gage Eads
2018-03-20 14:13 ` [dpdk-dev] [PATCH v4 2/2] event/sw: support " Gage Eads
2018-03-23 17:05 ` Van Haaren, Harry
2018-03-26 22:01 ` Eads, Gage
2018-04-02 8:03 ` Jerin Jacob
2018-04-02 15:50 ` Eads, Gage
2018-04-02 17:08 ` Jerin Jacob
2018-03-23 16:57 ` [dpdk-dev] [PATCH v4 1/2] eventdev: add " Van Haaren, Harry
2018-03-26 21:59 ` Eads, Gage
2018-03-27 8:20 ` Van Haaren, Harry
2018-03-29 11:02 ` Van Haaren, Harry
2018-03-29 18:34 ` Jerin Jacob
2018-03-30 9:54 ` Liang, Ma
2018-04-02 8:01 ` Jerin Jacob
2018-04-02 18:03 ` [dpdk-dev] [PATCH v5] " Gage Eads
2018-04-03 1:26 ` Jerin Jacob
2018-04-03 1:31 ` Jerin Jacob
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180320074404.GA2022@jerin \
--to=jerin.jacob@caviumnetworks.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=gage.eads@intel.com \
--cc=harry.van.haaren@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=nipun.gupta@nxp.com \
--cc=santosh.shukla@caviumnetworks.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).