From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 851732BBA for ; Wed, 29 Jun 2016 18:30:25 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP; 29 Jun 2016 09:30:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,547,1459839600"; d="scan'208";a="130701962" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.220.81]) by fmsmga004.fm.intel.com with SMTP; 29 Jun 2016 09:30:23 -0700 Received: by (sSMTP sendmail emulation); Wed, 29 Jun 2016 17:30:21 +0025 Date: Wed, 29 Jun 2016 17:30:21 +0100 From: Bruce Richardson To: dev@dpdk.org Cc: olivier.matz@6wind.com, thomas.monjalon@6wind.com, keith.wiles@intel.com Message-ID: <20160629163021.GA13740@bricha3-MOBL3> References: <1467208504-13029-1-git-send-email-bruce.richardson@intel.com> <1467217635-19766-1-git-send-email-bruce.richardson@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1467217635-19766-1-git-send-email-bruce.richardson@intel.com> Organization: Intel Research and =?iso-8859-1?Q?De=ACvel?= =?iso-8859-1?Q?opment?= Ireland Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v2] mempool: rename functions with confusing names X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Jun 2016 16:30:26 -0000 + 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 > --- > > 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 >