Re: [PATCH 06/11] xfs: replace i_flock with a sleeping bitlock

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

On Wed, Dec 14, 2011 at 09:19:17AM +1100, Dave Chinner wrote:
> > +static inline void xfs_ifunlock(struct xfs_inode *ip)
> > +{
> > +   xfs_iflags_clear(ip, XFS_IFLOCK);
> > +   wake_up_bit(&ip->i_flags, __XFS_IFLOCK_BIT);
> > +}
> 
> Should the wakeup be done whilst the ip->i_flags_lock is still held?

There is no reason to do so - __XFS_IFLOCK_BIT is only used as a wakeup key
in wake_up_bit, so it can easily be done after the unlocking.  Doing so
is indeed a lot more efficient as the waiter needs to take the locks as
the first thing after beeing woken.  

> The VFS code does the __I_SYNC wakeup while still holding the
> inode->i_lock so that the clear and wakeup are atomic, similarly the
> __I_NEW bit....

It implements a bit different semantics, and actually needs to hold the
lock for more synchronization than just the wakeup bit.  It does in fact
use the bit wakeup helpers using the atomic test/set bit operations
despite holding a lock for some reason.