Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()

[Michael S. Tsirkin]

On Tue, Dec 05, 2017 at 07:39:46PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 05, 2017 at 08:31:20PM +0200, Michael S. Tsirkin wrote:
> 
> > Apropos, READ_ONCE is now asymmetrical with WRITE_ONCE.
> > 
> > I can read a pointer with READ_ONCE and be sure the value
> > is sane, but only if I also remember to put in smp_wmb before
> > WRITE_ONCE. Otherwise the pointer is ok but no guarantees
> > about the data pointed to.
> 
> That was already the case on everything except Alpha. And the canonical
> match do the data dependency is store_release, not wmb.

Oh, interesting

static __always_inline void __write_once_size(volatile void *p, void *res, int size)
{
        switch (size) {
        case 1: *(volatile __u8 *)p = *(__u8 *)res; break;
        case 2: *(volatile __u16 *)p = *(__u16 *)res; break;
        case 4: *(volatile __u32 *)p = *(__u32 *)res; break;
        case 8: *(volatile __u64 *)p = *(__u64 *)res; break;
        default:
                barrier();
                __builtin_memcpy((void *)p, (const void *)res, size);
                barrier();
        }
}

#define WRITE_ONCE(x, val) \
({                                                      \
        union { typeof(x) __val; char __c[1]; } __u =   \
                { .__val = (__force typeof(x)) (val) }; \
        __write_once_size(&(x), __u.__c, sizeof(x));    \
        __u.__val;                                      \
})

I don't see WRITE_ONCE inserting any barriers, release or
write.

So it seems that on an architecture where writes can be reordered,
if I do

*pointer = 0xa;
WRITE_ONCE(array[x], pointer);

array write might bypass the pointer write,
and readers will read a stale value.




-- 
MST