diff options
author | Peter Zijlstra <a.p.zijlstra@chello.nl> | 2007-10-13 19:38:33 -0400 |
---|---|---|
committer | Peter Zijlstra <a.p.zijlstra@chello.nl> | 2007-10-13 19:38:33 -0400 |
commit | 14358e6ddaed27499d7d366b3e65c3e46b39e1c4 (patch) | |
tree | de5a8919db855568577d0388fe30b5d7689e1f90 | |
parent | d475fd428ce77aa2a8bc650d230e17663a4f49c3 (diff) |
lockdep: annotate dir vs file i_mutex
On Mon, 2007-09-24 at 22:13 -0400, Steven Rostedt wrote:
> The circular lock seems to be this:
>
> #1:
>
> sys_mmap2: down_write(&mm->mmap_sem);
> nfs_revalidate_mapping: mutex_lock(&inode->i_mutex);
>
>
> #0:
>
> vfs_readdir: mutex_lock(&inode->i_mutex);
> - during the readdir (filldir64), we take a user fault (missing page?)
> and call do_page_fault -
> do_page_fault: down_read(&mm->mmap_sem);
>
>
> So it does indeed look like a circular locking. Now the question is, "is
> this a bug?". Looking like the inode of #1 must be a file or something
> else that you can mmap and the inode of #0 seems it must be a directory.
> I would say "no".
>
> Now if you can readdir on a file or mmap a directory, then this could be
> an issue.
>
> Otherwise, I'd love to see someone teach lockdep about this issue! ;-)
Make a distinction between file and dir usage of i_mutex.
The inode should be complete and unused at unlock_new_inode(), re-init
i_mutex depending on its type.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
-rw-r--r-- | fs/inode.c | 12 | ||||
-rw-r--r-- | include/linux/fs.h | 1 |
2 files changed, 13 insertions, 0 deletions
diff --git a/fs/inode.c b/fs/inode.c index bf6adf122c68..f97de0aeb3b6 100644 --- a/fs/inode.c +++ b/fs/inode.c | |||
@@ -567,6 +567,18 @@ EXPORT_SYMBOL(new_inode); | |||
567 | 567 | ||
568 | void unlock_new_inode(struct inode *inode) | 568 | void unlock_new_inode(struct inode *inode) |
569 | { | 569 | { |
570 | #ifdef CONFIG_DEBUG_LOCK_ALLOC | ||
571 | struct file_system_type *type = inode->i_sb->s_type; | ||
572 | /* | ||
573 | * ensure nobody is actually holding i_mutex | ||
574 | */ | ||
575 | mutex_destroy(&inode->i_mutex); | ||
576 | mutex_init(&inode->i_mutex); | ||
577 | if (inode->i_mode & S_IFDIR) | ||
578 | lockdep_set_class(&inode->i_mutex, &type->i_mutex_dir_key); | ||
579 | else | ||
580 | lockdep_set_class(&inode->i_mutex, &type->i_mutex_key); | ||
581 | #endif | ||
570 | /* | 582 | /* |
571 | * This is special! We do not need the spinlock | 583 | * This is special! We do not need the spinlock |
572 | * when clearing I_LOCK, because we're guaranteed | 584 | * when clearing I_LOCK, because we're guaranteed |
diff --git a/include/linux/fs.h b/include/linux/fs.h index 0cad20e12585..6d760f1ad875 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h | |||
@@ -1308,6 +1308,7 @@ struct file_system_type { | |||
1308 | 1308 | ||
1309 | struct lock_class_key i_lock_key; | 1309 | struct lock_class_key i_lock_key; |
1310 | struct lock_class_key i_mutex_key; | 1310 | struct lock_class_key i_mutex_key; |
1311 | struct lock_class_key i_mutex_dir_key; | ||
1311 | struct lock_class_key i_alloc_sem_key; | 1312 | struct lock_class_key i_alloc_sem_key; |
1312 | }; | 1313 | }; |
1313 | 1314 | ||