[v2] mempool: fix memory allocation in memzones during retry.

Message ID 1594711565-28309-1-git-send-email-wangzhike@jd.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] mempool: fix memory allocation in memzones during retry. |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS

Commit Message

王志克 July 14, 2020, 7:26 a.m. UTC
  If allocation is successful on the first attempt, typically
there is no problem since we allocated everything required and
we'll terminate the loop (if memory chunk is really sufficient
to populate required number of mempool elements).

If the first attempt fails, we try to allocate half
of mem_size and it succeed, we'll have one more iteration of
the for-loop to allocate memory for remaining elements and
should not try the next time with quarter of the mem_size.

It is wrong that max_alloc_size is divided by 2 in the
case of successful allocation as well, or invalid memory
can be allocated, and leads to population failure, then errno
other than ENOMEM may be returned.

Fixes: 3a3d0c75b43e ("mempool: fix slow allocation of large pools")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Signed-off-by: Zhike Wang <wangzhike@jd.com>
---
 lib/librte_mempool/rte_mempool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Burakov, Anatoly July 14, 2020, 9:32 a.m. UTC | #1
On 14-Jul-20 8:26 AM, Zhike Wang wrote:
> If allocation is successful on the first attempt, typically
> there is no problem since we allocated everything required and
> we'll terminate the loop (if memory chunk is really sufficient
> to populate required number of mempool elements).
> 
> If the first attempt fails, we try to allocate half
> of mem_size and it succeed, we'll have one more iteration of
> the for-loop to allocate memory for remaining elements and
> should not try the next time with quarter of the mem_size.
> 
> It is wrong that max_alloc_size is divided by 2 in the
> case of successful allocation as well, or invalid memory
> can be allocated, and leads to population failure, then errno
> other than ENOMEM may be returned.
> 
> Fixes: 3a3d0c75b43e ("mempool: fix slow allocation of large pools")
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Signed-off-by: Zhike Wang <wangzhike@jd.com>
> ---
>   lib/librte_mempool/rte_mempool.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> index a2bd249..7774f0c 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -635,7 +635,7 @@ struct pagesz_walk_arg {
>   				RTE_MIN((size_t)mem_size, max_alloc_size),
>   				mp->socket_id, mz_flags, align);
>   
> -			if (mz == NULL && rte_errno != ENOMEM)
> +			if (mz != NULL || rte_errno != ENOMEM)
>   				break;
>   
>   			max_alloc_size = RTE_MIN(max_alloc_size,
> 

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
  
Thomas Monjalon July 21, 2020, 11:27 p.m. UTC | #2
14/07/2020 11:32, Burakov, Anatoly:
> On 14-Jul-20 8:26 AM, Zhike Wang wrote:
> > If allocation is successful on the first attempt, typically
> > there is no problem since we allocated everything required and
> > we'll terminate the loop (if memory chunk is really sufficient
> > to populate required number of mempool elements).
> > 
> > If the first attempt fails, we try to allocate half
> > of mem_size and it succeed, we'll have one more iteration of
> > the for-loop to allocate memory for remaining elements and
> > should not try the next time with quarter of the mem_size.
> > 
> > It is wrong that max_alloc_size is divided by 2 in the
> > case of successful allocation as well, or invalid memory
> > can be allocated, and leads to population failure, then errno
> > other than ENOMEM may be returned.
> > 
> > Fixes: 3a3d0c75b43e ("mempool: fix slow allocation of large pools")

Cc: stable@dpdk.org

> > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > Signed-off-by: Zhike Wang <wangzhike@jd.com>
> 
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks
  

Patch

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index a2bd249..7774f0c 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -635,7 +635,7 @@  struct pagesz_walk_arg {
 				RTE_MIN((size_t)mem_size, max_alloc_size),
 				mp->socket_id, mz_flags, align);
 
-			if (mz == NULL && rte_errno != ENOMEM)
+			if (mz != NULL || rte_errno != ENOMEM)
 				break;
 
 			max_alloc_size = RTE_MIN(max_alloc_size,