On Tue, Nov 05, 2013 at 02:05:48PM +0000, Will Deacon wrote:

quoted

quoted

+
+#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)

quoted

quoted

+#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? ;-)