The variable is accessed with atomic_load, so doesn't need to be declared volatile.

I think you can use a bool here instead.

Suppose we load stopped, and then are preempted. Then, the transmit path tries to enqueue a packet and fails with ENOBUFS, so it sets stopped = 1. The only way we can make progress is by running the cleanup task again, right?

I think this mechanism works, but I'm wondering if we really need a full barrier here. It looks like acquire/release loads/stores of stopped would be sufficient.

Removed the volatile, but a switch to bool angers atomic:

gve_tx.c:249:2: error: controlling expression type 'bool' (aka '_Bool') not compatible with any generic association type
        atomic_store_8(&tx->stopped, false);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/src/sys/sys/atomic_common.h:114:2: note: expanded from macro 'atomic_store_8'
        __atomic_store_generic(p, v, int8_t, uint8_t, 8)
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/src/sys/sys/atomic_common.h:86:11: note: expanded from macro '__atomic_store_generic'
        _Generic(*(p),                          \

Suppose we load stopped, and then are preempted. Then, the transmit path tries to enqueue a packet and fails with ENOBUFS, so it sets stopped = 1. The only way we can make progress is by running the cleanup task again, right?

Yup. The correctness relies on the ordering between the space made by this thread in the ring and its load of stopped.

If this thread loads a 0 and is preempted and then the xmit thread ENOBUFSs, that implies that the space this thread made before getting preempted wasn't enough to xmit a pkt, which means there is a future interrupt coming that will eventually make space.

Note that the xmit thread does not ever schedule this cleanup thread, only the hardware summons it to work by irq.

I think this mechanism works, but I'm wondering if we really need a full barrier here. It looks like acquire/release loads/stores of stopped would be sufficient.

Since I need to load stopped after making the space in the ring, am I not forced to use a barrier: there being no "release" variant of load?

You'd have to use atomic_store_bool.

We do also have atomic_thread_fence_*, to be used instead of mb() etc. when needed. In particular, atomic_thread_fence_seq_cst() should be equivalent to mb() on all platforms.

Sorry, was testing briefly on a version without https://reviews.freebsd.org/D36078 and missed its existence