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
