[ovs-dev] [PATCH] netdev-dpdk: Remove use of rte_mempool_ops_get_count.
Stokes, Ian
ian.stokes at intel.com
Wed May 23 11:53:28 UTC 2018
> > rte_mempool_ops_get_count is not exported by DPDK so it means it
> > cannot be used by OVS when using DPDK as a shared library.
> >
> > It was only being used for extra paranoid, things might change in the
> > future mode anyway, so remove it and just use rte_mempool_full.
>
> How about keeping the dpdk_mp_full() function in a following manner:
>
> static int
> dpdk_mp_full(const struct rte_mempool *mp) OVS_REQUIRES(dpdk_mp_mutex) {
> /* XXX: A comment about possible false positives and why we're not
> * worrying about them so much. */
> return rte_mempool_full(mp);
> }
>
> ?
>
> I feel that we still need some explanation inside the code.
> What do you think?
>
It probably makes sense so that others unfamiliar will have context in the future.
I've validated the other patches and was about to push them but I can hold off for this change instead. It's a minor change so I can push it as an incremental on the current set if it helps move the process along.
Ian
> Best regards, Ilya Maximets.
>
> >
> > The mempools are still removed later and are checked to see that the
> > buffers are back in it. Previously the mempools were removed straight
> > away and there was no checking.
> >
> > Fixes: 91fccdad72a2 ("netdev-dpdk: Free mempool only when no in-use
> > mbufs.")
> > Reported-by: Timothy Redaelli <tredaelli at redhat.com>
> > Reported-by: Markos Chandras <mchandras at suse.de>
> > Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
> > ---
> > lib/netdev-dpdk.c | 22 ++--------------------
> > 1 file changed, 2 insertions(+), 20 deletions(-)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> > 87152a7..ede1e5c 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -527,22 +527,4 @@ ovs_rte_pktmbuf_init(struct rte_mempool *mp
> > OVS_UNUSED, }
> >
> > -static int
> > -dpdk_mp_full(const struct rte_mempool *mp)
> > OVS_REQUIRES(dpdk_mp_mutex) -{
> > - unsigned ring_count;
> > - /* This logic is needed because rte_mempool_full() is not
> guaranteed to
> > - * be atomic and mbufs could be moved from mempool cache -->
> mempool ring
> > - * during the call. However, as no mbufs will be taken from the
> mempool
> > - * at this time, we can work around it by also checking the ring
> entries
> > - * separately and ensuring that they have not changed.
> > - */
> > - ring_count = rte_mempool_ops_get_count(mp);
> > - if (rte_mempool_full(mp) && rte_mempool_ops_get_count(mp) ==
> ring_count) {
> > - return 1;
> > - }
> > -
> > - return 0;
> > -}
> > -
> > /* Free unused mempools. */
> > static void
> > @@ -553,5 +535,5 @@ dpdk_mp_sweep(void)
> > ovs_mutex_lock(&dpdk_mp_mutex);
> > LIST_FOR_EACH_SAFE (dmp, next, list_node, &dpdk_mp_free_list) {
> > - if (dpdk_mp_full(dmp->mp)) {
> > + if (rte_mempool_full(dmp->mp)) {
> > VLOG_DBG("Freeing mempool \"%s\"", dmp->mp->name);
> > ovs_list_remove(&dmp->list_node); @@ -666,5 +648,5 @@
> > dpdk_mp_release(struct rte_mempool *mp)
> >
> > ovs_mutex_lock(&dpdk_mp_mutex);
> > - if (dpdk_mp_full(mp)) {
> > + if (rte_mempool_full(mp)) {
> > VLOG_DBG("Freeing mempool \"%s\"", mp->name);
> > rte_mempool_free(mp);
> > --
> > 1.8.3.1
More information about the dev
mailing list