[RFC,1/4] mempool: clarify default populate function
Checks
Commit Message
No functional change. Clarify the populate function to make
the next commit easier to understand.
Rename the variables:
- to avoid negation in the name
- to have more understandable names
Remove useless variable (no_pageshift is equivalent to pg_sz == 0).
Remove duplicate affectation of "external" variable.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
lib/librte_mempool/rte_mempool.c | 50 +++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 24 deletions(-)
Comments
On 7/19/19 4:38 PM, Olivier Matz wrote:
> No functional change. Clarify the populate function to make
> the next commit easier to understand.
>
> Rename the variables:
> - to avoid negation in the name
> - to have more understandable names
>
> Remove useless variable (no_pageshift is equivalent to pg_sz == 0).
>
> Remove duplicate affectation of "external" variable.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
LGTM
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
On Fri, Jul 19, 2019 at 06:42:06PM +0300, Andrew Rybchenko wrote:
> On 7/19/19 4:38 PM, Olivier Matz wrote:
> > No functional change. Clarify the populate function to make
> > the next commit easier to understand.
> >
> > Rename the variables:
> > - to avoid negation in the name
> > - to have more understandable names
> >
> > Remove useless variable (no_pageshift is equivalent to pg_sz == 0).
> >
> > Remove duplicate affectation of "external" variable.
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>
> LGTM
>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
>
I submitted this one in a separate patch:
http://patchwork.dpdk.org/patch/60687/
@@ -429,24 +429,18 @@ rte_mempool_populate_default(struct rte_mempool *mp)
rte_iova_t iova;
unsigned mz_id, n;
int ret;
- bool no_contig, try_contig, no_pageshift, external;
+ bool need_iova_contig_obj;
+ bool try_iova_contig_mempool;
+ bool alloc_in_ext_mem;
ret = mempool_ops_alloc_once(mp);
if (ret != 0)
return ret;
- /* check if we can retrieve a valid socket ID */
- ret = rte_malloc_heap_socket_is_external(mp->socket_id);
- if (ret < 0)
- return -EINVAL;
- external = ret;
-
/* mempool must not be populated */
if (mp->nb_mem_chunks != 0)
return -EEXIST;
- no_contig = mp->flags & MEMPOOL_F_NO_IOVA_CONTIG;
-
/*
* the following section calculates page shift and page size values.
*
@@ -496,16 +490,23 @@ rte_mempool_populate_default(struct rte_mempool *mp)
* to go for contiguous memory even if we're in no-huge mode, because
* external memory may in fact be IOVA-contiguous.
*/
- external = rte_malloc_heap_socket_is_external(mp->socket_id) == 1;
- no_pageshift = no_contig ||
- (!external && rte_eal_iova_mode() == RTE_IOVA_VA);
- try_contig = !no_contig && !no_pageshift &&
- (rte_eal_has_hugepages() || external);
- if (no_pageshift) {
+ /* check if we can retrieve a valid socket ID */
+ ret = rte_malloc_heap_socket_is_external(mp->socket_id);
+ if (ret < 0)
+ return -EINVAL;
+ alloc_in_ext_mem = (ret == 1);
+ need_iova_contig_obj = !(mp->flags & MEMPOOL_F_NO_IOVA_CONTIG);
+ try_iova_contig_mempool = false;
+
+ if (!need_iova_contig_obj) {
+ pg_sz = 0;
+ pg_shift = 0;
+ } else if (!alloc_in_ext_mem && rte_eal_iova_mode() == RTE_IOVA_VA) {
pg_sz = 0;
pg_shift = 0;
- } else if (try_contig) {
+ } else if (rte_eal_has_hugepages() || alloc_in_ext_mem) {
+ try_iova_contig_mempool = true;
pg_sz = get_min_page_size(mp->socket_id);
pg_shift = rte_bsf32(pg_sz);
} else {
@@ -517,7 +518,7 @@ rte_mempool_populate_default(struct rte_mempool *mp)
size_t min_chunk_size;
unsigned int flags;
- if (try_contig || no_pageshift)
+ if (try_iova_contig_mempool || pg_sz == 0)
mem_size = rte_mempool_ops_calc_mem_size(mp, n,
0, &min_chunk_size, &align);
else
@@ -541,7 +542,7 @@ rte_mempool_populate_default(struct rte_mempool *mp)
/* if we're trying to reserve contiguous memory, add appropriate
* memzone flag.
*/
- if (try_contig)
+ if (try_iova_contig_mempool)
flags |= RTE_MEMZONE_IOVA_CONTIG;
mz = rte_memzone_reserve_aligned(mz_name, mem_size,
@@ -551,8 +552,9 @@ rte_mempool_populate_default(struct rte_mempool *mp)
* minimum required contiguous chunk fits minimum page, adjust
* memzone size to the page size, and try again.
*/
- if (mz == NULL && try_contig && min_chunk_size <= pg_sz) {
- try_contig = false;
+ if (mz == NULL && try_iova_contig_mempool &&
+ min_chunk_size <= pg_sz) {
+ try_iova_contig_mempool = false;
flags &= ~RTE_MEMZONE_IOVA_CONTIG;
mem_size = rte_mempool_ops_calc_mem_size(mp, n,
@@ -587,12 +589,12 @@ rte_mempool_populate_default(struct rte_mempool *mp)
goto fail;
}
- if (no_contig)
- iova = RTE_BAD_IOVA;
- else
+ if (need_iova_contig_obj)
iova = mz->iova;
+ else
+ iova = RTE_BAD_IOVA;
- if (no_pageshift || try_contig)
+ if (try_iova_contig_mempool || pg_sz == 0)
ret = rte_mempool_populate_iova(mp, mz->addr,
iova, mz->len,
rte_mempool_memchunk_mz_free,