[PATCH] mempool: rename functions with confusing names

[PATCH] mempool: rename functions with confusing names

[Bruce Richardson]

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_unallocated_count to replace rte_mempool_count
* rte_mempool_allocated_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@intel.com>
---
 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..e5dfa97 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_unallocated_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_unallocated_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_unallocated_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..351697d 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_unallocated_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_unallocated_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_unallocated_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..f2c2006 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_unallocated_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_unallocated_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..6ea678a 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_unallocated_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_allocated_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..ec7cea6 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_unallocated_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..41421aa 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_unallocated_count and
+  rte_mempool_allocated_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 94f3c78..f2ff5f0 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_unallocated_count(rxq->mb_pool),
+			   rte_mempool_allocated_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..5812c96 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_unallocated_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..e9d0ede 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_unallocated_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_unallocated_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..642dc77 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_unallocated_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_allocated_count(const struct rte_mempool *mp)
+{
+	return mp->size - rte_mempool_unallocated_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_allocated_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_unallocated_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_unallocated_count(mp) == 0);
 }
 
 /**
diff --git a/lib/librte_mempool/rte_mempool_version.map b/lib/librte_mempool/rte_mempool_version.map
index 9bcbf17..b98a0f0 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_unallocated_count;
 
 } DPDK_2.0;
-- 
2.5.5

Re: [PATCH] mempool: rename functions with confusing names

[Thomas Monjalon]

2016-06-29 14:55, Bruce Richardson:
> 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_unallocated_count to replace rte_mempool_count
> * rte_mempool_allocated_count to replace rte_mempool_free_count

What about available/used instead of unallocated/allocated?

Re: [PATCH] mempool: rename functions with confusing names

[Bruce Richardson]

On Wed, Jun 29, 2016 at 05:55:27PM +0200, Thomas Monjalon wrote:
> 2016-06-29 14:55, Bruce Richardson:
> > 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_unallocated_count to replace rte_mempool_count
> > * rte_mempool_allocated_count to replace rte_mempool_free_count
> 
> What about available/used instead of unallocated/allocated?
> 

I don't particularly mind what the name is, to be honest. I like "avail"
because it is shorter, but I'm a little uncertain about "used", because it
implies that the entries are finished with i.e. like a used match, or tissue :-)

How about "avail/in_use"?

/Bruce

Re: [PATCH] mempool: rename functions with confusing names

[Wiles, Keith]

On 6/29/16, 11:00 AM, "dev on behalf of Bruce Richardson" <dev-bounces@dpdk.org on behalf of bruce.richardson@intel.com> wrote:

>On Wed, Jun 29, 2016 at 05:55:27PM +0200, Thomas Monjalon wrote:
>> 2016-06-29 14:55, Bruce Richardson:
>> > 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_unallocated_count to replace rte_mempool_count
>> > * rte_mempool_allocated_count to replace rte_mempool_free_count
>> 
>> What about available/used instead of unallocated/allocated?
>> 
>
>I don't particularly mind what the name is, to be honest. I like "avail"
>because it is shorter, but I'm a little uncertain about "used", because it
>implies that the entries are finished with i.e. like a used match, or tissue :-)
>
>How about "avail/in_use"?

+1 for those names.
>
>/Bruce
>

Re: [PATCH] mempool: rename functions with confusing names

[Olivier MATZ]

On 06/29/2016 06:02 PM, Wiles, Keith wrote:
>
> On 6/29/16, 11:00 AM, "dev on behalf of Bruce Richardson" <dev-bounces@dpdk.org on behalf of bruce.richardson@intel.com> wrote:
>
>> On Wed, Jun 29, 2016 at 05:55:27PM +0200, Thomas Monjalon wrote:
>>> 2016-06-29 14:55, Bruce Richardson:
>>>> 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.

I agree the current API is not appropriate.


>>>> Fix this by introducing two new functions to replace the old ones,
>>>> * rte_mempool_unallocated_count to replace rte_mempool_count
>>>> * rte_mempool_allocated_count to replace rte_mempool_free_count
>>>
>>> What about available/used instead of unallocated/allocated?
>>>
>>
>> I don't particularly mind what the name is, to be honest. I like "avail"
>> because it is shorter, but I'm a little uncertain about "used", because it
>> implies that the entries are finished with i.e. like a used match, or tissue :-)
>>
>> How about "avail/in_use"?
>
> +1 for those names.

+1 too.

rte_mempool_avail_count()
rte_mempool_in_use_count()


Thanks

Re: [PATCH] mempool: rename functions with confusing names

[Richardson, Bruce]

> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Wednesday, June 29, 2016 5:05 PM
> To: Wiles, Keith <keith.wiles@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Thomas Monjalon <thomas.monjalon@6wind.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] mempool: rename functions with confusing
> names
> 
> 
> 
> On 06/29/2016 06:02 PM, Wiles, Keith wrote:
> >
> > On 6/29/16, 11:00 AM, "dev on behalf of Bruce Richardson" <dev-
> bounces@dpdk.org on behalf of bruce.richardson@intel.com> wrote:
> >
> >> On Wed, Jun 29, 2016 at 05:55:27PM +0200, Thomas Monjalon wrote:
> >>> 2016-06-29 14:55, Bruce Richardson:
> >>>> 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.
> 
> I agree the current API is not appropriate.
> 
> 
> >>>> Fix this by introducing two new functions to replace the old ones,
> >>>> * rte_mempool_unallocated_count to replace rte_mempool_count
> >>>> * rte_mempool_allocated_count to replace rte_mempool_free_count
> >>>
> >>> What about available/used instead of unallocated/allocated?
> >>>
> >>
> >> I don't particularly mind what the name is, to be honest. I like
> "avail"
> >> because it is shorter, but I'm a little uncertain about "used",
> >> because it implies that the entries are finished with i.e. like a
> >> used match, or tissue :-)
> >>
> >> How about "avail/in_use"?
> >
> > +1 for those names.
> 
> +1 too.
> 
> rte_mempool_avail_count()
> rte_mempool_in_use_count()
> 

Ok, I'll see about doing a V2 for review.

/Bruce

[PATCH v2] mempool: rename functions with confusing names

[Bruce Richardson]

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@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

Re: [PATCH v2] mempool: rename functions with confusing names

[Bruce Richardson]

+ 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@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
> 

Re: [PATCH v2] mempool: rename functions with confusing names

[Thomas Monjalon]

2016-06-29 17:27, Bruce Richardson:
> 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

This patch needs to be rebased please.

> --- 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;

The "in_use_count" function is missing.
Please keep alphabetical order.

Re: [PATCH v2] mempool: rename functions with confusing names

[Panu Matilainen]

On 06/30/2016 03:00 PM, Thomas Monjalon wrote:
> 2016-06-29 17:27, Bruce Richardson:
>> 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
>
> This patch needs to be rebased please.
>
>> --- 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;
>
> The "in_use_count" function is missing.

Its missing because the function is static inline. If you ask me, this 
would be as fine time as any to remove that inlining ;)

	- Panu -

> Please keep alphabetical order.
>

Re: [PATCH v2] mempool: rename functions with confusing names

[Bruce Richardson]

On Thu, Jun 30, 2016 at 02:00:16PM +0200, Thomas Monjalon wrote:
> 2016-06-29 17:27, Bruce Richardson:
> > 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
> 
> This patch needs to be rebased please.
> 
> > --- 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;
> 
> The "in_use_count" function is missing.
> Please keep alphabetical order.

We need a search-replace function that is smart enough to reorder things for us!

Re: [PATCH v2] mempool: rename functions with confusing names

[Bruce Richardson]

On Thu, Jun 30, 2016 at 03:02:22PM +0300, Panu Matilainen wrote:
> On 06/30/2016 03:00 PM, Thomas Monjalon wrote:
> >2016-06-29 17:27, Bruce Richardson:
> >>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
> >
> >This patch needs to be rebased please.
> >
> >>--- 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;
> >
> >The "in_use_count" function is missing.
> 
> Its missing because the function is static inline. If you ask me, this would
> be as fine time as any to remove that inlining ;)
> 
> 	- Panu -
> 
Yep, and yep.
V3 needed, I'll see what I can do.

/Bruce

> >Please keep alphabetical order.
> >
> 
> 

[PATCH v3] mempool: rename functions with confusing names

[Bruce Richardson]

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@intel.com>
---

v3 changes:
* made rte_mempool_in_use_count a regular, not static inline, function
* fixed alphabetical ordering of functions in the map file

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           | 17 +++++++++++--
 lib/librte_mempool/rte_mempool.h           | 39 +++++++++++++++++++++++++++---
 lib/librte_mempool/rte_mempool_version.map |  2 ++
 12 files changed, 74 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 63c61f3..238ff52 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -231,7 +231,7 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 	printf("get object count\n");
 	/* We have to count the extra caches, one in this case. */
 	offset = use_external_cache ? 1 * cache->len : 0;
