On Tue, Nov 05, 2013 at 02:05:48PM +0000, Will Deacon wrote:
> > > +
> > > +#define smp_store_release(p, v)
> > > \
> > > +do { \
> > > + smp_mb(); \
> > > + ACCESS_ONCE(p) = (v); \
> > > +} while (0)
> > > +
> > > +#define smp_load_acquire(p, v)
> > > \
> > > +do { \
> > > + typeof(p) ___p1 = ACCESS_ONCE(p); \
> > > + smp_mb(); \
> > > + return ___p1; \
> > > +} while (0)
>
> What data sizes do these accessors operate on? Assuming that we want
> single-copy atomicity (with respect to interrupts in the UP case), we
> probably want a check to stop people passing in things like structs.
Fair enough; I think we should restrict to native word sizes same as we
do for atomics.
Something like so perhaps:
#ifdef CONFIG_64BIT
#define __check_native_word(t) (sizeof(t) == 4 || sizeof(t) == 8)
#else
#define __check_native_word(t) (sizeof(t) == 4)
#endif
#define smp_store_release(p, v) \
do { \
BUILD_BUG_ON(!__check_native_word(p)); \
smp_mb(); \
ACCESS_ONCE(p) = (v); \
} while (0)
> > > +#define smp_store_release(p, v)
> > > \
> > > +do { \
> > > + asm volatile ("stlr %w0 [%1]" : : "r" (v), "r" (&p) : "memory");\
>
> Missing comma between the operands. Also, that 'w' output modifier enforces
> a 32-bit store (same early question about sizes). Finally, it might be more
> efficient to use "=Q" for the addressing mode, rather than take the address
> of p manually.
so something like:
asm volatile ("stlr %0, [%1]" : : "r" (v), "=Q" (p) : "memory");
?
My inline asm foo is horrid and I mostly get by with copy paste from a
semi similar existing form :/
> Random other question: have you considered how these accessors should behave
> when presented with __iomem pointers?
A what? ;-)
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
