lkml.org 
[lkml]   [2013]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC] arch: Introduce new TSO memory barrier smp_tmb()
On Tue, Nov 05, 2013 at 06:49:43PM +0000, Peter Zijlstra wrote:
> 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)

Ok, if we want to support 32-bit accesses on 64-bit machines, that will
complicate some of your assembly (more below).

> #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 :/

Almost: you just need to drop the square brackets and make the memory
location an output operand:

asm volatile("stlr %1, %0" : "=Q" (p) : "r" (v) : "memory");

however, for a 32-bit access, you need to use an output modifier:

asm volatile("stlr %w1, %0" : "=Q" (p) : "r" (v) : "memory");

so I guess a switch on sizeof(p) is required.

> > Random other question: have you considered how these accessors should behave
> > when presented with __iomem pointers?
>
> A what? ;-)

Then let's go with Paul's suggestion of mandating __kernel, or the like
(unless we need to worry about __user addresses for things like futexes?).

Will


\
 
 \ /
  Last update: 2013-11-06 12:21    [W:0.082 / U:0.640 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site