On Mon, Feb 04, 2013 at 10:13:23AM -0600, Alex Elder wrote: > In xfs_inode_item_unpin() there is a call to wake_up_bit() following > an independent test for whether waiters should be awakened. This > requires a memory barrier in order to guarantee correct operation > (see the comment above wake_up_bit()). > > Signed-off-by: Alex Elder <elder@xxxxxxxxxxx> > --- > fs/xfs/xfs_inode_item.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c > index d041d47..a7cacf7 100644 > --- a/fs/xfs/xfs_inode_item.c > +++ b/fs/xfs/xfs_inode_item.c > @@ -474,8 +474,10 @@ xfs_inode_item_unpin( > > trace_xfs_inode_unpin(ip, _RET_IP_); > ASSERT(atomic_read(&ip->i_pincount) > 0); > - if (atomic_dec_and_test(&ip->i_pincount)) > - wake_up_bit(&ip->i_flags, __XFS_IPINNED_BIT); > + if (!atomic_dec_and_test(&ip->i_pincount)) > + return; > + smp_mb(); > + wake_up_bit(&ip->i_flags, __XFS_IPINNED_BIT); I'm not sure this a barrier is actually needed here. The "wake up" bit is never stored or cleared anywhere in this case, it is used only to define a wait channel and directed wake up. Hence the "need a barrier so all CPUs see the cleared bit" case doesn't arise here. We use an atomic variable instead, and that makes it safe. If you read Documentation/atomic_ops.txt, you'll find that atomic modification operations are required to have explicit barrier semantics. i.e. that atomic_dec_and_test() must behave like it has both a smp_mb() before and after the atomic operation. i.e: Unlike the above routines, it is required that explicit memory barriers are performed before and after the operation. It must be done such that all memory operations before and after the atomic operation calls are strongly ordered with respect to the atomic operation itself. So, the smp_mb() that is added here is redundant - the atomic_dec_and_test() call already has the necesary memory barriers that wake_up_bit() requires. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
