On Thu, Jun 18, 2026 at 02:41:54AM -0400, Emil Tsalapatis wrote:
[...]
> Sashiko is right on this, let's use regular kernel style for new
> comments.
Done in v2.
[...]
> The extra new variable feels overly complicated. I think this is
> becuase the new code is going off of the got_new_data pattern, which
> imo is also unnecessary. We should just remove got_new data and just
> do:
>
> while (true) {
> prod_pos = __atomic_load_n(r->producer_pos, __ATOMIC_ACQUIRE);
> if (cons_pos == prod_pos)
> break;
>
> while (cons_pos != prod_pos) {
> ...
> }
> }
V2 removes `needs_wakeup` and leaves one fence after each batch that
advances the consumer position. I kept `got_new_data` because the
proposed unconditional outer loop would retry the same busy record
indefinitely. The revised loop retries a busy record once after
publishing progress and returns if the record is still busy.
[...]
> We just add an smp_mb() here. And since AFAICT smb_mb is available
> here, we can also avoid the move from smp_* to __atomic in the
> previous patch.
I kept the compiler atomics because `smp_mb()` is not a portable full
barrier in the tools headers. Only x86, arm64, and RISC-V define it
directly; the generic fallback can reduce `mb()` to a compiler barrier.
A single `__atomic_thread_fence()` after each batch orders the preceding
consumer-position stores without a full barrier after every record.
[...]
> Why __ATOMIC_RELAXED here? Maybe just declare the variable volatile
> and do regular reads?
I kept the stop flag as a relaxed atomic because `volatile` would leave
a C data race; the flag needs atomicity but no ordering.
[...]
> Label is unnecessary, just call break;
Done in v2.
