On 12/21/15, 9:21 AM, "Xie, Huawei" <huawei.xie at intel.com> wrote:
>On 12/19/2015 3:27 AM, Wiles, Keith wrote: >> On 12/18/15, 11:32 AM, "dev on behalf of Stephen Hemminger" <dev-bounces at >> dpdk.org on behalf of stephen at networkplumber.org> wrote: >> >>> On Fri, 18 Dec 2015 10:44:02 +0000 >>> "Ananyev, Konstantin" <konstantin.ananyev at intel.com> wrote: >>> >>>> >>>>> -----Original Message----- >>>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Stephen Hemminger >>>>> Sent: Friday, December 18, 2015 5:01 AM >>>>> To: Xie, Huawei >>>>> Cc: dev at dpdk.org >>>>> Subject: Re: [dpdk-dev] [PATCH v2 1/2] mbuf: provide >>>>> rte_pktmbuf_alloc_bulk API >>>>> >>>>> On Mon, 14 Dec 2015 09:14:41 +0800 >>>>> Huawei Xie <huawei.xie at intel.com> wrote: >>>>> >>>>>> v2 changes: >>>>>> unroll the loop a bit to help the performance >>>>>> >>>>>> rte_pktmbuf_alloc_bulk allocates a bulk of packet mbufs. >>>>>> >>>>>> There is related thread about this bulk API. >>>>>> http://dpdk.org/dev/patchwork/patch/4718/ >>>>>> Thanks to Konstantin's loop unrolling. >>>>>> >>>>>> Signed-off-by: Gerald Rogers <gerald.rogers at intel.com> >>>>>> Signed-off-by: Huawei Xie <huawei.xie at intel.com> >>>>>> Acked-by: Konstantin Ananyev <konstantin.ananyev at intel.com> >>>>>> --- >>>>>> lib/librte_mbuf/rte_mbuf.h | 50 >>>>>> ++++++++++++++++++++++++++++++++++++++++++++++ >>>>>> 1 file changed, 50 insertions(+) >>>>>> >>>>>> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h >>>>>> index f234ac9..4e209e0 100644 >>>>>> --- a/lib/librte_mbuf/rte_mbuf.h >>>>>> +++ b/lib/librte_mbuf/rte_mbuf.h >>>>>> @@ -1336,6 +1336,56 @@ static inline struct rte_mbuf >>>>>> *rte_pktmbuf_alloc(struct rte_mempool *mp) >>>>>> } >>>>>> >>>>>> /** >>>>>> + * Allocate a bulk of mbufs, initialize refcnt and reset the fields to >>>>>> default >>>>>> + * values. >>>>>> + * >>>>>> + * @param pool >>>>>> + * The mempool from which mbufs are allocated. >>>>>> + * @param mbufs >>>>>> + * Array of pointers to mbufs >>>>>> + * @param count >>>>>> + * Array size >>>>>> + * @return >>>>>> + * - 0: Success >>>>>> + */ >>>>>> +static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool, >>>>>> + struct rte_mbuf **mbufs, unsigned count) >>>>>> +{ >>>>>> + unsigned idx = 0; >>>>>> + int rc; >>>>>> + >>>>>> + rc = rte_mempool_get_bulk(pool, (void **)mbufs, count); >>>>>> + if (unlikely(rc)) >>>>>> + return rc; >>>>>> + >>>>>> + switch (count % 4) { >>>>>> + while (idx != count) { >>>>>> + case 0: >>>>>> + >>>>>> RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0); >>>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1); >>>>>> + rte_pktmbuf_reset(mbufs[idx]); >>>>>> + idx++; >>>>>> + case 3: >>>>>> + >>>>>> RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0); >>>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1); >>>>>> + rte_pktmbuf_reset(mbufs[idx]); >>>>>> + idx++; >>>>>> + case 2: >>>>>> + >>>>>> RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0); >>>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1); >>>>>> + rte_pktmbuf_reset(mbufs[idx]); >>>>>> + idx++; >>>>>> + case 1: >>>>>> + >>>>>> RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0); >>>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1); >>>>>> + rte_pktmbuf_reset(mbufs[idx]); >>>>>> + idx++; >>>>>> + } >>>>>> + } >>>>>> + return 0; >>>>>> +} >>>>> This is weird. Why not just use Duff's device in a more normal manner. >>>> But it is a sort of Duff's method. >>>> Not sure what looks weird to you here? >>>> while () {} instead of do {} while();? >>>> Konstantin >>>> >>>> >>>> >>> It is unusual to have cases not associated with block of the switch. >>> Unusual to me means, "not used commonly in most code". >>> >>> Since you are jumping into the loop, might make more sense as a do { } >>> while() >> I find this a very odd coding practice and I would suggest we not do this, >> unless it gives us some great performance gain. >> >> Keith >The loop unwinding could give performance gain. The only problem is the >switch/loop combination makes people feel weird at the first glance but >soon they will grasp this style. Since this is inherited from old famous >duff's device, i prefer to keep this style which saves lines of code. Please add a comment to the code to reflex where this style came from and why you are using it, would be very handy here. >>> >> >> Regards, >> Keith >> >> >> >> > > Regards, Keith
