Messages in this thread | | | Date | Thu, 13 Feb 2020 09:58:49 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH 7/7] microblaze: Do atomic operations by using exclusive ops |
| |
On Thu, Feb 13, 2020 at 09:06:24AM +0100, Michal Simek wrote: > On 12. 02. 20 16:55, Peter Zijlstra wrote: > > On Wed, Feb 12, 2020 at 04:42:29PM +0100, Michal Simek wrote:
> >> +static inline void atomic_set(atomic_t *v, int i) > >> +{ > >> + int result, tmp; > >> + > >> + __asm__ __volatile__ ( > >> + /* load conditional address in %2 to %0 */ > >> + "1: lwx %0, %2, r0;\n" > >> + /* attempt store */ > >> + " swx %3, %2, r0;\n" > >> + /* checking msr carry flag */ > >> + " addic %1, r0, 0;\n" > >> + /* store failed (MSR[C] set)? try again */ > >> + " bnei %1, 1b;\n" > >> + /* Outputs: result value */ > >> + : "=&r" (result), "=&r" (tmp) > >> + /* Inputs: counter address */ > >> + : "r" (&v->counter), "r" (i) > >> + : "cc", "memory" > >> + ); > >> +} > >> +#define atomic_set atomic_set > > > > Uuuuhh.. *what* ?!? > > > > Are you telling me your LL/SC implementation is so bugger that > > atomic_set() being a WRITE_ONCE() does not in fact work? > > Just keep in your mind that this code was written long time ago and > there could be a lot of things/technique used at that time by IIRC > powerpc and I hope that review process will fix these things and I > really appreciation your comments.
I don't think I've ever seen Power do this, but I've not checked the git history.
> Stefan is the right person to say if we really need to use exclusive > loads/stores instructions or use what I see in include/linux/compiler.h. > > Please correct me if I am wrong. > WRITE_ONCE is __write_once_size which is normal write in C which I > expect will be converted in asm to non exclusive writes. And barrier is > called only for cases above 8bytes. > > READ_ONCE is normal read follow by barrier all the time.
Right:
WRITE_ONCE() is something like:
*(volatile typeof(var)*)(&(var)) = val;
And should translate to just a regular store; the volatile just tells the C compiler it should not do funny things with it.
READ_ONCE() is something like:
val = *(volatile typeof(var)*)(&(var));
And should translate to just a regular load; the volatile again tells the compiler to not be funny about it.
No memory barriers what so ever, not even a compiler barrier as such.
The thing is, your bog standard LL/SC _SHOULD_ fail the SC if someone else does a regular store to the same variable. See the example in Documentation/atomic_t.txt.
That is, a competing SW/SWI should result in the interconnect responding with something other than EXOKAY, the SWX should fail and MSR[C] <- 1.
> Also is there any testsuite I should run to verify all these atomics > operations? That would really help but I haven't seen any tool (but also > didn't try hard to find it out).
Will, Paul; can't this LKMM thing generate kernel modules to run? And do we have a 'nice' collection of litmus tests that cover atomic_t ?
The one in atomic_t.txt should cover this one at least.
| |