From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Wiles, Roger Keith" <keith.wiles-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
Subject: [RFC] More changes for rte_mempool.h:__mempool_get_bulk()
Date: Sun, 28 Sep 2014 17:52:16 +0000
Message-ID: <3B9A624B-ABBF-4A20-96CD-8D5607006FEA@windriver.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="Windows-1252"
Content-Transfer-Encoding: quoted-printable
To: "<dev-VfR2kkLFssw@public.gmane.org>" <dev-VfR2kkLFssw@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
Content-Language: en-US
Content-ID: <F04C2CAC6D56B644B3E89E4226625A96@local>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>

Here is a Request for Comment on __mempool_get_bulk() routine. I believe I =
am seeing a few more issues in this routine, please look at the code below =
and see if these seem to fix some concerns in how the ring is handled.

The first issue I believe is cache->len is increased by ret and not req as =
we do not know if ret =3D=3D req. This also means the cache->len may still =
not satisfy the request from the cache.

The second issue is if you believe the above code then we have to account f=
or that issue in the stats.

Let me know what you think?
++Keith
=97=97=97

diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_memp=
ool.h
index 199a493..b1b1f7a 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -945,9 +945,7 @@ __mempool_get_bulk(struct rte_mempool *mp, void **obj_t=
able,
                   unsigned n, int is_mc)
 {
        int ret;
-#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
-       unsigned n_orig =3D n;
-#endif
+
 #if RTE_MEMPOOL_CACHE_MAX_SIZE > 0
        struct rte_mempool_cache *cache;
        uint32_t index, len;
@@ -979,7 +977,21 @@ __mempool_get_bulk(struct rte_mempool *mp, void **obj_=
table,
                        goto ring_dequeue;
                }
=20
-               cache->len +=3D req;
+               cache->len +=3D ret;      // Need to adjust len by ret not =
req, as (ret !=3D req)
+
+               if ( cache->len < n ) {
+                       /*
+                        * Number (ret + cache->len) may not be >=3D n. As
+                        * the 'ret' value maybe zero or less then 'req'.
+                        *
+                        * Note:
+                        * An issue of order from the cache and common pool=
 could
+                        * be an issue if (cache->len !=3D 0 and less then =
n), but the
+                        * normal case it should be OK. If the user needs t=
o preserve
+                        * the order of packets then he must set cache_size=
 =3D=3D 0.
+                        */
+                       goto ring_dequeue;
+               }
        }
=20
        /* Now fill in the response ... */
@@ -1002,9 +1014,12 @@ ring_dequeue:
                ret =3D rte_ring_sc_dequeue_bulk(mp->ring, obj_table, n);
=20
        if (ret < 0)
-               __MEMPOOL_STAT_ADD(mp, get_fail, n_orig);
-       else
+               __MEMPOOL_STAT_ADD(mp, get_fail, n);
+       else {
                __MEMPOOL_STAT_ADD(mp, get_success, ret);
+               // Catch the case when ret !=3D n, adding zero should not b=
e a problem.
+               __MEMPOOL_STAT_ADD(mp, get_fail, n - ret);
+       }
=20
        return ret;
 }

Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-=
213-5533