Re: [PATCH 1/6] ring: change head and tail to pointer-width size

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 11-Jan-19 7:27 PM, Eads, Gage wrote:
> 
> 
>> -----Original Message-----
>> From: Richardson, Bruce
>> Sent: Friday, January 11, 2019 5:59 AM
>> To: Burakov, Anatoly <anatoly.burakov@intel.com>
>> Cc: Eads, Gage <gage.eads@intel.com>; dev@dpdk.org;
>> olivier.matz@6wind.com; arybchenko@solarflare.com; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH 1/6] ring: change head and tail to pointer-width
>> size
>>
>> On Fri, Jan 11, 2019 at 11:30:24AM +0000, Burakov, Anatoly wrote:
>>> On 11-Jan-19 10:58 AM, Bruce Richardson wrote:
>>>> On Fri, Jan 11, 2019 at 10:40:19AM +0000, Burakov, Anatoly wrote:
>>>>> <...>
>>>>>
>>>>>> + * Copyright(c) 2016-2019 Intel Corporation
>>>>>>      */
>>>>>>     /**
>>>>>> @@ -88,7 +88,7 @@ rte_event_ring_enqueue_burst(struct
>> rte_event_ring *r,
>>>>>>     		const struct rte_event *events,
>>>>>>     		unsigned int n, uint16_t *free_space)
>>>>>>     {
>>>>>> -	uint32_t prod_head, prod_next;
>>>>>> +	uintptr_t prod_head, prod_next;
>>>>>
>>>>> I would also question the use of uinptr_t. I think semnatically,
>>>>> size_t is more appropriate.
>>>>>
>>>> Yes, it would, but I believe in this case they want to use the
>>>> largest size of (unsigned)int where there exists an atomic for
>>>> manipulating 2 of them simultaneously. [The largest size is to
>>>> minimize any chance of an ABA issue occuring]. Therefore we need
>>>> 32-bit values on 32-bit and 64-bit on 64, and I suspect the best way
>>>> to guarantee this is to use pointer-sized values. If size_t is
>>>> guaranteed across all OS's to have the same size as uintptr_t it could also be
>> used, though.
>>>>
>>>> /Bruce
>>>>
>>>
>>> Technically, size_t and uintptr_t are not guaranteed to match. In
>>> practice, they won't match only on architectures that DPDK doesn't
>>> intend to run on (such as 16-bit segmented archs, where size_t would
>>> be 16-bit but uinptr_t would be 32-bit).
>>>
>>> In all the rest of DPDK code, we use size_t for this kind of thing.
>>>
>>
>> Ok.
>> If we do use size_t, I think we also need to add a compile-time check into the
>> build too, to error out if sizeof(size_t) != sizeof(uintptr_t).
> 
> Ok, I wasn't aware of the precedent of using size_t for this purpose. I'll change it and look into adding a static assert.

RTE_BUILD_BUG_ON?

> 
> Thanks,
> Gage
> 


-- 
Thanks,
Anatoly