On Thu, Dec 08, 2011 at 10:58:01AM -0500, Christoph Hellwig wrote:
> We almost never block on i_flock, the exception is synchronous inode
> flushing. Instead of bloating the inode with a 16/24-byte completion
> that we abuse as a semaphore just implement it as a bitlock that uses
> a bit waitqueue for the rare sleeping path. This primarily is a
> tradeoff between a much smaller inode and a faster non-blocking
> path vs faster wakeups, and we are much better off with the former.
>
> A small downside is that we will lose lockdep checking for i_flock, but
> given that it's always taken inside the ilock that should be acceptable.
I didn't think we had lockdep checking on the i_flock because it
uses completions rather than real lock primitives. Either way, I'm
not concerned about this aspect of the change - lockdep doesn't pick
up the typical sort of holdoff problems that i_flock vs delwri
trigger...
> Note that for example the inode writeback locking is implemented in a
> very similar way.
....
> +
> +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?
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....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|