-----Original Message----- > Date: Mon, 17 Sep 2018 16:17:22 +0800 > From: Gavin Hu <[email protected]> > To: [email protected] > CC: [email protected], [email protected], [email protected], > [email protected], [email protected], [email protected], > [email protected] > Subject: [PATCH v3 1/3] ring: read tail using atomic load > X-Mailer: git-send-email 2.7.4 > > External Email > > In update_tail, read ht->tail using __atomic_load.Although the > compiler currently seems to be doing the right thing even without > _atomic_load, we don't want to give the compiler freedom to optimise > what should be an atomic load, it should not be arbitarily moved > around. > > Fixes: 39368ebfc6 ("ring: introduce C11 memory model barrier option") > Cc: [email protected] > > Signed-off-by: Gavin Hu <[email protected]> > Reviewed-by: Honnappa Nagarahalli <[email protected]> > Reviewed-by: Steve Capper <[email protected]> > Reviewed-by: Ola Liljedahl <[email protected]> > --- > lib/librte_ring/rte_ring_c11_mem.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_ring/rte_ring_c11_mem.h > b/lib/librte_ring/rte_ring_c11_mem.h > index 94df3c4..234fea0 100644 > --- a/lib/librte_ring/rte_ring_c11_mem.h > +++ b/lib/librte_ring/rte_ring_c11_mem.h > @@ -21,7 +21,8 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, > uint32_t new_val, > * we need to wait for them to complete > */ > if (!single) > - while (unlikely(ht->tail != old_val)) > + while (unlikely(old_val != __atomic_load_n(&ht->tail, > + __ATOMIC_RELAXED))) > rte_pause();
Since it is a while loop with rte_pause(), IMO, There is no scope of false compiler optimization. IMO, this change may not required though I don't see any performance difference with two core ring_perf_autotest test. May be more core case it may have effect. IMO, If it not absolutely required, we can avoid this change. > > __atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE); > -- > 2.7.4 >
