On Fri, Nov 25, 2016 at 3:56 PM, Boqun Feng <boqun.feng at gmail.com> wrote:> On Fri, Nov 25, 2016 at 01:44:04PM +0100, Peter Zijlstra wrote: >> On Fri, Nov 25, 2016 at 01:40:44PM +0100, Peter Zijlstra wrote: >> > #define SINGLE_LOAD(x) \ >> > {( \ >> > compiletime_assert_atomic_type(typeof(x)); \ >> >> Should be: >> >> compiletime_assert_atomic_type(x); >> >> > WARN_SINGLE_COPY_ALIGNMENT(&(x)); \ > > Do we need to worry about the side effect on x? Maybe > > #define SINGLE_LOAD(x) \ > ({ \ > typeof(x) *_____ptr; \ > \ > compiletime_assert_atomic_type(typeof(x)); \ > \ > _____ptr = &(x); \ > \ > WARN_SINGLE_COPY_ALIGNMENT(_____ptr); \ > \ > READ_ONCE(*_____ptr); \ > }) > > Ditto for SINGLE_STORE() > > Regards, > Boqun > >> > READ_ONCE(x); \ >> > }) >> > >> > #define SINGLE_STORE(x, v) \ >> > ({ \ >> > compiletime_assert_atomic_type(typeof(x)); \ >> >> idem >> >> > WARN_SINGLE_COPY_ALIGNMENT(&(x)); \ >> > WRITE_ONCE(x, v); \ >> > })READ/WRITE_ONCE imply atomicity. Even if their names don't spell it (a function name doesn't have to spell all of its guarantees). Most of the uses of READ/WRITE_ONCE will be broken if they are not atomic. "Read once but not necessary atomically" is a very subtle primitive which is very easy to misuse. What are use cases for such primitive that won't be OK with "read once _and_ atomically"? Copy to/from user is obviously one such case, but it is already handled specially.
On Fri, Nov 25, 2016 at 04:21:39PM +0100, Dmitry Vyukov wrote:> > READ/WRITE_ONCE imply atomicity. Even if their names don't spell it (a > function name doesn't have to spell all of its guarantees). Most of > the uses of READ/WRITE_ONCE will be broken if they are not atomic.In practice, this is certainly the assumption made by many/most users of the *_ONCE() accessors. Looking again, Linus does seem to agree that word-sized accesses should result in single instructions (and be single-copy atomic) [1], so in contrast to [2], that's clearly *part* of the point of the *_ONCE() accessors...> "Read once but not necessary atomically" is a very subtle primitive > which is very easy to misuse.I agree. Unfortunately, Linus does not appear to [2].> What are use cases for such primitive that won't be OK with "read once > _and_ atomically"?I have none to hand. Thanks, Mark. [1] http://lkml.iu.edu/hypermail/linux/kernel/1503.3/02674.html [2] http://lkml.iu.edu/hypermail/linux/kernel/1503.3/02670.html
On Fri, Nov 25, 2016 at 04:10:04PM +0000, Mark Rutland wrote:> On Fri, Nov 25, 2016 at 04:21:39PM +0100, Dmitry Vyukov wrote:> > What are use cases for such primitive that won't be OK with "read once > > _and_ atomically"? > > I have none to hand.Whatever triggers the __builtin_memcpy() paths, and even the size==8 paths on 32bit. You could put a WARN in there to easily find them. The advantage of introducing the SINGLE_{LOAD,STORE}() helpers is that they compiletime validate this the size is 'right' and can runtime check alignment constraints. IE, they are strictly stronger than {READ,WRITE}_ONCE().