On Tue, Aug 30, 2011 at 02:39:49AM -0400, Christoph Hellwig wrote:
> On Tue, Aug 30, 2011 at 04:24:16PM +1000, Dave Chinner wrote:
> > > xfs_mark_inode_dirty_sync(
> > > @@ -82,6 +81,10 @@ xfs_mark_inode_dirty_sync(
> > >
> > > if (!(inode->i_state & (I_WILL_FREE|I_FREEING)))
> > > mark_inode_dirty_sync(inode);
> > > + else {
> > > + barrier();
> > > + ip->i_update_core = 1;
> > > + }
> > > }
> >
> > Why the barrier()? Isn't that just a compiler barrier? If you are
> > worried about catching the update vs clearing it in transaction
> > commit, shouldn't that use smp_mb() instead (in both places)?
>
> It's a blind copy & past from xfs_fs_dirty_inode. The comments
> there suggests it is for update ordering.
Right, I've always wondered about that, because the corresponding
code talks about requiring strongly ordered memory semantics and
that the compiler does this via SYNCHRONIZE() (barrier).
>From xfs_inode_item_format:
* We clear i_update_core before copying out the data.
* This is for coordination with our timestamp updates
* that don't hold the inode lock. They will always
* update the timestamps BEFORE setting i_update_core,
* so if we clear i_update_core after they set it we
* are guaranteed to see their updates to the timestamps
* either here. Likewise, if they set it after we clear it
* here, we'll see it either on the next commit of this
* inode or the next time the inode gets flushed via
* xfs_iflush(). This depends on strongly ordered memory
* semantics, but we have that. We use the SYNCHRONIZE
* macro to make sure that the compiler does not reorder
* the i_update_core access below the data copy below.
*/
if (ip->i_update_core) {
ip->i_update_core = 0;
SYNCHRONIZE();
}
Now that may have been true on Irix/MIPS which had strong memory
ordering so only compiler barriers were necessary.
However, normally when we talk about ordered memory semantics in
Linux, we cannot assume strong ordering - if we have ordering
requirements, we have to guarantee ordering by explicit use of
memory barriers, right?
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
