[dpdk-dev] [PATCH] ring: fix unaligned memory access on aarch32

[Ruifeng Wang]

On 2023/11/4 8:04 AM, Morten Brørup wrote:
> I have for a long time now wondered why the ring functions for enqueue/dequeue of 64-bit objects supports unaligned addresses, and now I finally found the patch introducing it.
> 
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Phil Yang
>> Sent: Monday, 9 March 2020 18.20
>>
>> The 32-bit arm machine doesn't support unaligned memory access. It
>> will cause a bus error on aarch32 with the custom element size ring.
>>
>> Thread 1 "test" received signal SIGBUS, Bus error.
>> __rte_ring_enqueue_elems_64 (n=1, obj_table=0xf5edfe41, prod_head=0, \
>> r=0xf5edfb80) at /build/dpdk/build/include/rte_ring_elem.h:177
>> 177                             ring[idx++] = obj[i++];
> 
> Which test is this? Why is it using an unaligned array of 64-bit objects? (Notice that obj_table=0xf5edfe41.)

The test case is:
https://elixir.bootlin.com/dpdk/latest/source/app/test/test_ring.c#L1121
This case deliberately use unaligned objects.

> 
> Nobody in their right mind would use an unaligned array of 64-bit objects. You can only create such an array if you force the compiler to prevent automatic alignment! And all the functions in your application using this array would also need to support unaligned addressing of these objects.
> 
> This seems extremely exotic, and not something any real application would do!
> 
> I would like to revert this patch for performance reasons.
> 
>>
>> Fixes: cc4b218790f6 ("ring: support configurable element size")
>>
>> Signed-off-by: Phil Yang <phil.yang at arm.com>
>> Reviewed-by: Ruifeng Wang <ruifeng.wang at arm.com>
>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli at arm.com>
>> ---
>>   lib/librte_ring/rte_ring_elem.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_ring/rte_ring_elem.h
>> b/lib/librte_ring/rte_ring_elem.h
>> index 3976757..663addc 100644
>> --- a/lib/librte_ring/rte_ring_elem.h
>> +++ b/lib/librte_ring/rte_ring_elem.h
>> @@ -160,7 +160,7 @@ __rte_ring_enqueue_elems_64(struct rte_ring *r,
>> uint32_t prod_head,
>>   	const uint32_t size = r->size;
>>   	uint32_t idx = prod_head & r->mask;
>>   	uint64_t *ring = (uint64_t *)&r[1];
>> -	const uint64_t *obj = (const uint64_t *)obj_table;
>> +	const unaligned_uint64_t *obj = (const unaligned_uint64_t
>> *)obj_table;
>>   	if (likely(idx + n < size)) {
>>   		for (i = 0; i < (n & ~0x3); i += 4, idx += 4) {
>>   			ring[idx] = obj[i];
>> @@ -294,7 +294,7 @@ __rte_ring_dequeue_elems_64(struct rte_ring *r,
>> uint32_t prod_head,
>>   	const uint32_t size = r->size;
>>   	uint32_t idx = prod_head & r->mask;
>>   	uint64_t *ring = (uint64_t *)&r[1];
>> -	uint64_t *obj = (uint64_t *)obj_table;
>> +	unaligned_uint64_t *obj = (unaligned_uint64_t *)obj_table;
>>   	if (likely(idx + n < size)) {
>>   		for (i = 0; i < (n & ~0x3); i += 4, idx += 4) {
>>   			obj[i] = ring[idx];
>> --
>> 2.7.4
>>
> 
> References:
> https://git.dpdk.org/dpdk/commit/lib/librte_ring/rte_ring_elem.h?id=3ba51478a3ab3132c33effc8b132641233275b36
> https://patchwork.dpdk.org/project/dpdk/patch/1583774395-10233-1-git-send-email-phil.yang@arm.com/
>