Re: [lttng-dev] [PATCH 04/11] urcu/arch/generic: Use atomic builtins if configured

On Wed, Jun 21, 2023 at 09:48:10PM -0400, Mathieu Desnoyers wrote:
> On 6/21/23 20:53, Olivier Dion wrote:
> > On Wed, 21 Jun 2023, "Paul E. McKenney" <[email protected]> wrote:
> > > On Mon, May 15, 2023 at 04:17:11PM -0400, Olivier Dion wrote:
> > > >   #ifndef cmm_mb
> > > >   #define cmm_mb()    __sync_synchronize()
> > > 
> > > Just out of curiosity, why not also implement cmm_mb() in terms of
> > > __atomic_thread_fence(__ATOMIC_SEQ_CST)?  (Or is that a later patch?)
> > 
> > IIRC, Mathieu and I agree that the definition of a thread fence -- acts
> > as a synchronization fence between threads -- is too weak for what we
> > want here.  For example, with I/O devices.
> > 
> > Although __sync_synchronize() is probably an alias for a SEQ_CST thread
> > fence, its definition -- issues a full memory barrier -- is stronger.
> > 
> > We do not want to rely on this assumption (alias) and prefer to rely on
> > the documented definition instead.
> 
> We should document this rationale with a new comment near the #define,
> in case anyone mistakenly decides to use a thread fence there to make it
> similar to the rest of the code in the future.

That would be good, thank you!

Ah, and I did not find any issues with the rest of the patchset.

							Thanx, Paul
_______________________________________________
lttng-dev mailing list
[email protected]
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev