On Sun, Sep 19, 2010 at 10:15:51PM +0200, Miklos Szeredi wrote: > On Sun, 19 Sep 2010, Paul E. McKenney wrote: > > Give it a few years. There are reportedly already other compilers that do > > this, which is not too surprising given that the perception of insanity > > is limited to lockless parallel code. If you have single-threaded code, > > such as code and data under a lock (where the data is never accessed > > without holding that lock), then this sort of optimization is pretty safe. > > I still don't like it, but the compiler guys would argue that this is > > because I am one of those insane parallel-programming guys. > > > > Furthermore, there are other ways to get into trouble. If the code > > continued as follows: > > > > LOAD inode = next.dentry->inode > > if (inode != NULL) > > LOAD inode->f_op > > do_something_using_lots_of_registers(); > > LOAD inode->some_other_field > > > > and if the code expected ->f_op and ->some_other_field to be from the > > same inode structure, severe disappointment could ensue. This is because > > the compiler is within its rights to reload from next.dentry->inode, > > especially given register pressure. In fact, the compiler would be within > > its rights to reload from next.dentry->inode in the "LOAD inode->f_op" > > statement. And it might well get NULL from such a reload. > > Except the VFS doesn't allow that. dentry->inode can go from NULL to > non-NULL anytime but will only go from non-NULL to NULL when there are > no possible external references to the dentry. > > The compiler and the CPU cannot move the "LOAD inode->some_field" > before the "LOAD dentry->inode", because of the conditional, right? Other than Alpha, the CPU cannot. The standard -does- permit the compiler to guess the value of the pointer, thus effectively moving the load prior to the conditional. At present, as far as I know, gcc does not actually do this. Again, please put at least an ACCESS_ONCE() in. Trivial to do now, possibly saving much pain and headache later on. Thanx, Paul -- 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
