[dpdk-dev] [PATCH v2] ring: check for zero objects mc dequeue / mp enqueue

Xie, Huawei huawei.xie at intel.com
Mon Mar 21 18:47:44 CET 2016


On 3/18/2016 10:17 PM, Bruce Richardson wrote:
> On Fri, Mar 18, 2016 at 01:47:29PM +0100, Mauricio Vásquez wrote:
>> Hi,
>>
>>
>> On Fri, Mar 18, 2016 at 11:35 AM, Thomas Monjalon <thomas.monjalon at 6wind.com
>>> wrote:
>>> 2016-03-18 11:27, Olivier Matz:
>>>> On 03/18/2016 11:18 AM, Bruce Richardson wrote:
>>>>>>> +       /* Avoid the unnecessary cmpset operation below, which is
>>> also
>>>>>>> +        * potentially harmful when n equals 0. */
>>>>>>> +       if (n == 0)
>>>>>>>
>>>>>> What about using unlikely here?
>>>>>>
>>>>> Unless there is a measurable performance increase by adding in
>>> likely/unlikely
>>>>> I'd suggest avoiding it's use. In general, likely/unlikely should only
>>> be used
>>>>> for things like catestrophic errors because the penalty for taking the
>>> unlikely
>>>>> leg of the code can be quite severe. For normal stuff, where the code
>>> nearly
>>>>> always goes one way in the branch but occasionally goes the other, the
>>> hardware
>>>>> branch predictors generally do a good enough job.
>>>> Do you mean using likely/unlikely could be worst than not using it
>>>> in this case?
>>>>
>>>> To me, using unlikely here is not a bad idea: it shows to the compiler
>>>> and to the reader of the code that is case is not the usual case.
>>> It would be nice to have a guideline section about likely/unlikely in
>>> doc/guides/contributing/design.rst
>>>
>>> Bruce gave a talk at Dublin about this kind of things.
>>> I'm sure he could contribute more design guidelines ;)
>>>
>> There is a small explanation in the section "Branch Prediction" of
>> doc/guides/contributing/coding_style.rst, but I do not know if that is
>> enough to understand when to use them.
>>
>> I've made a fast check and there are many PMDs that use them to check if
>> number of packets is zero in the transmission function.
> Yeah, and I wonder how many of those are actually necessary too :-)
>
> It's not a big deal either way, I just think the patch is fine as-is without
> the extra macros.

IMO we use likely/unlikely in two cases, catastrophic errors and the
code nearly always goes one way, i.e, preferred/favored fast path.
Likely/unlikely helps not only for branch predication but also for cache
usage. The code generated for the likely path will directly follow the
branch instruction. To me, it is reasonable enough to add unlikely for n
== 0, which we don't expect to happen.
I remember with/without likely, compiler could generate three kind of
instructions. Didn't deep dive into it.

>
> /Bruce
>



More information about the dev mailing list