+ Correct email for Thomas :-(
On Wed, Jun 29, 2016 at 05:27:15PM +0100, Bruce Richardson wrote:
> The mempool_count and mempool_free_count behaved contrary to what their
> names suggested. The free_count function actually returned the number of
> elements that were allocated from the pool, not the number unallocated as
> the name implied.
>
> Fix this by introducing two new functions to replace the old ones,
> * rte_mempool_avail_count to replace rte_mempool_count
> * rte_mempool_in_use_count to replace rte_mempool_free_count
>
> In this patch, the new functions are added, and the old ones are marked
> as deprecated. All apps and examples that use the old functions are
> updated to use the new functions.
>
> Fixes: af75078fece3 ("first public release")
>
> Signed-off-by: Bruce Richardson <bruce.richardson at intel.com>
> ---
>
> v2 changes
> * original patch used allocated/unallocated for new functions. Change
> those to in_use/avail.
>
> ---
> app/test/test_cryptodev.c | 6 ++---
> app/test/test_cryptodev_perf.c | 6 ++---
> app/test/test_mbuf.c | 4 +--
> app/test/test_mempool.c | 4 +--
> app/test/test_mempool_perf.c | 2 +-
> doc/guides/rel_notes/deprecation.rst | 6 +++++
> drivers/net/qede/qede_rxtx.c | 4 +--
> examples/multi_process/l2fwd_fork/main.c | 3 ++-
> examples/qos_sched/main.c | 2 --
> lib/librte_mempool/rte_mempool.c | 10 +++++--
> lib/librte_mempool/rte_mempool.h | 42
> +++++++++++++++++++++++++++---
> lib/librte_mempool/rte_mempool_version.map | 1 +
> 12 files changed, 69 insertions(+), 21 deletions(-)
>
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> index 9dfe34f..fbfe1d0 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -316,12 +316,12 @@ testsuite_teardown(void)
>
> if (ts_params->mbuf_pool != NULL) {
> RTE_LOG(DEBUG, USER1, "CRYPTO_MBUFPOOL count %u\n",
> - rte_mempool_count(ts_params->mbuf_pool));
> + rte_mempool_avail_count(ts_params->mbuf_pool));
> }
>
> if (ts_params->op_mpool != NULL) {
> RTE_LOG(DEBUG, USER1, "CRYPTO_OP_POOL count %u\n",
> - rte_mempool_count(ts_params->op_mpool));
> + rte_mempool_avail_count(ts_params->op_mpool));
> }
>
> }
> @@ -412,7 +412,7 @@ ut_teardown(void)
>
> if (ts_params->mbuf_pool != NULL)
> RTE_LOG(DEBUG, USER1, "CRYPTO_MBUFPOOL count %u\n",
> - rte_mempool_count(ts_params->mbuf_pool));
> + rte_mempool_avail_count(ts_params->mbuf_pool));
>
> rte_cryptodev_stats_get(ts_params->valid_devs[0], &stats);
>
> diff --git a/app/test/test_cryptodev_perf.c b/app/test/test_cryptodev_perf.c
> index e484cbb..d728211 100644
> --- a/app/test/test_cryptodev_perf.c
> +++ b/app/test/test_cryptodev_perf.c
> @@ -343,10 +343,10 @@ testsuite_teardown(void)
>
> if (ts_params->mbuf_mp != NULL)
> RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_MBUFPOOL count %u\n",
> - rte_mempool_count(ts_params->mbuf_mp));
> + rte_mempool_avail_count(ts_params->mbuf_mp));
> if (ts_params->op_mpool != NULL)
> RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_OP POOL count %u\n",
> - rte_mempool_count(ts_params->op_mpool));
> + rte_mempool_avail_count(ts_params->op_mpool));
> }
>
> static int
> @@ -395,7 +395,7 @@ ut_teardown(void)
>
> if (ts_params->mbuf_mp != NULL)
> RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_MBUFPOOL count %u\n",
> - rte_mempool_count(ts_params->mbuf_mp));
> + rte_mempool_avail_count(ts_params->mbuf_mp));
>
> rte_cryptodev_stats_get(ts_params->dev_id, &stats);
>
> diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
> index 1835acc..8664885 100644
> --- a/app/test/test_mbuf.c
> +++ b/app/test/test_mbuf.c
> @@ -717,7 +717,7 @@ test_refcnt_iter(unsigned lcore, unsigned iter)
> * - increment it's reference up to N+1,
> * - enqueue it N times into the ring for slave cores to free.
> */
> - for (i = 0, n = rte_mempool_count(refcnt_pool);
> + for (i = 0, n = rte_mempool_avail_count(refcnt_pool);
> i != n && (m = rte_pktmbuf_alloc(refcnt_pool)) != NULL;
> i++) {
> ref = RTE_MAX(rte_rand() % REFCNT_MAX_REF, 1UL);
> @@ -745,7 +745,7 @@ test_refcnt_iter(unsigned lcore, unsigned iter)
>
> /* check that all mbufs are back into mempool by now */
> for (wn = 0; wn != REFCNT_MAX_TIMEOUT; wn++) {
> - if ((i = rte_mempool_count(refcnt_pool)) == n) {
> + if ((i = rte_mempool_avail_count(refcnt_pool)) == n) {
> refcnt_lcore[lcore] += tref;
> printf("%s(lcore=%u, iter=%u) completed, "
> "%u references processed\n",
> diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
> index 31582d8..db159aa 100644
> --- a/app/test/test_mempool.c
> +++ b/app/test/test_mempool.c
> @@ -210,7 +210,7 @@ test_mempool_basic(struct rte_mempool *mp)
>
> /* tests that improve coverage */
> printf("get object count\n");
> - if (rte_mempool_count(mp) != MEMPOOL_SIZE - 1)
> + if (rte_mempool_avail_count(mp) != MEMPOOL_SIZE - 1)
> RET_ERR();
>
> printf("get private data\n");
> @@ -470,7 +470,7 @@ test_mempool_basic_ex(struct rte_mempool *mp)
> return ret;
> }
> printf("test_mempool_basic_ex now mempool (%s) has %u free entries\n",
> - mp->name, rte_mempool_free_count(mp));
> + mp->name, rte_mempool_in_use_count(mp));
> if (rte_mempool_full(mp) != 1) {
> printf("test_mempool_basic_ex the mempool should be full\n");
> goto fail_mp_basic_ex;
> diff --git a/app/test/test_mempool_perf.c b/app/test/test_mempool_perf.c
> index c5f8455..e638a4a 100644
> --- a/app/test/test_mempool_perf.c
> +++ b/app/test/test_mempool_perf.c
> @@ -201,7 +201,7 @@ launch_cores(unsigned cores)
> "n_put_bulk=%u n_keep=%u ",
> (unsigned) mp->cache_size, cores, n_get_bulk, n_put_bulk,
> n_keep);
>
> - if (rte_mempool_count(mp) != MEMPOOL_SIZE) {
> + if (rte_mempool_avail_count(mp) != MEMPOOL_SIZE) {
> printf("mempool is not full\n");
> return -1;
> }
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 80f873d..1472db2 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -34,6 +34,12 @@ Deprecation Notices
> compact API. The ones that remain are backwards compatible and use the
> per-lcore default cache if available. This change targets release 16.07.
>
> +* The APIs rte_mempool_count and rte_mempool_free_count are being deprecated
> + on the basis that they are confusing to use - free_count actually returns
> + the number of allocated entries, not the number of free entries as
> expected.
> + They are being replaced by rte_mempool_avail_count and
> + rte_mempool_in_use_count respectively.
> +
> * The mbuf flags PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT are deprecated and
> are respectively replaced by PKT_RX_VLAN_STRIPPED and
> PKT_RX_QINQ_STRIPPED, that are better described. The old flags and
> diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c
> index ccce5fd..b5a40fe 100644
> --- a/drivers/net/qede/qede_rxtx.c
> +++ b/drivers/net/qede/qede_rxtx.c
> @@ -23,8 +23,8 @@ static inline int qede_alloc_rx_buffer(struct qede_rx_queue
> *rxq)
> "Failed to allocate rx buffer "
> "sw_rx_prod %u sw_rx_cons %u mp entries %u free %u",
> idx, rxq->sw_rx_cons & NUM_RX_BDS(rxq),
> - rte_mempool_count(rxq->mb_pool),
> - rte_mempool_free_count(rxq->mb_pool));
> + rte_mempool_avail_count(rxq->mb_pool),
> + rte_mempool_in_use_count(rxq->mb_pool));
> return -ENOMEM;
> }
> rxq->sw_rx_ring[idx].mbuf = new_mb;
> diff --git a/examples/multi_process/l2fwd_fork/main.c
> b/examples/multi_process/l2fwd_fork/main.c
> index 368b309..2d951d9 100644
> --- a/examples/multi_process/l2fwd_fork/main.c
> +++ b/examples/multi_process/l2fwd_fork/main.c
> @@ -442,7 +442,8 @@ reset_slave_all_ports(unsigned slaveid)
> pool = rte_mempool_lookup(buf_name);
> if (pool)
> printf("Port %d mempool free object is %u(%u)\n",
> slave->port[i],
> - rte_mempool_count(pool), (unsigned)NB_MBUF);
> + rte_mempool_avail_count(pool),
> + (unsigned int)NB_MBUF);
> else
> printf("Can't find mempool %s\n", buf_name);
>
> diff --git a/examples/qos_sched/main.c b/examples/qos_sched/main.c
> index e16b164..e10cfd4 100644
> --- a/examples/qos_sched/main.c
> +++ b/examples/qos_sched/main.c
> @@ -201,8 +201,6 @@ app_stat(void)
> stats.oerrors - tx_stats[i].oerrors);
> memcpy(&tx_stats[i], &stats, sizeof(stats));
>
> - //printf("MP = %d\n",
> rte_mempool_count(conf->app_pktmbuf_pool));
> -
> #if APP_COLLECT_STAT
> printf("-------+------------+------------+\n");
> printf(" | received | dropped |\n");
> diff --git a/lib/librte_mempool/rte_mempool.c
> b/lib/librte_mempool/rte_mempool.c
> index e6a83d0..c86d3fd 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -905,8 +905,8 @@ rte_mempool_xmem_create(const char *name, unsigned n,
> unsigned elt_size,
> }
>
> /* Return the number of entries in the mempool */
> -unsigned
> -rte_mempool_count(const struct rte_mempool *mp)
> +unsigned int
> +rte_mempool_avail_count(const struct rte_mempool *mp)
> {
> unsigned count;
> unsigned lcore_id;
> @@ -928,6 +928,12 @@ rte_mempool_count(const struct rte_mempool *mp)
> return count;
> }
>
> +unsigned int
> +rte_mempool_count(const struct rte_mempool *mp)
> +{
> + return rte_mempool_avail_count(mp);
> +}
> +
> /* dump the cache status */
> static unsigned
> rte_mempool_dump_cache(FILE *f, const struct rte_mempool *mp)
> diff --git a/lib/librte_mempool/rte_mempool.h
> b/lib/librte_mempool/rte_mempool.h
> index 0a1777c..389ef16 100644
> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h
> @@ -1368,9 +1368,44 @@ rte_mempool_get(struct rte_mempool *mp, void **obj_p)
> * @return
> * The number of entries in the mempool.
> */
> +unsigned int rte_mempool_avail_count(const struct rte_mempool *mp);
> +
> +/**
> + * @deprecated
> + * Return the number of entries in the mempool.
> + *
> + * When cache is enabled, this function has to browse the length of
> + * all lcores, so it should not be used in a data path, but only for
> + * debug purposes.
> + *
> + * @param mp
> + * A pointer to the mempool structure.
> + * @return
> + * The number of entries in the mempool.
> + */
> +__rte_deprecated
> unsigned rte_mempool_count(const struct rte_mempool *mp);
>
> /**
> + * Return the number of elements which have been allocated from the mempool
> + *
> + * When cache is enabled, this function has to browse the length of
> + * all lcores, so it should not be used in a data path, but only for
> + * debug purposes.
> + *
> + * @param mp
> + * A pointer to the mempool structure.
> + * @return
> + * The number of free entries in the mempool.
> + */
> +static inline unsigned int
> +rte_mempool_in_use_count(const struct rte_mempool *mp)
> +{
> + return mp->size - rte_mempool_avail_count(mp);
> +}
> +
> +/**
> + * @deprecated
> * Return the number of free entries in the mempool ring.
> * i.e. how many entries can be freed back to the mempool.
> *
> @@ -1387,10 +1422,11 @@ unsigned rte_mempool_count(const struct rte_mempool
> *mp);
> * @return
> * The number of free entries in the mempool.
> */
> +__rte_deprecated
> static inline unsigned
> rte_mempool_free_count(const struct rte_mempool *mp)
> {
> - return mp->size - rte_mempool_count(mp);
> + return rte_mempool_in_use_count(mp);
> }
>
> /**
> @@ -1409,7 +1445,7 @@ rte_mempool_free_count(const struct rte_mempool *mp)
> static inline int
> rte_mempool_full(const struct rte_mempool *mp)
> {
> - return !!(rte_mempool_count(mp) == mp->size);
> + return !!(rte_mempool_avail_count(mp) == mp->size);
> }
>
> /**
> @@ -1428,7 +1464,7 @@ rte_mempool_full(const struct rte_mempool *mp)
> static inline int
> rte_mempool_empty(const struct rte_mempool *mp)
> {
> - return !!(rte_mempool_count(mp) == 0);
> + return !!(rte_mempool_avail_count(mp) == 0);
> }
>
> /**
> diff --git a/lib/librte_mempool/rte_mempool_version.map
> b/lib/librte_mempool/rte_mempool_version.map
> index 9bcbf17..ffd0daf 100644
> --- a/lib/librte_mempool/rte_mempool_version.map
> +++ b/lib/librte_mempool/rte_mempool_version.map
> @@ -32,5 +32,6 @@ DPDK_16.07 {
> rte_mempool_populate_virt;
> rte_mempool_register_ops;
> rte_mempool_set_ops_byname;
> + rte_mempool_avail_count;
>
> } DPDK_2.0;
> --
> 2.5.5
>
