On 02/18/2014 11:16 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The change to add the IO lock to protect the directory extent map
> during readdir operations has cause lockdep to have a heart attack
> as it now sees a different locking order on inodes w.r.t. the
> mmap_sem because readdir has a different ordering to write().
> 
> Add a new lockdep class for directory inodes to avoid this false
> positive.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Hey Dave,

I'm not terribly familiar with lockdep, but I hit the attached "possible
circular locking dependency detected" warning when running with this patch.

(Reproduces by running generic/001 after a reboot).

Brian

>  fs/xfs/xfs_iops.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 9ddfb81..bb3bb65 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -48,6 +48,18 @@
>  #include <linux/fiemap.h>
>  #include <linux/slab.h>
>  
> +/*
> + * Directories have different lock order w.r.t. mmap_sem compared to regular
> + * files. This is due to readdir potentially triggering page faults on a user
> + * buffer inside filldir(), and this happens with the ilock on the directory
> + * held. For regular files, the lock order is the other way around - the
> + * mmap_sem is taken during the page fault, and then we lock the ilock to do
> + * block mapping. Hence we need a different class for the directory ilock so
> + * that lockdep can tell them apart.
> + */
> +static struct lock_class_key xfs_nondir_ilock_class;
> +static struct lock_class_key xfs_dir_ilock_class;
> +
>  static int
>  xfs_initxattrs(
>  	struct inode		*inode,
> @@ -1191,6 +1203,7 @@ xfs_setup_inode(
>  	xfs_diflags_to_iflags(inode, ip);
>  
>  	ip->d_ops = ip->i_mount->m_nondir_inode_ops;
> +	lockdep_set_class(&ip->i_lock.mr_lock, &xfs_nondir_ilock_class);
>  	switch (inode->i_mode & S_IFMT) {
>  	case S_IFREG:
>  		inode->i_op = &xfs_inode_operations;
> @@ -1198,6 +1211,7 @@ xfs_setup_inode(
>  		inode->i_mapping->a_ops = &xfs_address_space_operations;
>  		break;
>  	case S_IFDIR:
> +		lockdep_set_class(&ip->i_lock.mr_lock, &xfs_dir_ilock_class);
>  		if (xfs_sb_version_hasasciici(&XFS_M(inode->i_sb)->m_sb))
>  			inode->i_op = &xfs_dir_ci_inode_operations;
>  		else
>