From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>,
rsanford@akamai.com, thomas@monjalon.net
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] timer: fix resource leak in finalize
Date: Thu, 2 May 2019 10:18:06 +0100 [thread overview]
Message-ID: <fcf4377c-ebbc-3778-b631-55c011431dba@intel.com> (raw)
In-Reply-To: <1556737217-24338-1-git-send-email-erik.g.carrillo@intel.com>
On 01-May-19 8:00 PM, Erik Gabriel Carrillo wrote:
> The finalize function should free the memzone created in the init
> function, rather than freeing the allocation the memzone references,
> otherwise a memzone descriptor can be leaked.
>
> Fixes: c0749f7096c7 ("timer: allow management in shared memory")
>
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> ---
> lib/librte_timer/rte_timer.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
> index eb46009..fb7a87e 100644
> --- a/lib/librte_timer/rte_timer.c
> +++ b/lib/librte_timer/rte_timer.c
> @@ -60,6 +60,7 @@ struct rte_timer_data {
> };
>
> #define RTE_MAX_DATA_ELS 64
> +static const struct rte_memzone *rte_timer_data_mz;
> static struct rte_timer_data *rte_timer_data_arr;
> static const uint32_t default_data_id;
> static uint32_t rte_timer_subsystem_initialized;
> @@ -164,6 +165,7 @@ rte_timer_subsystem_init_v1905(void)
> if (mz == NULL)
> return -EEXIST;
>
> + rte_timer_data_mz = mz;
> rte_timer_data_arr = mz->addr;
>
> rte_timer_data_arr[default_data_id].internal_flags |=
> @@ -180,6 +182,7 @@ rte_timer_subsystem_init_v1905(void)
> if (mz == NULL)
> return -ENOMEM;
>
> + rte_timer_data_mz = mz;
> rte_timer_data_arr = mz->addr;
>
> for (i = 0; i < RTE_MAX_DATA_ELS; i++) {
> @@ -205,8 +208,13 @@ BIND_DEFAULT_SYMBOL(rte_timer_subsystem_init, _v1905, 19.05);
> void __rte_experimental
> rte_timer_subsystem_finalize(void)
> {
> - if (rte_timer_data_arr)
> - rte_free(rte_timer_data_arr);
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> + return;
> +
> + if (!rte_timer_subsystem_initialized)
> + return;
> +
> + rte_memzone_free(rte_timer_data_mz);
The patch is a correct fix, but the whole idea of this looks dangerous
to me.
If we exit the primary while secondaries are still running, wouldn't it
basically pull out timer data from under secondaries' feet?
>
> rte_timer_subsystem_initialized = 0;
> }
>
--
Thanks,
Anatoly
next prev parent reply other threads:[~2019-05-02 9:18 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-01 19:00 Erik Gabriel Carrillo
2019-05-01 19:00 ` Erik Gabriel Carrillo
2019-05-02 9:18 ` Burakov, Anatoly [this message]
2019-05-02 9:18 ` Burakov, Anatoly
2019-05-02 12:19 ` Carrillo, Erik G
2019-05-02 12:19 ` Carrillo, Erik G
2019-05-02 13:03 ` Burakov, Anatoly
2019-05-02 13:03 ` Burakov, Anatoly
2019-05-02 13:48 ` Carrillo, Erik G
2019-05-02 13:48 ` Carrillo, Erik G
2019-05-03 22:54 ` [dpdk-dev] [PATCH v2] " Erik Gabriel Carrillo
2019-05-03 22:54 ` Erik Gabriel Carrillo
2019-05-07 11:03 ` Burakov, Anatoly
2019-05-07 11:03 ` Burakov, Anatoly
2019-05-07 22:04 ` Carrillo, Erik G
2019-05-07 22:04 ` Carrillo, Erik G
2019-05-08 8:49 ` Burakov, Anatoly
2019-05-08 8:49 ` Burakov, Anatoly
2019-05-08 23:01 ` Carrillo, Erik G
2019-05-08 23:01 ` Carrillo, Erik G
2019-05-09 7:44 ` Thomas Monjalon
2019-05-09 7:44 ` Thomas Monjalon
2019-05-08 22:35 ` [dpdk-dev] [PATCH v3] " Erik Gabriel Carrillo
2019-05-08 22:35 ` Erik Gabriel Carrillo
2019-05-09 8:29 ` Burakov, Anatoly
2019-05-09 8:29 ` Burakov, Anatoly
2019-06-05 9:33 ` Thomas Monjalon
2019-06-05 9:47 ` Burakov, Anatoly
2019-06-25 16:11 ` [dpdk-dev] [PATCH 0/2] Fix timer resource leak Anatoly Burakov
2019-07-05 13:20 ` [dpdk-dev] [PATCH v2 0/1] " Anatoly Burakov
2019-07-05 17:22 ` [dpdk-dev] [PATCH v3 " Anatoly Burakov
2019-07-05 17:22 ` [dpdk-dev] [PATCH v3 1/1] timer: fix resource leak in finalize Anatoly Burakov
2019-07-05 22:06 ` Thomas Monjalon
2019-07-05 13:20 ` [dpdk-dev] [PATCH v2 " Anatoly Burakov
2019-06-25 16:11 ` [dpdk-dev] [PATCH 1/2] eal: add internal locks for timer lib into EAL Anatoly Burakov
2019-06-27 18:41 ` Carrillo, Erik G
2019-07-04 9:09 ` David Marchand
2019-07-04 10:44 ` Burakov, Anatoly
2019-06-25 16:11 ` [dpdk-dev] [PATCH 2/2] timer: fix resource leak in finalize Anatoly Burakov
2019-06-27 18:48 ` Carrillo, Erik G
2019-07-04 9:10 ` David Marchand
2019-07-04 10:45 ` Burakov, Anatoly
2019-07-04 10:50 ` David Marchand
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=fcf4377c-ebbc-3778-b631-55c011431dba@intel.com \
--to=anatoly.burakov@intel.com \
--cc=dev@dpdk.org \
--cc=erik.g.carrillo@intel.com \
--cc=rsanford@akamai.com \
--cc=thomas@monjalon.net \
/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).