From: Ferruh Yigit <ferruh.yigit@amd.com>
To: longli@microsoft.com, Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Cc: dev@dpdk.org
Subject: Re: [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it
Date: Wed, 7 Feb 2024 18:40:31 +0000 [thread overview]
Message-ID: <f800a596-540d-4499-8834-f2a58afb5759@amd.com> (raw)
In-Reply-To: <1706577886-29483-1-git-send-email-longli@linuxonhyperv.com>
On 1/30/2024 1:24 AM, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> The content of the MR is copied to the cache trees, it's not necessary to
> allocate a MR to do this. Use a variable on the stack instead.
>
> This also fixes the memory leak in the code where a MR is allocated but
> never freed.
>
patch title describes what is done, but not gives information about
reasoning and impact.
Is this a performance improvement (if so how much), or is this a fix for
the memory leak (if so we need fixes tag for backport), or just a
refactoring?
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> drivers/net/mana/mr.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/mana/mr.c b/drivers/net/mana/mr.c
> index d6a5ad1460..c9d0f7ef5a 100644
> --- a/drivers/net/mana/mr.c
> +++ b/drivers/net/mana/mr.c
> @@ -40,7 +40,7 @@ mana_new_pmd_mr(struct mana_mr_btree *local_tree, struct mana_priv *priv,
> struct ibv_mr *ibv_mr;
> struct mana_range ranges[pool->nb_mem_chunks];
> uint32_t i;
> - struct mana_mr_cache *mr;
> + struct mana_mr_cache mr;
> int ret;
>
> rte_mempool_mem_iter(pool, mana_mempool_chunk_cb, ranges);
> @@ -75,14 +75,13 @@ mana_new_pmd_mr(struct mana_mr_btree *local_tree, struct mana_priv *priv,
> DP_LOG(DEBUG, "MR lkey %u addr %p len %zu",
> ibv_mr->lkey, ibv_mr->addr, ibv_mr->length);
>
> - mr = rte_calloc("MANA MR", 1, sizeof(*mr), 0);
> - mr->lkey = ibv_mr->lkey;
> - mr->addr = (uintptr_t)ibv_mr->addr;
> - mr->len = ibv_mr->length;
> - mr->verb_obj = ibv_mr;
> + mr.lkey = ibv_mr->lkey;
> + mr.addr = (uintptr_t)ibv_mr->addr;
> + mr.len = ibv_mr->length;
> + mr.verb_obj = ibv_mr;
>
> rte_spinlock_lock(&priv->mr_btree_lock);
> - ret = mana_mr_btree_insert(&priv->mr_btree, mr);
> + ret = mana_mr_btree_insert(&priv->mr_btree, &mr);
> rte_spinlock_unlock(&priv->mr_btree_lock);
> if (ret) {
> ibv_dereg_mr(ibv_mr);
> @@ -90,7 +89,7 @@ mana_new_pmd_mr(struct mana_mr_btree *local_tree, struct mana_priv *priv,
> return ret;
> }
>
> - ret = mana_mr_btree_insert(local_tree, mr);
> + ret = mana_mr_btree_insert(local_tree, &mr);
> if (ret) {
> /* Don't need to clean up MR as it's already
> * in the global tree
next prev parent reply other threads:[~2024-02-07 18:40 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-30 1:24 longli
2024-01-30 1:24 ` [PATCH 2/2] net/mana: properly deal with MR cache expansion failure longli
2024-02-07 18:41 ` Ferruh Yigit
2024-02-07 18:43 ` Long Li
2024-02-09 0:05 ` [Patch v2 1/2] net/mana: fix memory leak on MR variable allocation longli
2024-02-09 0:05 ` [Patch v2 2/2] net/mana: properly deal with MR cache expansion failure longli
2024-02-10 0:09 ` [Patch v2 1/2] net/mana: fix memory leak on MR variable allocation Ferruh Yigit
2024-02-07 18:40 ` Ferruh Yigit [this message]
2024-02-07 18:42 ` [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it Long Li
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=f800a596-540d-4499-8834-f2a58afb5759@amd.com \
--to=ferruh.yigit@amd.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=longli@microsoft.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).