[4/5] mempool: introduce function to get mempool page size
Checks
Commit Message
In rte_mempool_populate_default(), we determine the page size,
which is needed for calc_size and allocation of memory.
Move this in a function and export it, it will be used in next
commit.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
lib/librte_mempool/rte_mempool.c | 51 ++++++++++++++--------
lib/librte_mempool/rte_mempool.h | 7 +++
lib/librte_mempool/rte_mempool_version.map | 1 +
3 files changed, 40 insertions(+), 19 deletions(-)
Comments
On 10/28/19 5:01 PM, Olivier Matz wrote:
> In rte_mempool_populate_default(), we determine the page size,
> which is needed for calc_size and allocation of memory.
>
> Move this in a function and export it, it will be used in next
> commit.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
One question below:
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
[snip]
> diff --git a/lib/librte_mempool/rte_mempool_version.map b/lib/librte_mempool/rte_mempool_version.map
> index 17cbca460..4eff2767d 100644
> --- a/lib/librte_mempool/rte_mempool_version.map
> +++ b/lib/librte_mempool/rte_mempool_version.map
> @@ -56,5 +56,6 @@ DPDK_18.05 {
> EXPERIMENTAL {
> global:
>
> + rte_mempool_get_page_size;
> rte_mempool_ops_get_info;
> };
Should internal function be here?
On Tue, Oct 29, 2019 at 01:31:22PM +0300, Andrew Rybchenko wrote:
> On 10/28/19 5:01 PM, Olivier Matz wrote:
> > In rte_mempool_populate_default(), we determine the page size,
> > which is needed for calc_size and allocation of memory.
> >
> > Move this in a function and export it, it will be used in next
> > commit.
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>
> One question below:
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
>
> [snip]
>
> > diff --git a/lib/librte_mempool/rte_mempool_version.map b/lib/librte_mempool/rte_mempool_version.map
> > index 17cbca460..4eff2767d 100644
> > --- a/lib/librte_mempool/rte_mempool_version.map
> > +++ b/lib/librte_mempool/rte_mempool_version.map
> > @@ -56,5 +56,6 @@ DPDK_18.05 {
> > EXPERIMENTAL {
> > global:
> > + rte_mempool_get_page_size;
> > rte_mempool_ops_get_info;
> > };
>
> Should internal function be here?
>
Good question. Let me ask a friend ;)
On Tue, Oct 29, 2019 at 06:20:47PM +0100, Olivier Matz wrote:
> On Tue, Oct 29, 2019 at 01:31:22PM +0300, Andrew Rybchenko wrote:
> > On 10/28/19 5:01 PM, Olivier Matz wrote:
> > > In rte_mempool_populate_default(), we determine the page size,
> > > which is needed for calc_size and allocation of memory.
> > >
> > > Move this in a function and export it, it will be used in next
> > > commit.
> > >
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> >
> > One question below:
> > Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >
> > [snip]
> >
> > > diff --git a/lib/librte_mempool/rte_mempool_version.map b/lib/librte_mempool/rte_mempool_version.map
> > > index 17cbca460..4eff2767d 100644
> > > --- a/lib/librte_mempool/rte_mempool_version.map
> > > +++ b/lib/librte_mempool/rte_mempool_version.map
> > > @@ -56,5 +56,6 @@ DPDK_18.05 {
> > > EXPERIMENTAL {
> > > global:
> > > + rte_mempool_get_page_size;
> > > rte_mempool_ops_get_info;
> > > };
> >
> > Should internal function be here?
> >
>
> Good question. Let me ask a friend ;)
I was influenced by a warning saying "rte_mempool_get_page_size is
flagged as experimental but is not listed in version map", but actually
it should not be flagged as experimental. I'll remove both.
My friend also suggested me to add it in a private header, which is a
good idea, but I think it should be in another patch because there are
already several functions in this case.
On Wed, Oct 30, 2019 at 09:32:08AM +0100, Olivier Matz wrote:
> On Tue, Oct 29, 2019 at 06:20:47PM +0100, Olivier Matz wrote:
> > On Tue, Oct 29, 2019 at 01:31:22PM +0300, Andrew Rybchenko wrote:
> > > On 10/28/19 5:01 PM, Olivier Matz wrote:
> > > > In rte_mempool_populate_default(), we determine the page size,
> > > > which is needed for calc_size and allocation of memory.
> > > >
> > > > Move this in a function and export it, it will be used in next
> > > > commit.
> > > >
> > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > >
> > > One question below:
> > > Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > >
> > > [snip]
> > >
> > > > diff --git a/lib/librte_mempool/rte_mempool_version.map b/lib/librte_mempool/rte_mempool_version.map
> > > > index 17cbca460..4eff2767d 100644
> > > > --- a/lib/librte_mempool/rte_mempool_version.map
> > > > +++ b/lib/librte_mempool/rte_mempool_version.map
> > > > @@ -56,5 +56,6 @@ DPDK_18.05 {
> > > > EXPERIMENTAL {
> > > > global:
> > > > + rte_mempool_get_page_size;
> > > > rte_mempool_ops_get_info;
> > > > };
> > >
> > > Should internal function be here?
> > >
> >
> > Good question. Let me ask a friend ;)
>
> I was influenced by a warning saying "rte_mempool_get_page_size is
> flagged as experimental but is not listed in version map", but actually
> it should not be flagged as experimental. I'll remove both.
>
> My friend also suggested me to add it in a private header, which is a
> good idea, but I think it should be in another patch because there are
> already several functions in this case.
Finally, I had to keep it in the API, because the octeontx2 driver will
need it. I also kept the @internal tag in the comment, because I think
this function should only be used in mempool drivers.
@@ -411,6 +411,33 @@ rte_mempool_populate_virt(struct rte_mempool *mp, char *addr,
return ret;
}
+/* Get the minimal page size used in a mempool before populating it. */
+int
+rte_mempool_get_page_size(struct rte_mempool *mp, size_t *pg_sz)
+{
+ bool need_iova_contig_obj;
+ bool alloc_in_ext_mem;
+ int 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;
+ alloc_in_ext_mem = (ret == 1);
+ need_iova_contig_obj = !(mp->flags & MEMPOOL_F_NO_IOVA_CONTIG);
+
+ if (!need_iova_contig_obj)
+ *pg_sz = 0;
+ else if (!alloc_in_ext_mem && rte_eal_iova_mode() == RTE_IOVA_VA)
+ *pg_sz = 0;
+ else if (rte_eal_has_hugepages() || alloc_in_ext_mem)
+ *pg_sz = get_min_page_size(mp->socket_id);
+ else
+ *pg_sz = getpagesize();
+
+ return 0;
+}
+
/* Default function to populate the mempool: allocate memory in memzones,
* and populate them. Return the number of objects added, or a negative
* value on error.
@@ -422,12 +449,11 @@ rte_mempool_populate_default(struct rte_mempool *mp)
char mz_name[RTE_MEMZONE_NAMESIZE];
const struct rte_memzone *mz;
ssize_t mem_size;
- size_t align, pg_sz, pg_shift;
+ size_t align, pg_sz, pg_shift = 0;
rte_iova_t iova;
unsigned mz_id, n;
int ret;
bool need_iova_contig_obj;
- bool alloc_in_ext_mem;
ret = mempool_ops_alloc_once(mp);
if (ret != 0)
@@ -482,26 +508,13 @@ rte_mempool_populate_default(struct rte_mempool *mp)
* synonymous with IOVA contiguousness will not hold.
*/
- /* 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);
+ ret = rte_mempool_get_page_size(mp, &pg_sz);
+ if (ret < 0)
+ return ret;
- 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 (rte_eal_has_hugepages() || alloc_in_ext_mem) {
- pg_sz = get_min_page_size(mp->socket_id);
- pg_shift = rte_bsf32(pg_sz);
- } else {
- pg_sz = getpagesize();
+ if (pg_sz != 0)
pg_shift = rte_bsf32(pg_sz);
- }
for (mz_id = 0, n = mp->size; n > 0; mz_id++, n -= ret) {
size_t min_chunk_size;
@@ -1691,6 +1691,13 @@ uint32_t rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags,
void rte_mempool_walk(void (*func)(struct rte_mempool *, void *arg),
void *arg);
+/**
+ * @internal Get page size used for mempool object allocation.
+ */
+__rte_experimental
+int
+rte_mempool_get_page_size(struct rte_mempool *mp, size_t *pg_sz);
+
#ifdef __cplusplus
}
#endif
@@ -56,5 +56,6 @@ DPDK_18.05 {
EXPERIMENTAL {
global:
+ rte_mempool_get_page_size;
rte_mempool_ops_get_info;
};