[v3,3/3] jffs2: put directories f->sem on a separate lockdep class

Message ID c578ea017c7bc73a5b3b0025f049a3c90efc1bd2.1545126261.git.helmut.grohne@intenta.de
State New
Delegated to: Richard Weinberger
Headers show
Series
  • [v3,1/3] jffs2: avoid unnecessarily taking f->sem
Related show

Commit Message

Helmut Grohne Dec. 18, 2018, 9:52 a.m.
We get a lockdep warning if we read a directory (e.g. ls) and fault a
page on a regular file:

[  181.206207] mmaptest/392 is trying to acquire lock:
[  181.211066]  (&f->sem){+.+.}, at: [<c01ff7d8>] jffs2_readpage+0x30/0x5c
[  181.217663]
[  181.217663] but task is already holding lock:
[  181.223477]  (&mm->mmap_sem){++++}, at: [<c04928e8>] do_page_fault+0xa8/0x454
[  181.230596]
[  181.230596] which lock already depends on the new lock.
[  181.230596]
[  181.238755]
[  181.238755] the existing dependency chain (in reverse order) is:
[  181.246219]
[  181.246219] -> #1 (&mm->mmap_sem){++++}:
[  181.251614]        __might_fault+0x90/0xc0
[  181.255694]        filldir+0xa4/0x1f4
[  181.259334]        jffs2_readdir+0xd8/0x1d4
[  181.263499]        iterate_dir+0x78/0x128
[  181.267490]        SyS_getdents+0x8c/0x12c
[  181.271576]        ret_fast_syscall+0x0/0x28
[  181.275818]
[  181.275818] -> #0 (&f->sem){+.+.}:
[  181.280690]        lock_acquire+0xfc/0x2b0
[  181.284763]        __mutex_lock+0x8c/0xb0c
[  181.288842]        mutex_lock_nested+0x2c/0x34
[  181.293272]        jffs2_readpage+0x30/0x5c
[  181.297438]        filemap_fault+0x600/0x73c
[  181.301690]        __do_fault+0x28/0xb8
[  181.305510]        handle_mm_fault+0x854/0xec0
[  181.309937]        do_page_fault+0x384/0x454
[  181.314188]        do_DataAbort+0x4c/0xc8
[  181.318181]        __dabt_usr+0x44/0x60
[  181.321999]        0xb6dffb02

Like with inode->i_rwsem, we recognize that this can never result in a
dead lock, because we cannot page fault directory inodes and we cannot
call getdents on non-directories. Thus we need different lockdep classes
depending on the mode. This mirrors what
lockdep_annotate_inode_mutex_key does to inodes.

Link: https://www.spinics.net/lists/linux-rt-users/msg07644.html
Signed-off-by: Helmut Grohne <helmut.grohne@intenta.de>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 fs/jffs2/fs.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Patch

diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
index 89a10b398d00..d4d691488a19 100644
--- a/fs/jffs2/fs.c
+++ b/fs/jffs2/fs.c
@@ -26,6 +26,9 @@ 
 #include <linux/crc32.h>
 #include "nodelist.h"
 
+static struct lock_class_key jffs2_inode_info_sem_key,
+	 jffs2_inode_info_sem_dir_key;
+
 static int jffs2_flash_setup(struct jffs2_sb_info *c);
 
 int jffs2_do_setattr (struct inode *inode, struct iattr *iattr)
@@ -277,6 +280,13 @@  struct inode *jffs2_iget(struct super_block *sb, unsigned long ino)
 
 	inode->i_mode = jemode_to_cpu(latest_node.mode);
 
+	/* Mirror lock class of inode->i_rwsem, see
+	 * lockdep_annotate_inode_mutex_key.
+	 */
+	if (S_ISDIR(inode->i_mode))
+		lockdep_set_class(&f->sem, &jffs2_inode_info_sem_dir_key);
+	else
+		lockdep_set_class(&f->sem, &jffs2_inode_info_sem_key);
 	mutex_lock(&f->sem);
 
 	i_uid_write(inode, je16_to_cpu(latest_node.uid));
@@ -439,6 +449,13 @@  struct inode *jffs2_new_inode (struct inode *dir_i, umode_t mode, struct jffs2_r
 
 	f = JFFS2_INODE_INFO(inode);
 	jffs2_init_inode_info(f);
+	/* Mirror lock class of inode->i_rwsem, see
+	 * lockdep_annotate_inode_mutex_key.
+	 */
+	if (S_ISDIR(inode->i_mode))
+		lockdep_set_class(&f->sem, &jffs2_inode_info_sem_dir_key);
+	else
+		lockdep_set_class(&f->sem, &jffs2_inode_info_sem_key);
 	mutex_lock(&f->sem);
 
 	memset(ri, 0, sizeof(*ri));