On Fri, 9 Oct 2026 01:05:56 -0500
Wathsala Vithanage <[email protected]> wrote:
> > /*
> > * Marks given callback as used by datapath.
> > */
> > static __rte_always_inline void
> > bpf_eth_cbi_inuse(struct bpf_eth_cbi *cbi)
> > {
> > - cbi->use++;
> > - /* make sure no store/load reordering could happen */
> > - rte_smp_mb();
> > + rte_atomic_store_explicit(&cbi->use,
> > + rte_atomic_load_explicit(&cbi->use, rte_memory_order_relaxed) +
> > 1,
> > + rte_memory_order_relaxed);
> > +
> > + /* full barrier: count must be visible before cb is read */
> > + rte_atomic_thread_fence(rte_memory_order_seq_cst);
>
> This is correct but why use a barrier when the above store itself could
> be SEQ_CST that
> synchronizes with load in bpf_eth_cbi_wait?
There are two different things be covered for safety here.
The counter and the callback pointer. The counter uses atomic
operations and the callback pointer is referenced without atomic.
AI explains it as:
A seq_cst store on the counter is not enough. This is a store/load
pattern across two objects:
datapath: store use, load cb
unload: store cb, load use
At least one side has to see the other's store. Making the store
of use seq_cst does not order the datapath's later load of cb,
which is a plain load. On arm64 that compiles to stlr followed by
ldr, and the ldr can be satisfied before the stlr is visible.
Only stlr/ldar pairs are kept in order.
It can be done without fences by making cb atomic and using
seq_cst for all four accesses (stlr + ldar on arm64). That touches
every callback and is a bigger change than replacing the barriers,
so I would rather do it as a follow-up if at all.
There is no cost difference on x86. A seq_cst store is xchg and
the fence is lock addl, one locked instruction per burst either
way.
