On Thu, 16 Sep 2010, David Howells wrote: > Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > > Consider the following example: > > > > Start: > > p = NULL; > > x = 0; > > > > CPU1: > > atomic_inc(&x); > > p = &x; > > > > CPU2: > > if (p) > > z = atomic_read(p); > > > > Is it possible to end up with z == 0? > > I think so. I'm not sure that you can assume that CPU1 does its two > 'operations' in the same order. You can guarantee that the read of x, > increment, and write of x will be done in an order, and that no one else will > see an intermediate state, but you can't guarantee that CPU2 will see x > changed before p is changed. > > In Documentation/memory-barriers.txt, it says: > > The following also do _not_ imply memory barriers, and so may require > explicit memory barriers under some circumstances > (smp_mb__before_atomic_dec() for instance): > > atomic_add(); > atomic_sub(); > atomic_inc(); > atomic_dec(); > > so you need _two_ memory barriers, e.g.: > > CPU1: > atomic_inc(&x); > smp_mb__after_atomic_inc() > p = &x; > > CPU2: > q = p; > smp_rmb(); > if (q) > z = atomic_read(q); > > Note that atomic_inc() may imply a suitable memory barrier on some arches, and > so has special variant barrier functions of its own. Is the rmb() really needed? Take this code from fs/namei.c for example: inode = next.dentry->d_inode; if (!inode) goto out_dput; if (inode->i_op->follow_link) { It happily dereferences dentry->d_inode without a barrier after checking it for non-null, while that d_inode might have just been initialized on another CPU with a freshly created inode. There's absolutely no synchornization with that on this side. Isn't the fact that we check the pointer for being non-null (together with locking/barrier on the other side) enough to ensure that it's safe to dereference it? Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html