Re: [dpdk-dev] [PATCH v3 1/2] mbuf: provide rte_pktmbuf_alloc_bulk API

["Xie, Huawei" <huawei.xie@intel.com>]

On 12/24/2015 2:49 AM, Ananyev, Konstantin wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
>> Sent: Wednesday, December 23, 2015 6:38 PM
>> To: Xie, Huawei
>> Cc: dev@dpdk.org; dprovan@bivio.net
>> Subject: Re: [dpdk-dev] [PATCH v3 1/2] mbuf: provide rte_pktmbuf_alloc_bulk API
>>
>> On Wed, 23 Dec 2015 00:17:53 +0800
>> Huawei Xie <huawei.xie@intel.com> wrote:
>>
>>> +
>>> +	rc = rte_mempool_get_bulk(pool, (void **)mbufs, count);
>>> +	if (unlikely(rc))
>>> +		return rc;
>>> +
>>> +	switch (count % 4) {
>>> +	case 0: while (idx != count) {
>>> +			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;
>>> +}
>> Since function will not work if count can not be 0 (otherwise rte_mempool_get_bulk will fail),
> As I understand, rte_mempool_get_bulk() will work correctly and return 0, if count==0.
> That's why Huawei prefers while() {}, instead of do {} while() - to avoid extra check for
> (count != 0) at the start. 
> Konstantin

Yes.

>
>
>> why not:
>> 	1. Document that assumption
>> 	2. Use that assumption to speed up code.
>>
>>
>>
>> 	switch(count % 4) {
>> 		do {
>> 			case 0:
>> 			...
>> 			case 1:
>> 			...
>> 		} while (idx != count);
>> 	}
>>
>> Also you really need to add a big block comment about this loop, to explain
>> what it does and why.

Since we change duff's implementation a bit, and for people who don't
know duff's device, we could add comment.
Is comment like this enough?
Use Duff's device to unroll the loop a bit to gain more performance
Use while() rather than do {} while() as count could be zero.