* [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it @ 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:40 ` [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it Ferruh Yigit 0 siblings, 2 replies; 9+ messages in thread From: longli @ 2024-01-30 1:24 UTC (permalink / raw) To: Ferruh Yigit, Andrew Rybchenko; +Cc: dev, Long Li 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. 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 -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] net/mana: properly deal with MR cache expansion failure 2024-01-30 1:24 [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it longli @ 2024-01-30 1:24 ` longli 2024-02-07 18:41 ` Ferruh Yigit 2024-02-09 0:05 ` [Patch v2 1/2] net/mana: fix memory leak on MR variable allocation longli 2024-02-07 18:40 ` [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it Ferruh Yigit 1 sibling, 2 replies; 9+ messages in thread From: longli @ 2024-01-30 1:24 UTC (permalink / raw) To: Ferruh Yigit, Andrew Rybchenko; +Cc: dev, Long Li From: Long Li <longli@microsoft.com> On MR cache expension failure, the request should fail as there is no path to get a new MR into the tree. Attempting to insert a new MR to the cache tree will result in memory violation. Signed-off-by: Long Li <longli@microsoft.com> --- drivers/net/mana/mana.h | 6 +++--- drivers/net/mana/mr.c | 45 ++++++++++++++++++++++++++++------------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/drivers/net/mana/mana.h b/drivers/net/mana/mana.h index eadcd01858..309d553956 100644 --- a/drivers/net/mana/mana.h +++ b/drivers/net/mana/mana.h @@ -522,9 +522,9 @@ void mana_del_pmd_mr(struct mana_mr_cache *mr); void mana_mempool_chunk_cb(struct rte_mempool *mp, void *opaque, struct rte_mempool_memhdr *memhdr, unsigned int idx); -struct mana_mr_cache *mana_mr_btree_lookup(struct mana_mr_btree *bt, - uint16_t *idx, - uintptr_t addr, size_t len); +int mana_mr_btree_lookup(struct mana_mr_btree *bt, uint16_t *idx, + uintptr_t addr, size_t len, + struct mana_mr_cache **cache); int mana_mr_btree_insert(struct mana_mr_btree *bt, struct mana_mr_cache *entry); int mana_mr_btree_init(struct mana_mr_btree *bt, int n, int socket); void mana_mr_btree_free(struct mana_mr_btree *bt); diff --git a/drivers/net/mana/mr.c b/drivers/net/mana/mr.c index c9d0f7ef5a..c4045141bc 100644 --- a/drivers/net/mana/mr.c +++ b/drivers/net/mana/mr.c @@ -138,8 +138,12 @@ mana_alloc_pmd_mr(struct mana_mr_btree *local_mr_btree, struct mana_priv *priv, try_again: /* First try to find the MR in local queue tree */ - mr = mana_mr_btree_lookup(local_mr_btree, &idx, - (uintptr_t)mbuf->buf_addr, mbuf->buf_len); + ret = mana_mr_btree_lookup(local_mr_btree, &idx, + (uintptr_t)mbuf->buf_addr, mbuf->buf_len, + &mr); + if (ret) + return NULL; + if (mr) { DP_LOG(DEBUG, "Local mr lkey %u addr 0x%" PRIxPTR " len %zu", mr->lkey, mr->addr, mr->len); @@ -148,11 +152,14 @@ mana_alloc_pmd_mr(struct mana_mr_btree *local_mr_btree, struct mana_priv *priv, /* If not found, try to find the MR in global tree */ rte_spinlock_lock(&priv->mr_btree_lock); - mr = mana_mr_btree_lookup(&priv->mr_btree, &idx, - (uintptr_t)mbuf->buf_addr, - mbuf->buf_len); + ret = mana_mr_btree_lookup(&priv->mr_btree, &idx, + (uintptr_t)mbuf->buf_addr, + mbuf->buf_len, &mr); rte_spinlock_unlock(&priv->mr_btree_lock); + if (ret) + return NULL; + /* If found in the global tree, add it to the local tree */ if (mr) { ret = mana_mr_btree_insert(local_mr_btree, mr); @@ -228,22 +235,23 @@ mana_mr_btree_expand(struct mana_mr_btree *bt, int n) /* * Look for a region of memory in MR cache. */ -struct mana_mr_cache * -mana_mr_btree_lookup(struct mana_mr_btree *bt, uint16_t *idx, - uintptr_t addr, size_t len) +int mana_mr_btree_lookup(struct mana_mr_btree *bt, uint16_t *idx, + uintptr_t addr, size_t len, + struct mana_mr_cache **cache) { struct mana_mr_cache *table; uint16_t n; uint16_t base = 0; int ret; - n = bt->len; + *cache = NULL; + n = bt->len; /* Try to double the cache if it's full */ if (n == bt->size) { ret = mana_mr_btree_expand(bt, bt->size << 1); if (ret) - return NULL; + return ret; } table = bt->table; @@ -262,14 +270,16 @@ mana_mr_btree_lookup(struct mana_mr_btree *bt, uint16_t *idx, *idx = base; - if (addr + len <= table[base].addr + table[base].len) - return &table[base]; + if (addr + len <= table[base].addr + table[base].len) { + *cache = &table[base]; + return 0; + } DP_LOG(DEBUG, "addr 0x%" PRIxPTR " len %zu idx %u sum 0x%" PRIxPTR " not found", addr, len, *idx, addr + len); - return NULL; + return 0; } int @@ -314,14 +324,21 @@ mana_mr_btree_insert(struct mana_mr_btree *bt, struct mana_mr_cache *entry) struct mana_mr_cache *table; uint16_t idx = 0; uint16_t shift; + int ret; + + ret = mana_mr_btree_lookup(bt, &idx, entry->addr, entry->len, &table); + if (ret) + return ret; - if (mana_mr_btree_lookup(bt, &idx, entry->addr, entry->len)) { + if (table) { DP_LOG(DEBUG, "Addr 0x%" PRIxPTR " len %zu exists in btree", entry->addr, entry->len); return 0; } if (bt->len >= bt->size) { + DP_LOG(ERR, "Btree overflow detected len %u size %u", + bt->len, bt->size); bt->overflow = 1; return -1; } -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] net/mana: properly deal with MR cache expansion failure 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 1 sibling, 1 reply; 9+ messages in thread From: Ferruh Yigit @ 2024-02-07 18:41 UTC (permalink / raw) To: longli, Andrew Rybchenko; +Cc: dev On 1/30/2024 1:24 AM, longli@linuxonhyperv.com wrote: > From: Long Li <longli@microsoft.com> > > On MR cache expension failure, the request should fail as there is no path > to get a new MR into the tree. Attempting to insert a new MR to the cache > tree will result in memory violation. > if this patch is fixing memory violation, can you please update commit log as fix commit and add fixes tag? ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 2/2] net/mana: properly deal with MR cache expansion failure 2024-02-07 18:41 ` Ferruh Yigit @ 2024-02-07 18:43 ` Long Li 0 siblings, 0 replies; 9+ messages in thread From: Long Li @ 2024-02-07 18:43 UTC (permalink / raw) To: Ferruh Yigit, Andrew Rybchenko; +Cc: dev > > On MR cache expension failure, the request should fail as there is no > > path to get a new MR into the tree. Attempting to insert a new MR to > > the cache tree will result in memory violation. > > > > if this patch is fixing memory violation, can you please update commit log as fix > commit and add fixes tag? Will send v2. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Patch v2 1/2] net/mana: fix memory leak on MR variable allocation 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-09 0:05 ` 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 1 sibling, 2 replies; 9+ messages in thread From: longli @ 2024-02-09 0:05 UTC (permalink / raw) To: Ferruh Yigit, Andrew Rybchenko; +Cc: dev, Long Li, stable From: Long Li <longli@microsoft.com> Use a MR on the stack instead of allocating it. This fixes the memory leak in the code where a MR is allocated but never freed. Fixes: 0f5db3c68ba7 ("net/mana: implement memory registration") Cc: stable@dpdk.org Signed-off-by: Long Li <longli@microsoft.com> --- Change in v2: change commit message to indicate this is a fix. added "Fixes" tag. 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 -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Patch v2 2/2] net/mana: properly deal with MR cache expansion failure 2024-02-09 0:05 ` [Patch v2 1/2] net/mana: fix memory leak on MR variable allocation longli @ 2024-02-09 0:05 ` longli 2024-02-10 0:09 ` [Patch v2 1/2] net/mana: fix memory leak on MR variable allocation Ferruh Yigit 1 sibling, 0 replies; 9+ messages in thread From: longli @ 2024-02-09 0:05 UTC (permalink / raw) To: Ferruh Yigit, Andrew Rybchenko; +Cc: dev, Long Li, stable From: Long Li <longli@microsoft.com> On MR cache expension failure, the request should fail as there is no path to get a new MR into the tree. Attempting to insert a new MR to the cache tree will result in memory violation. Fixes: 0f5db3c68ba7 ("net/mana: implement memory registration") Cc: stable@dpdk.org Signed-off-by: Long Li <longli@microsoft.com> --- Change in v2: added "Fixes:" tag. drivers/net/mana/mana.h | 6 +++--- drivers/net/mana/mr.c | 45 ++++++++++++++++++++++++++++------------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/drivers/net/mana/mana.h b/drivers/net/mana/mana.h index eadcd01858..309d553956 100644 --- a/drivers/net/mana/mana.h +++ b/drivers/net/mana/mana.h @@ -522,9 +522,9 @@ void mana_del_pmd_mr(struct mana_mr_cache *mr); void mana_mempool_chunk_cb(struct rte_mempool *mp, void *opaque, struct rte_mempool_memhdr *memhdr, unsigned int idx); -struct mana_mr_cache *mana_mr_btree_lookup(struct mana_mr_btree *bt, - uint16_t *idx, - uintptr_t addr, size_t len); +int mana_mr_btree_lookup(struct mana_mr_btree *bt, uint16_t *idx, + uintptr_t addr, size_t len, + struct mana_mr_cache **cache); int mana_mr_btree_insert(struct mana_mr_btree *bt, struct mana_mr_cache *entry); int mana_mr_btree_init(struct mana_mr_btree *bt, int n, int socket); void mana_mr_btree_free(struct mana_mr_btree *bt); diff --git a/drivers/net/mana/mr.c b/drivers/net/mana/mr.c index c9d0f7ef5a..c4045141bc 100644 --- a/drivers/net/mana/mr.c +++ b/drivers/net/mana/mr.c @@ -138,8 +138,12 @@ mana_alloc_pmd_mr(struct mana_mr_btree *local_mr_btree, struct mana_priv *priv, try_again: /* First try to find the MR in local queue tree */ - mr = mana_mr_btree_lookup(local_mr_btree, &idx, - (uintptr_t)mbuf->buf_addr, mbuf->buf_len); + ret = mana_mr_btree_lookup(local_mr_btree, &idx, + (uintptr_t)mbuf->buf_addr, mbuf->buf_len, + &mr); + if (ret) + return NULL; + if (mr) { DP_LOG(DEBUG, "Local mr lkey %u addr 0x%" PRIxPTR " len %zu", mr->lkey, mr->addr, mr->len); @@ -148,11 +152,14 @@ mana_alloc_pmd_mr(struct mana_mr_btree *local_mr_btree, struct mana_priv *priv, /* If not found, try to find the MR in global tree */ rte_spinlock_lock(&priv->mr_btree_lock); - mr = mana_mr_btree_lookup(&priv->mr_btree, &idx, - (uintptr_t)mbuf->buf_addr, - mbuf->buf_len); + ret = mana_mr_btree_lookup(&priv->mr_btree, &idx, + (uintptr_t)mbuf->buf_addr, + mbuf->buf_len, &mr); rte_spinlock_unlock(&priv->mr_btree_lock); + if (ret) + return NULL; + /* If found in the global tree, add it to the local tree */ if (mr) { ret = mana_mr_btree_insert(local_mr_btree, mr); @@ -228,22 +235,23 @@ mana_mr_btree_expand(struct mana_mr_btree *bt, int n) /* * Look for a region of memory in MR cache. */ -struct mana_mr_cache * -mana_mr_btree_lookup(struct mana_mr_btree *bt, uint16_t *idx, - uintptr_t addr, size_t len) +int mana_mr_btree_lookup(struct mana_mr_btree *bt, uint16_t *idx, + uintptr_t addr, size_t len, + struct mana_mr_cache **cache) { struct mana_mr_cache *table; uint16_t n; uint16_t base = 0; int ret; - n = bt->len; + *cache = NULL; + n = bt->len; /* Try to double the cache if it's full */ if (n == bt->size) { ret = mana_mr_btree_expand(bt, bt->size << 1); if (ret) - return NULL; + return ret; } table = bt->table; @@ -262,14 +270,16 @@ mana_mr_btree_lookup(struct mana_mr_btree *bt, uint16_t *idx, *idx = base; - if (addr + len <= table[base].addr + table[base].len) - return &table[base]; + if (addr + len <= table[base].addr + table[base].len) { + *cache = &table[base]; + return 0; + } DP_LOG(DEBUG, "addr 0x%" PRIxPTR " len %zu idx %u sum 0x%" PRIxPTR " not found", addr, len, *idx, addr + len); - return NULL; + return 0; } int @@ -314,14 +324,21 @@ mana_mr_btree_insert(struct mana_mr_btree *bt, struct mana_mr_cache *entry) struct mana_mr_cache *table; uint16_t idx = 0; uint16_t shift; + int ret; + + ret = mana_mr_btree_lookup(bt, &idx, entry->addr, entry->len, &table); + if (ret) + return ret; - if (mana_mr_btree_lookup(bt, &idx, entry->addr, entry->len)) { + if (table) { DP_LOG(DEBUG, "Addr 0x%" PRIxPTR " len %zu exists in btree", entry->addr, entry->len); return 0; } if (bt->len >= bt->size) { + DP_LOG(ERR, "Btree overflow detected len %u size %u", + bt->len, bt->size); bt->overflow = 1; return -1; } -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch v2 1/2] net/mana: fix memory leak on MR variable allocation 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 ` Ferruh Yigit 1 sibling, 0 replies; 9+ messages in thread From: Ferruh Yigit @ 2024-02-10 0:09 UTC (permalink / raw) To: longli, Andrew Rybchenko; +Cc: dev, stable On 2/9/2024 12:05 AM, longli@linuxonhyperv.com wrote: > From: Long Li <longli@microsoft.com> > > Use a MR on the stack instead of allocating it. This fixes the memory > leak in the code where a MR is allocated but never freed. > > Fixes: 0f5db3c68ba7 ("net/mana: implement memory registration") > Cc: stable@dpdk.org > > Signed-off-by: Long Li <longli@microsoft.com> > Series applied to dpdk-next-net/main, thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it 2024-01-30 1:24 [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it longli 2024-01-30 1:24 ` [PATCH 2/2] net/mana: properly deal with MR cache expansion failure longli @ 2024-02-07 18:40 ` Ferruh Yigit 2024-02-07 18:42 ` Long Li 1 sibling, 1 reply; 9+ messages in thread From: Ferruh Yigit @ 2024-02-07 18:40 UTC (permalink / raw) To: longli, Andrew Rybchenko; +Cc: dev 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it 2024-02-07 18:40 ` [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it Ferruh Yigit @ 2024-02-07 18:42 ` Long Li 0 siblings, 0 replies; 9+ messages in thread From: Long Li @ 2024-02-07 18:42 UTC (permalink / raw) To: Ferruh Yigit, Andrew Rybchenko; +Cc: dev > > 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? It's for fixing memory leak. I'll send v2 for better wording. > > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-02-10 0:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-01-30 1:24 [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it 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 ` [PATCH 1/2] net/mana: use a MR variable on the stack instead of allocating it Ferruh Yigit 2024-02-07 18:42 ` Long Li
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).