DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] mempool: return ENOMEM if initial alloc size can not be satisfied
@ 2020-07-03  8:41 Zhike Wang
  2020-07-03  9:23 ` Andrew Rybchenko
  0 siblings, 1 reply; 3+ messages in thread
From: Zhike Wang @ 2020-07-03  8:41 UTC (permalink / raw)
  To: dev; +Cc: olivier.matz, arybchenko, Zhike Wang

Signed-off-by: Zhike Wang <wangzhike@jd.com>
---
 lib/librte_mempool/rte_mempool.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index 0bde995..b24feb6 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -622,6 +622,12 @@ struct pagesz_walk_arg {
 			goto fail;
 		}
 
+		if (max_alloc_size < min_chunk_size) {
+			rte_errno = ENOMEM;
+			ret = -rte_errno;
+			goto fail;
+		}
+
 		/* if we're trying to reserve contiguous memory, add appropriate
 		 * memzone flag.
 		 */
-- 
1.8.3.1



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] mempool: return ENOMEM if initial alloc size can not be satisfied
  2020-07-03  8:41 [dpdk-dev] [PATCH] mempool: return ENOMEM if initial alloc size can not be satisfied Zhike Wang
@ 2020-07-03  9:23 ` Andrew Rybchenko
  2020-07-10  8:44   ` 王志克
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Rybchenko @ 2020-07-03  9:23 UTC (permalink / raw)
  To: Zhike Wang, dev; +Cc: olivier.matz

On 7/3/20 11:41 AM, Zhike Wang wrote:
> Signed-off-by: Zhike Wang <wangzhike@jd.com>
> ---
>  lib/librte_mempool/rte_mempool.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> index 0bde995..b24feb6 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -622,6 +622,12 @@ struct pagesz_walk_arg {
>  			goto fail;
>  		}
>  
> +		if (max_alloc_size < min_chunk_size) {
> +			rte_errno = ENOMEM;
> +			ret = -rte_errno;
> +			goto fail;
> +		}
> +
>  		/* if we're trying to reserve contiguous memory, add appropriate
>  		 * memzone flag.
>  		 */
> 

As far as I can see there is really a bug in nearby code, but
the fix suggested here is a wrong direction.

The check is already present below in do-while loop condition,
but it is wrong that max_alloc_size is divided by 2 in the
case of successful allocation as well.
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).

However, 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.
mem_size will be recalculated, but max_alloc_size will limit
allocation attempt size.

So, I think it is required to add "mz != NULL ||" to the
if condition in do-while loop. It will guarantee that
max_alloc_size is reduced if and only if mz == NULL and
if it becomes smaller than min_chunk_size, if condition
after do-while loop will return error.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] mempool: return ENOMEM if initial alloc size can not be satisfied
  2020-07-03  9:23 ` Andrew Rybchenko
@ 2020-07-10  8:44   ` 王志克
  0 siblings, 0 replies; 3+ messages in thread
From: 王志克 @ 2020-07-10  8:44 UTC (permalink / raw)
  To: Andrew Rybchenko, dev; +Cc: olivier.matz

Thanks, you are right.

Will send new patch.

Br,
Zhike Wang 
JDCloud, Product Development, IaaS   
------------------------------------------------------------------------------------------------
Mobile/+86 13466719566
E- mail/wangzhike@jd.com
Address/5F Building A,North-Star Century Center,8 Beichen West Street,Chaoyang District Beijing
Https://JDCloud.com
------------------------------------------------------------------------------------------------



-----Original Message-----
From: Andrew Rybchenko [mailto:arybchenko@solarflare.com] 
Sent: Friday, July 03, 2020 5:23 PM
To: 王志克; dev@dpdk.org
Cc: olivier.matz@6wind.com
Subject: Re: [dpdk-dev] [PATCH] mempool: return ENOMEM if initial alloc size can not be satisfied

On 7/3/20 11:41 AM, Zhike Wang wrote:
> Signed-off-by: Zhike Wang <wangzhike@jd.com>
> ---
>  lib/librte_mempool/rte_mempool.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> index 0bde995..b24feb6 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -622,6 +622,12 @@ struct pagesz_walk_arg {
>  			goto fail;
>  		}
>  
> +		if (max_alloc_size < min_chunk_size) {
> +			rte_errno = ENOMEM;
> +			ret = -rte_errno;
> +			goto fail;
> +		}
> +
>  		/* if we're trying to reserve contiguous memory, add appropriate
>  		 * memzone flag.
>  		 */
> 

As far as I can see there is really a bug in nearby code, but
the fix suggested here is a wrong direction.

The check is already present below in do-while loop condition,
but it is wrong that max_alloc_size is divided by 2 in the
case of successful allocation as well.
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).

However, 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.
mem_size will be recalculated, but max_alloc_size will limit
allocation attempt size.

So, I think it is required to add "mz != NULL ||" to the
if condition in do-while loop. It will guarantee that
max_alloc_size is reduced if and only if mz == NULL and
if it becomes smaller than min_chunk_size, if condition
after do-while loop will return error.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-07-10  8:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-03  8:41 [dpdk-dev] [PATCH] mempool: return ENOMEM if initial alloc size can not be satisfied Zhike Wang
2020-07-03  9:23 ` Andrew Rybchenko
2020-07-10  8:44   ` 王志克

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).