2019-06-13 15:12:27

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH v2 0/4] atomic: Fixes to smp_mb__{before,after}_atomic() and mips.

Hi,

This all started when Andrea Parri found a 'surprising' behaviour for x86:

http://lkml.kernel.org/r/20190418125412.GA10817@andrea

Basically we fail for:

*x = 1;
atomic_inc(u);
smp_mb__after_atomic();
r0 = *y;

Because, while the atomic_inc() implies memory order, it
(surprisingly) does not provide a compiler barrier. This then allows
the compiler to re-order like so:

atomic_inc(u);
*x = 1;
smp_mb__after_atomic();
r0 = *y;

Which the CPU is then allowed to re-order (under TSO rules) like:

atomic_inc(u);
r0 = *y;
*x = 1;

And this very much was not intended.

This had me audit all the (strong) architectures that had weak
smp_mb__{before,after}_atomic: ia64, mips, sparc, s390, x86, xtensa.

Of those, only x86 and mips were affected. Looking at MIPS to solve this, led
to the other MIPS patches.

All these patches have been through 0day for quite a while.

Paul, how do you want to route the MIPS bits?


2019-06-13 16:32:58

by Paul Burton

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] atomic: Fixes to smp_mb__{before,after}_atomic() and mips.

Hi Peter,

On Thu, Jun 13, 2019 at 03:43:17PM +0200, Peter Zijlstra wrote:
> Paul, how do you want to route the MIPS bits?

Thanks for following up on these issues. I'd be happy to take the MIPS
patches through the mips-fixes branch - do you have a preference?

Paul

2019-06-13 17:50:54

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] atomic: Fixes to smp_mb__{before,after}_atomic() and mips.

On Thu, Jun 13, 2019 at 04:32:24PM +0000, Paul Burton wrote:
> Hi Peter,
>
> On Thu, Jun 13, 2019 at 03:43:17PM +0200, Peter Zijlstra wrote:
> > Paul, how do you want to route the MIPS bits?
>
> Thanks for following up on these issues. I'd be happy to take the MIPS
> patches through the mips-fixes branch - do you have a preference?

That works for me; I'll make sure the x86 one goes through tip.

Thanks!

2019-08-31 09:02:26

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] atomic: Fixes to smp_mb__{before,after}_atomic() and mips.

On Thu, Jun 13, 2019 at 03:43:17PM +0200, Peter Zijlstra wrote:
> Hi,
>
> This all started when Andrea Parri found a 'surprising' behaviour for x86:
>
> http://lkml.kernel.org/r/20190418125412.GA10817@andrea
>
> Basically we fail for:
>
> *x = 1;
> atomic_inc(u);
> smp_mb__after_atomic();
> r0 = *y;
>
> Because, while the atomic_inc() implies memory order, it
> (surprisingly) does not provide a compiler barrier. This then allows
> the compiler to re-order like so:
>
> atomic_inc(u);
> *x = 1;
> smp_mb__after_atomic();
> r0 = *y;
>
> Which the CPU is then allowed to re-order (under TSO rules) like:
>
> atomic_inc(u);
> r0 = *y;
> *x = 1;
>
> And this very much was not intended.
>
> This had me audit all the (strong) architectures that had weak
> smp_mb__{before,after}_atomic: ia64, mips, sparc, s390, x86, xtensa.
>
> Of those, only x86 and mips were affected. Looking at MIPS to solve this, led
> to the other MIPS patches.
>
> All these patches have been through 0day for quite a while.
>
> Paul, how do you want to route the MIPS bits?

Paul; afaict the MIPS patches still apply (ie. they've not made their
way into Linus' tree yet).

I thought you were going to take them?

2019-08-31 10:04:43

by Paul Burton

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] atomic: Fixes to smp_mb__{before,after}_atomic() and mips.

Hi Peter,

On Sat, Aug 31, 2019 at 11:00:55AM +0200, Peter Zijlstra wrote:
> On Thu, Jun 13, 2019 at 03:43:17PM +0200, Peter Zijlstra wrote:
> > Hi,
> >
> > This all started when Andrea Parri found a 'surprising' behaviour for x86:
> >
> > http://lkml.kernel.org/r/20190418125412.GA10817@andrea
> >
> > Basically we fail for:
> >
> > *x = 1;
> > atomic_inc(u);
> > smp_mb__after_atomic();
> > r0 = *y;
> >
> > Because, while the atomic_inc() implies memory order, it
> > (surprisingly) does not provide a compiler barrier. This then allows
> > the compiler to re-order like so:
> >
> > atomic_inc(u);
> > *x = 1;
> > smp_mb__after_atomic();
> > r0 = *y;
> >
> > Which the CPU is then allowed to re-order (under TSO rules) like:
> >
> > atomic_inc(u);
> > r0 = *y;
> > *x = 1;
> >
> > And this very much was not intended.
> >
> > This had me audit all the (strong) architectures that had weak
> > smp_mb__{before,after}_atomic: ia64, mips, sparc, s390, x86, xtensa.
> >
> > Of those, only x86 and mips were affected. Looking at MIPS to solve this, led
> > to the other MIPS patches.
> >
> > All these patches have been through 0day for quite a while.
> >
> > Paul, how do you want to route the MIPS bits?
>
> Paul; afaict the MIPS patches still apply (ie. they've not made their
> way into Linus' tree yet).
>
> I thought you were going to take them?

My apologies, between the linux-mips mailing list not being copied (so
this didn't end up in patchwork) and my being away for my father's
funeral this fell through the cracks.

I'll go apply them to mips-next for v5.4.

Thanks for following up again,

Paul