-	if (rte_mempool_count(mp) + offset != MEMPOOL_SIZE - 1)
+	if (rte_mempool_avail_count(mp) + offset != MEMPOOL_SIZE - 1)
 		GOTO_ERR(ret, out);
 
 	printf("get private data\n");
@@ -497,7 +497,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 b80a1dd..4fac04c 100644
--- a/app/test/test_mempool_perf.c
+++ b/app/test/test_mempool_perf.c
@@ -240,7 +240,7 @@ launch_cores(unsigned cores)
 		   external_cache_size : (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 56cb1d9..f502f86 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -32,6 +32,12 @@ Deprecation Notices
   PKT_RX_QINQ_STRIPPED, that are better described. The old flags and
   their behavior will be kept in 16.07 and will be removed in 16.11.
 
+* 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 mempool functions for single/multi producer/consumer are deprecated and
   will be removed in 16.11.
   It is replaced by rte_mempool_generic_get/put functions.
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 4f159fc..d78d02b 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -960,8 +960,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;
@@ -983,6 +983,19 @@ rte_mempool_count(const struct rte_mempool *mp)
 	return count;
 }
 
+/* return the number of entries allocated from the mempool */
+unsigned int
+rte_mempool_in_use_count(const struct rte_mempool *mp)
+{
+	return mp->size - rte_mempool_avail_count(mp);
+}
+
+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 b2a5197..e94a6d7 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -1499,9 +1499,41 @@ 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.
+ */
+unsigned int
+rte_mempool_in_use_count(const struct rte_mempool *mp);
+
+/**
+ * @deprecated
  * Return the number of free entries in the mempool ring.
  * i.e. how many entries can be freed back to the mempool.
  *
@@ -1518,10 +1550,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);
 }
 
 /**
@@ -1540,7 +1573,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);
 }
 
 /**
@@ -1559,7 +1592,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 729ea97..dee1c99 100644
--- a/lib/librte_mempool/rte_mempool_version.map
+++ b/lib/librte_mempool/rte_mempool_version.map
@@ -19,6 +19,7 @@ DPDK_2.0 {
 DPDK_16.07 {
 	global:
 
+	rte_mempool_avail_count;
 	rte_mempool_cache_create;
 	rte_mempool_cache_flush;
 	rte_mempool_cache_free;
@@ -28,6 +29,7 @@ DPDK_16.07 {
 	rte_mempool_free;
 	rte_mempool_generic_get;
 	rte_mempool_generic_put;
+	rte_mempool_in_use_count;
 	rte_mempool_mem_iter;
 	rte_mempool_obj_iter;
 	rte_mempool_ops_table;
-- 
2.5.5

Re: [PATCH v3] mempool: rename functions with confusing names

[Thomas Monjalon]

2016-06-30 13:49, Bruce Richardson:
> 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.

The ThunderX driver uses rte_mempool_count and needs an update.
Except that, it looks good.

Re: [PATCH v3] mempool: rename functions with confusing names

[Olivier MATZ]

On 06/30/2016 10:05 PM, Thomas Monjalon wrote:
> 2016-06-30 13:49, Bruce Richardson:
>> 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.
> 
> The ThunderX driver uses rte_mempool_count and needs an update.
> Except that, it looks good.
> 

Yep, I don't see any other issue.

Re: [PATCH v3] mempool: rename functions with confusing names

[Thomas Monjalon]

2016-07-01 10:57, Olivier MATZ:
> 
> On 06/30/2016 10:05 PM, Thomas Monjalon wrote:
> > 2016-06-30 13:49, Bruce Richardson:
> >> 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.
> > 
> > The ThunderX driver uses rte_mempool_count and needs an update.
> > Except that, it looks good.
> > 
> 
> Yep, I don't see any other issue.

Applied with ThunderX update, thanks