Re: [dpdk-dev] [PATCH] mempool: fix memory allocation in memzones during retry.

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 13-Jul-20 4:40 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.
> 
> 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..b8f2629 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) || (mz == NULL && rte_errno != ENOMEM))

I think checking mz == NULL for the second time is redundant, as if 
we're hitting the second branch, we've already failed the "mz != NULL" 
test and can therefore assume that mz == NULL.

That said, i'm struggling to think of circumstances where this would 
matter. Could you please provide an example?

>   				break;
>   
>   			max_alloc_size = RTE_MIN(max_alloc_size,
> 

This should have a Fixes: tag.

-- 
Thanks,
Anatoly