Skip to content

Commit e096d0c

Browse files
jwboyertorvalds
authored andcommitted
lockdep: Add helper function for dir vs file i_mutex annotation
Purely in-memory filesystems do not use the inode hash as the dcache tells us if an entry already exists. As a result, they do not call unlock_new_inode, and thus directory inodes do not get put into a different lockdep class for i_sem. We need the different lockdep classes, because the locking order for i_mutex is different for directory inodes and regular inodes. Directory inodes can do "readdir()", which takes i_mutex *before* possibly taking mm->mmap_sem (due to a page fault while copying the directory entry to user space). In contrast, regular inodes can be mmap'ed, which takes mm->mmap_sem before accessing i_mutex. The two cases can never happen for the same inode, so no real deadlock can occur, but without the different lockdep classes, lockdep cannot understand that. As a result, if CONFIG_DEBUG_LOCK_ALLOC is set, this can lead to false positives from lockdep like below: find/645 is trying to acquire lock: (&mm->mmap_sem){++++++}, at: [<ffffffff81109514>] might_fault+0x5c/0xac but task is already holding lock: (&sb->s_type->i_mutex_key#15){+.+.+.}, at: [<ffffffff81149f34>] vfs_readdir+0x5b/0xb4 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&sb->s_type->i_mutex_key#15){+.+.+.}: [<ffffffff8108ac26>] lock_acquire+0xbf/0x103 [<ffffffff814db822>] __mutex_lock_common+0x4c/0x361 [<ffffffff814dbc46>] mutex_lock_nested+0x40/0x45 [<ffffffff811daa87>] hugetlbfs_file_mmap+0x82/0x110 [<ffffffff81111557>] mmap_region+0x258/0x432 [<ffffffff811119dd>] do_mmap_pgoff+0x2ac/0x306 [<ffffffff81111b4f>] sys_mmap_pgoff+0x118/0x16a [<ffffffff8100c858>] sys_mmap+0x22/0x24 [<ffffffff814e3ec2>] system_call_fastpath+0x16/0x1b -> #0 (&mm->mmap_sem){++++++}: [<ffffffff8108a4bc>] __lock_acquire+0xa1a/0xcf7 [<ffffffff8108ac26>] lock_acquire+0xbf/0x103 [<ffffffff81109541>] might_fault+0x89/0xac [<ffffffff81149cff>] filldir+0x6f/0xc7 [<ffffffff811586ea>] dcache_readdir+0x67/0x205 [<ffffffff81149f54>] vfs_readdir+0x7b/0xb4 [<ffffffff8114a073>] sys_getdents+0x7e/0xd1 [<ffffffff814e3ec2>] system_call_fastpath+0x16/0x1b This patch moves the directory vs file lockdep annotation into a helper function that can be called by in-memory filesystems and has hugetlbfs call it. Signed-off-by: Josh Boyer <[email protected]> Acked-by: Peter Zijlstra <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent e33f2d2 commit e096d0c

File tree

3 files changed

+21
-9
lines changed

3 files changed

+21
-9
lines changed

fs/hugetlbfs/inode.c

+1
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ static struct inode *hugetlbfs_get_inode(struct super_block *sb, uid_t uid,
491491
inode->i_op = &page_symlink_inode_operations;
492492
break;
493493
}
494+
lockdep_annotate_inode_mutex_key(inode);
494495
}
495496
return inode;
496497
}

fs/inode.c

+15-9
Original file line numberDiff line numberDiff line change
@@ -848,16 +848,9 @@ struct inode *new_inode(struct super_block *sb)
848848
}
849849
EXPORT_SYMBOL(new_inode);
850850

851-
/**
852-
* unlock_new_inode - clear the I_NEW state and wake up any waiters
853-
* @inode: new inode to unlock
854-
*
855-
* Called when the inode is fully initialised to clear the new state of the
856-
* inode and wake up anyone waiting for the inode to finish initialisation.
857-
*/
858-
void unlock_new_inode(struct inode *inode)
859-
{
860851
#ifdef CONFIG_DEBUG_LOCK_ALLOC
852+
void lockdep_annotate_inode_mutex_key(struct inode *inode)
853+
{
861854
if (S_ISDIR(inode->i_mode)) {
862855
struct file_system_type *type = inode->i_sb->s_type;
863856

@@ -873,7 +866,20 @@ void unlock_new_inode(struct inode *inode)
873866
&type->i_mutex_dir_key);
874867
}
875868
}
869+
}
870+
EXPORT_SYMBOL(lockdep_annotate_inode_mutex_key);
876871
#endif
872+
873+
/**
874+
* unlock_new_inode - clear the I_NEW state and wake up any waiters
875+
* @inode: new inode to unlock
876+
*
877+
* Called when the inode is fully initialised to clear the new state of the
878+
* inode and wake up anyone waiting for the inode to finish initialisation.
879+
*/
880+
void unlock_new_inode(struct inode *inode)
881+
{
882+
lockdep_annotate_inode_mutex_key(inode);
877883
spin_lock(&inode->i_lock);
878884
WARN_ON(!(inode->i_state & I_NEW));
879885
inode->i_state &= ~I_NEW;

include/linux/fs.h

+5
Original file line numberDiff line numberDiff line change
@@ -2318,6 +2318,11 @@ extern struct inode * iget5_locked(struct super_block *, unsigned long, int (*te
23182318
extern struct inode * iget_locked(struct super_block *, unsigned long);
23192319
extern int insert_inode_locked4(struct inode *, unsigned long, int (*test)(struct inode *, void *), void *);
23202320
extern int insert_inode_locked(struct inode *);
2321+
#ifdef CONFIG_DEBUG_LOCK_ALLOC
2322+
extern void lockdep_annotate_inode_mutex_key(struct inode *inode);
2323+
#else
2324+
static inline void lockdep_annotate_inode_mutex_key(struct inode *inode) { };
2325+
#endif
23212326
extern void unlock_new_inode(struct inode *);
23222327
extern unsigned int get_next_ino(void);
23232328

0 commit comments

Comments
 (0)