Re: [PATCH v5 11/24] drivers: replace rte_atomic16 with stdatomic

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 22 Jun 2026 17:46:14 +0300
saeed bishara <saeed.bishara.os@gmail.com> wrote:

> On Sat, Jun 20, 2026 at 5:41 AM Stephen Hemminger
> <stephen@networkplumber.org> 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 <saeed.bishara.os@gmail.com> 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.