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