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-boun...@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.y...@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.w...@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagaraha...@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.y...@arm.com/

Reply via email to