Re: memory barrier question

[Miklos Szeredi]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/