Hi Jules,
On Thu, Jan 26, 2023 at 10:57:20AM +0100, Jules Maselbas wrote:
> Hi Mark,
>
> On Fri, Jan 20, 2023 at 03:18:48PM +0000, Mark Rutland wrote:
> > On Fri, Jan 20, 2023 at 03:09:42PM +0100, Yann Sionneau wrote:
> > > +#define ATOMIC64_RETURN_OP(op, c_op) \
> > > +static inline long arch_atomic64_##op##_return(long i, atomic64_t *v) \
> > > +{ \
> > > + long new, old, ret; \
> > > + \
> > > + do { \
> > > + old = v->counter; \
> >
> > This should be arch_atomic64_read(v), in order to avoid the potential for the
> > compiler to replay the access and introduce ABA races and other such problems.
> Thanks for the suggestion, this will be into v3.
>
> > For details, see:
> >
> > https://lore.kernel.org/lkml/Y70SWXHDmOc3RhMd@osiris/
> > https://lore.kernel.org/lkml/Y71LoCIl+IFdy9D8@FVFF77S0Q05N/
> >
> > I see that the generic 32-bit atomic code suffers from that issue, and we
> > should fix it.
> I took a look at the generic 32-bit atomic, but I am unsure if this
> needs to be done for both the SMP and non-SMP implementations. But I
> can send a first patch and we can discuss from there.
Sounds good to me; thanks!
[...]
> > > +static inline int arch_atomic_add_return(int i, atomic_t *v)
> > > +{
> > > + int new, old, ret;
> > > +
> > > + do {
> > > + old = v->counter;
> >
> > Likewise, arch_atomic64_read(v) here.
> ack, this will bt arch_atomic_read(v) here since this is not atomic64_t
> here.
Ah, yes, my bad!
Thanks,
Mark.
