Re: [lttng-dev] [PATCH 02/11] urcu/uatomic: Use atomic builtins if configured

On Thu, Jun 22, 2023 at 11:55:55AM -0400, Mathieu Desnoyers wrote:
> On 6/21/23 19:19, Paul E. McKenney wrote:
> [...]
> > > diff --git a/include/urcu/uatomic/builtins-generic.h b/include/urcu/uatomic/builtins-generic.h
> > > new file mode 100644
> > > index 0000000..8e6a9b5
> > > --- /dev/null
> > > +++ b/include/urcu/uatomic/builtins-generic.h
> > > @@ -0,0 +1,85 @@
> > > +/*
> > > + * urcu/uatomic/builtins-generic.h
> > > + *
> > > + * Copyright (c) 2023 Olivier Dion <[email protected]>
> > > + *
> > > + * This library is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU Lesser General Public
> > > + * License as published by the Free Software Foundation; either
> > > + * version 2.1 of the License, or (at your option) any later version.
> > > + *
> > > + * This library is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > > + * Lesser General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU Lesser General Public
> > > + * License along with this library; if not, write to the Free Software
> > > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > > + */
> > > +
> > > +#ifndef _URCU_UATOMIC_BUILTINS_GENERIC_H
> > > +#define _URCU_UATOMIC_BUILTINS_GENERIC_H
> > > +
> > > +#include <urcu/system.h>
> > > +
> > > +#define uatomic_set(addr, v) __atomic_store_n(addr, v, __ATOMIC_RELAXED)
> > > +
> > > +#define uatomic_read(addr) __atomic_load_n(addr, __ATOMIC_RELAXED)
> > 
> > Does this lose the volatile semantics that the old-style definitions
> > had?
> > 
> 
> Yes.
> 
> [...]
> 
> > > +++ b/include/urcu/uatomic/builtins-x86.h
> > > @@ -0,0 +1,85 @@
> > > +/*
> > > + * urcu/uatomic/builtins-x86.h
> > > + *
> > > + * Copyright (c) 2023 Olivier Dion <[email protected]>
> > > + *
> > > + * This library is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU Lesser General Public
> > > + * License as published by the Free Software Foundation; either
> > > + * version 2.1 of the License, or (at your option) any later version.
> > > + *
> > > + * This library is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > > + * Lesser General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU Lesser General Public
> > > + * License along with this library; if not, write to the Free Software
> > > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > > + */
> > > +
> > > +#ifndef _URCU_UATOMIC_BUILTINS_X86_H
> > > +#define _URCU_UATOMIC_BUILTINS_X86_H
> > > +
> > > +#include <urcu/system.h>
> > > +
> > > +#define uatomic_set(addr, v) __atomic_store_n(addr, v, __ATOMIC_RELAXED)
> > > +
> > > +#define uatomic_read(addr) __atomic_load_n(addr, __ATOMIC_RELAXED)
> > 
> > And same question here.
> 
> Yes, this opens interesting questions:
> 
> * what semantic do we want for uatomic_read/set ?
> 
> * what semantic do we want for CMM_LOAD_SHARED/CMM_STORE_SHARED ?
> 
> * do we want to allow load/store-shared to work on variables larger than a
> word ? (e.g. on a uint64_t on a 32-bit architecture, or on a structure)
> 
> * what are the guarantees of a volatile type ?
> 
> * what are the guarantees of a load/store relaxed in C11 ?
> 
> Does the delta between volatile and C11 relaxed guarantees matter ?
> 
> Is there an advantage to use C11 load/store relaxed over volatile ? Should
> we combine both C11 load/store _and_ volatile ? Should we use
> atomic_signal_fence instead ?

I suggest C11 volatile atomic load/store.  Load/store fusing is permitted
for non-volatile atomic loads and stores, and such fusing can ruin your
code's entire day.  ;-)

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