On Thu, 18 Feb 2021 at 14:53, Jason A. Donenfeld <ja...@zx2c4.com> wrote: > > Hey Bjorn, > > On Thu, Feb 18, 2021 at 2:50 PM Björn Töpel <bj...@kernel.org> wrote: > > > + > > > +static void __wg_prev_queue_enqueue(struct prev_queue *queue, struct > > > sk_buff *skb) > > > +{ > > > + WRITE_ONCE(NEXT(skb), NULL); > > > + smp_wmb(); > > > + WRITE_ONCE(NEXT(xchg_relaxed(&queue->head, skb)), skb); > > > +} > > > + > > > > I'll chime in with Toke; This MPSC and Dmitry's links really took me > > to the "verify with pen/paper"-level! Thanks! > > > > I'd replace the smp_wmb()/_relaxed above with a xchg_release(), which > > might perform better on some platforms. Also, it'll be a nicer pair > > with the ldacq below. :-P In general, it would be nice with some > > wording how the fences pair. It would help the readers (like me!) a > > lot. > > Exactly. This is what's been in my dev tree for the last week or so: >
Ah, nice! > +static void __wg_prev_queue_enqueue(struct prev_queue *queue, struct > sk_buff *skb) > +{ > + WRITE_ONCE(NEXT(skb), NULL); > + WRITE_ONCE(NEXT(xchg_release(&queue->head, skb)), skb); > +} > > Look good? > Yes, exactly like that! Cheers, Björn