On Mon, 22 Jun 2026 17:46:14 +0300 saeed bishara <[email protected]> wrote:
> On Sat, Jun 20, 2026 at 5:41 AM Stephen Hemminger > <[email protected]> wrote: > > > @@ -84,7 +84,7 @@ dpaa2_create_dpbp_device(int vdev_fd __rte_unused, > > } > > > > dpbp_node->dpbp_id = dpbp_id; > > - rte_atomic16_init(&dpbp_node->in_use); > > + dpbp_node->in_use = 0; > The previous code implies an ordering barrier, so it guarantees that > dpbp_node->dpbp_id is visible before in_use, while the new code > doesn't. isn't the a problem? That is incorrect assumption to make here. Atomic init is not a barrier at all, it is just an assignment: static inline void rte_atomic16_init(rte_atomic16_t *v) { v->cnt = 0; } > > > > TAILQ_INSERT_TAIL(&dpbp_dev_list, dpbp_node, next); > > > > @@ -103,7 +103,10 @@ struct dpaa2_dpbp_dev *dpaa2_alloc_dpbp_dev(void) > > > > /* Get DPBP dev handle from list using index */ > > TAILQ_FOREACH(dpbp_dev, &dpbp_dev_list, next) { > > - if (dpbp_dev && > > rte_atomic16_test_and_set(&dpbp_dev->in_use)) > > + uint16_t expected = 0; > > + if (rte_atomic_compare_exchange_strong_explicit( > > + &dpbp_dev->in_use, &expected, 1, > > + rte_memory_order_acquire, > > rte_memory_order_relaxed)) > > aren't rte_atomic_flag_test_and_set_explicit/rte_atomic_flag_clear_explicit > a better candidates instead of > rte_atomic_compare_exchange_strong_explicit/rte_atomic_store_explicit Atomic flags are not used in DPDK for a number of reasons. - limited operations only test and set, no load - lots of variation in between stdatomic and compilers - no improvement in code generation Instead DPDK has chosen to just use RTE_ATOMIC(bool) More wordy AI response: On Mon, 22 Jun 2026 17:46:14 +0300 saeed bishara <[email protected]> wrote: > > - rte_atomic16_init(&dpbp_node->in_use); > > + dpbp_node->in_use = 0; > The previous code implies an ordering barrier, so it guarantees that > dpbp_node->dpbp_id is visible before in_use, while the new code > doesn't. isn't the a problem? rte_atomic16_init() is a plain store: static inline void rte_atomic16_init(rte_atomic16_t *v) { v->cnt = 0; } Same for rte_atomic16_clear(). Only test_and_set() and dec() implied a barrier, via __sync_*. So no ordering is dropped. Neither version has a barrier between these stores and TAILQ_INSERT_TAIL(), and the list is not atomic either, so a reader concurrent with device creation would be unsafe regardless. Devices are created during bus probe. The point does apply in reverse though: with the field declared RTE_ATOMIC(uint16_t), a plain assignment is a seq_cst store when built with enable_stdatomic=true, and a plain store otherwise. v7 uses rte_atomic_store_explicit(..., rte_memory_order_relaxed) so both builds behave the same. > aren't rte_atomic_flag_test_and_set_explicit/rte_atomic_flag_clear_explicit > a better candidates instead of > rte_atomic_compare_exchange_strong_explicit/rte_atomic_store_explicit ? Semantically yes, but there is no portable type for the struct member. In rte_stdatomic.h those map to C11 atomic_flag with enable_stdatomic=true and to __atomic_test_and_set()/__atomic_clear() (bool or char) otherwise. atomic_flag also has no load operation and no initializer other than ATOMIC_FLAG_INIT. That is why rte_atomic_flag_* has no users in the tree. If an rte_atomic_flag type covering both backends is added, these sites are good candidates to convert.
