Re: [PATCH] hugetlb: fix lockdep splat caused by pmd sharing

[Peter Zijlstra]

On Tue, Jul 30, 2013 at 04:46:00PM +0200, Michal Hocko wrote:
> which is a false positive caused by hugetlb pmd sharing code which
> allocates a new pmd from withing mappint->i_mmap_mutex. If this
> allocation causes reclaim then the lockdep detector complains that we
> might self-deadlock.
> 
> This is not correct though, because hugetlb pages are not reclaimable so
> their mapping will be never touched from the reclaim path.
> 
> The patch tells lockup detector that hugetlb i_mmap_mutex is special
> by assigning it a separate lockdep class so it won't report possible
> deadlocks on unrelated mappings.
> 
> Reported-by: Dave Jones <davej@xxxxxxxxxx>
> Signed-off-by: Michal Hocko <mhocko@xxxxxxx>
> ---
>  fs/hugetlbfs/inode.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index a3f868a..230533d 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -463,6 +463,12 @@ static struct inode *hugetlbfs_get_root(struct super_block *sb,
>  	return inode;
>  }
>  
> +/*
> + * Now, reclaim path never holds hugetlbfs_inode->i_mmap_mutex while it could
> + * hold normal inode->i_mmap_mutex so this annotation avoids a lockdep splat.

How about something like:

/*
 * Hugetlbfs is not reclaimable; therefore its i_mmap_mutex will never
 * be taken from reclaim -- unlike regular filesystems. This needs an
 * annotation because huge_pmd_share() does an allocation under
 * i_mmap_mutex.
 */

It clarifies the exact conditions and makes easier to verify the
validity of the annotation.

> + */
> +struct lock_class_key hugetlbfs_i_mmap_mutex_key;
> +
>  static struct inode *hugetlbfs_get_inode(struct super_block *sb,
>  					struct inode *dir,
>  					umode_t mode, dev_t dev)
> @@ -474,6 +480,8 @@ static struct inode *hugetlbfs_get_inode(struct super_block *sb,
>  		struct hugetlbfs_inode_info *info;
>  		inode->i_ino = get_next_ino();
>  		inode_init_owner(inode, dir, mode);
> +		lockdep_set_class(&inode->i_mapping->i_mmap_mutex,
> +				&hugetlbfs_i_mmap_mutex_key);
>  		inode->i_mapping->a_ops = &hugetlbfs_aops;
>  		inode->i_mapping->backing_dev_info =&hugetlbfs_backing_dev_info;
>  		inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
> -- 
> 1.8.3.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/