-----Original Message-----
> Date: Mon, 6 Nov 2017 15:25:12 +0800
> From: Jia He <hejia...@gmail.com>
> To: Jerin Jacob <jerin.ja...@caviumnetworks.com>
> Cc: dev@dpdk.org, olivier.m...@6wind.com, konstantin.anan...@intel.com,
> bruce.richard...@intel.com, jianbo....@arm.com, hemant.agra...@nxp.com,
> jie2....@hxt-semitech.com, bing.z...@hxt-semitech.com,
> jia...@hxt-semitech.com
> Subject: Re: [PATCH v2] ring: guarantee ordering of cons/prod loading when
> doing
> User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101
> Thunderbird/52.4.0
>
> Hi Jerin
>
>
> On 11/3/2017 8:56 PM, Jerin Jacob Wrote:
> > -----Original Message-----
> > >
> [...]
> > > g like that.
> > > Ok, but how to distinguish following 2 options?
> > No clearly understood this question. For arm64 case, you can add
> > CONFIG_RTE_RING_USE_C11_MEM_MODEL=y in config/defconfig_arm64-armv8a-*
> Sorry for my unclear expressions.
> I mean there should be one additional config macro besides
> CONFIG_RTE_RING_USE_C11_MEM_MODEL
> for users to choose?
>
> i.e.
> - On X86:CONFIG_RTE_RING_USE_C11_MEM_MODEL=n
> include rte_ring_generic.h, no changes
> - On arm64，CONFIG_RTE_RING_USE_C11_MEM_MODEL=y
> include rte_ring_c11_mem.h by default.
> In rte_ring_c11_mem.h, implement new version of
> __rte_ring_move_prod_head/__rte_ring_move_cons_head/update_tail
>
> Then, how to distinguish the option of using rte_smp_rmb() or
> __atomic_load/store_n()?
On option could be to change the prototype of update_tail() and make
compiler accommodate it for zero cost for arm64(Which I think, it it the
case. But you can check the generated instructions)
If not, move, __rte_ring_do_dequeue() and __rte_ring_do_enqueue() instead of
__rte_ring_move_prod_head/__rte_ring_move_cons_head/update_tail()
➜ [master][dpdk.org] $ git diff
diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index 5e9b3b7b4..b32648825 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -358,8 +358,12 @@ void rte_ring_dump(FILE *f, const struct rte_ring
*r);
static __rte_always_inline void
update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t
new_val,
- uint32_t single)
+ uint32_t single, const uint32_t enqueue)
{
+ if (enqueue)
+ rte_smp_wmb();
+ else
+ rte_smp_rmb();
/*
* If there are other enqueues/dequeues in progress that
* preceded us,
* we need to wait for them to complete
@@ -470,9 +474,8 @@ __rte_ring_do_enqueue(struct rte_ring *r, void *
const *obj_table,
goto end;
ENQUEUE_PTRS(r, &r[1], prod_head, obj_table, n, void *);
- rte_smp_wmb();
- update_tail(&r->prod, prod_head, prod_next, is_sp);
+ update_tail(&r->prod, prod_head, prod_next, is_sp, 1);
end:
if (free_space != NULL)
*free_space = free_entries - n;
@@ -575,9 +578,8 @@ __rte_ring_do_dequeue(struct rte_ring *r, void
**obj_table,
goto end;
DEQUEUE_PTRS(r, &r[1], cons_head, obj_table, n, void *);
- rte_smp_rmb();
- update_tail(&r->cons, cons_head, cons_next, is_sc);
+ update_tail(&r->cons, cons_head, cons_next, is_sc, 0);
end:
if (available != NULL)
>
> Thanks for the clarification.
>
> --
> Cheers,
> Jia
>
