lockdep: Add helper function for dir vs file i_mutex annotation

jwboyer · torvalds · commit e096d0c7e2e4 · 2011-08-25T10:50:18.000-07:00
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-&gt;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-&gt;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:
     (&amp;mm-&gt;mmap_sem){++++++}, at: [&lt;ffffffff81109514&gt;] might_fault+0x5c/0xac

    but task is already holding lock:
     (&amp;sb-&gt;s_type-&gt;i_mutex_key#15){+.+.+.}, at: [&lt;ffffffff81149f34&gt;]
    vfs_readdir+0x5b/0xb4

    which lock already depends on the new lock.

    the existing dependency chain (in reverse order) is:

    -&gt; #1 (&amp;sb-&gt;s_type-&gt;i_mutex_key#15){+.+.+.}:
          [&lt;ffffffff8108ac26&gt;] lock_acquire+0xbf/0x103
          [&lt;ffffffff814db822&gt;] __mutex_lock_common+0x4c/0x361
          [&lt;ffffffff814dbc46&gt;] mutex_lock_nested+0x40/0x45
          [&lt;ffffffff811daa87&gt;] hugetlbfs_file_mmap+0x82/0x110
          [&lt;ffffffff81111557&gt;] mmap_region+0x258/0x432
          [&lt;ffffffff811119dd&gt;] do_mmap_pgoff+0x2ac/0x306
          [&lt;ffffffff81111b4f&gt;] sys_mmap_pgoff+0x118/0x16a
          [&lt;ffffffff8100c858&gt;] sys_mmap+0x22/0x24
          [&lt;ffffffff814e3ec2&gt;] system_call_fastpath+0x16/0x1b

    -&gt; #0 (&amp;mm-&gt;mmap_sem){++++++}:
          [&lt;ffffffff8108a4bc&gt;] __lock_acquire+0xa1a/0xcf7
          [&lt;ffffffff8108ac26&gt;] lock_acquire+0xbf/0x103
          [&lt;ffffffff81109541&gt;] might_fault+0x89/0xac
          [&lt;ffffffff81149cff&gt;] filldir+0x6f/0xc7
          [&lt;ffffffff811586ea&gt;] dcache_readdir+0x67/0x205
          [&lt;ffffffff81149f54&gt;] vfs_readdir+0x7b/0xb4
          [&lt;ffffffff8114a073&gt;] sys_getdents+0x7e/0xd1
          [&lt;ffffffff814e3ec2&gt;] 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 &lt;jwboyer@redhat.com&gt;
Acked-by: Peter Zijlstra &lt;a.p.zijlstra@chello.nl&gt;
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
@@ -491,6 +491,7 @@ static struct inode *hugetlbfs_get_inode(struct super_block *sb, uid_t uid,
 			inode->i_op = &page_symlink_inode_operations;
 			break;
 		}
+		lockdep_annotate_inode_mutex_key(inode);
 	}
 	return inode;
 }
diff --git a/fs/inode.c b/fs/inode.c
@@ -848,16 +848,9 @@ struct inode *new_inode(struct super_block *sb)
 }
 EXPORT_SYMBOL(new_inode);
 
-/**
- * unlock_new_inode - clear the I_NEW state and wake up any waiters
- * @inode:	new inode to unlock
- *
- * Called when the inode is fully initialised to clear the new state of the
- * inode and wake up anyone waiting for the inode to finish initialisation.
- */
-void unlock_new_inode(struct inode *inode)
-{
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
+void lockdep_annotate_inode_mutex_key(struct inode *inode)
+{
 	if (S_ISDIR(inode->i_mode)) {
 		struct file_system_type *type = inode->i_sb->s_type;
 
@@ -873,7 +866,20 @@ void unlock_new_inode(struct inode *inode)
 					  &type->i_mutex_dir_key);
 		}
 	}
+}
+EXPORT_SYMBOL(lockdep_annotate_inode_mutex_key);
 #endif
+
+/**
+ * unlock_new_inode - clear the I_NEW state and wake up any waiters
+ * @inode:	new inode to unlock
+ *
+ * Called when the inode is fully initialised to clear the new state of the
+ * inode and wake up anyone waiting for the inode to finish initialisation.
+ */
+void unlock_new_inode(struct inode *inode)
+{
+	lockdep_annotate_inode_mutex_key(inode);
 	spin_lock(&inode->i_lock);
 	WARN_ON(!(inode->i_state & I_NEW));
 	inode->i_state &= ~I_NEW;
diff --git a/include/linux/fs.h b/include/linux/fs.h
@@ -2318,6 +2318,11 @@ extern struct inode * iget5_locked(struct super_block *, unsigned long, int (*te
 extern struct inode * iget_locked(struct super_block *, unsigned long);
 extern int insert_inode_locked4(struct inode *, unsigned long, int (*test)(struct inode *, void *), void *);
 extern int insert_inode_locked(struct inode *);
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+extern void lockdep_annotate_inode_mutex_key(struct inode *inode);
+#else
+static inline void lockdep_annotate_inode_mutex_key(struct inode *inode) { };
+#endif
 extern void unlock_new_inode(struct inode *);
 extern unsigned int get_next_ino(void);
 

Original file line number	Diff line number	Diff line change
`@@ -491,6 +491,7 @@ static struct inode hugetlbfs_get_inode(struct super_block sb, uid_t uid,`
`491`	`491`	`inode->i_op = &page_symlink_inode_operations;`
`492`	`492`	`break;`
`493`	`493`	`}`
	`494`	`+ lockdep_annotate_inode_mutex_key(inode);`
`494`	`495`	`}`
`495`	`496`	`return inode;`
`496`	`497`	`}`