Thanks for doing this. I see that comments were expanded since v1, but
the code stayed the same. Using both atomic operations _and_ memory
barriers looks redundant and misleading, and the original got away with
just memory barriers. In the reviews to v1 there were different opinions
on which way to choose, just barriers (for the minimal change) or just
atomics, but I think we need to choose at most one.
On 20/09/2026 19:09, Stephen Hemminger wrote:
The use counter is a seqcount, not a reference count: odd means the
datapath is inside the callback, even means it is not. The two
barriers around it play different roles.
In bpf_eth_cbi_inuse() the counter goes odd and the following loads
of cb, bpf and jit must not be hoisted above that store. Ordering a
store against later loads needs a full barrier, so rte_smp_mb()
becomes a seq_cst thread fence.
This explanation probably belongs to the code, not commit message.
In bpf_eth_cbi_unuse() the read barrier becomes an acquire fence.
What must not happen there is the critical section loads sinking
past the store that makes the counter even, which is load/store
ordering, and that is what a standalone acquire fence provides.
acq_rel would additionally order prior stores, but the read side
only loads from the cbi and so has nothing to publish; it would
only upgrade dmb ishld to dmb ish on the datapath for no benefit.
The last part is not very readable for uninitiated. Perhaps it's better
not to mix C abstract machine language with ARM instructions when
justifying the solution.
