From 34a3d1e83708702ac6cb872215e68cd07dae298b Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Thu, 11 Oct 2007 22:11:12 +0200 Subject: lockdep: annotate journal_start() On Fri, 2007-07-13 at 02:05 -0700, Andrew Morton wrote: > Except lockdep doesn't know about journal_start(), which has ranking > requirements similar to a semaphore. Signed-off-by: Peter Zijlstra Signed-off-by: Ingo Molnar --- fs/jbd/transaction.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'fs') diff --git a/fs/jbd/transaction.c b/fs/jbd/transaction.c index 772b6531a2a2..8df5bac0b7a5 100644 --- a/fs/jbd/transaction.c +++ b/fs/jbd/transaction.c @@ -233,6 +233,8 @@ out: return ret; } +static struct lock_class_key jbd_handle_key; + /* Allocate a new handle. This should probably be in a slab... */ static handle_t *new_handle(int nblocks) { @@ -243,6 +245,8 @@ static handle_t *new_handle(int nblocks) handle->h_buffer_credits = nblocks; handle->h_ref = 1; + lockdep_init_map(&handle->h_lockdep_map, "jbd_handle", &jbd_handle_key, 0); + return handle; } @@ -286,6 +290,9 @@ handle_t *journal_start(journal_t *journal, int nblocks) current->journal_info = NULL; handle = ERR_PTR(err); } + + lock_acquire(&handle->h_lockdep_map, 0, 0, 0, 2, _THIS_IP_); + return handle; } @@ -1411,6 +1418,8 @@ int journal_stop(handle_t *handle) spin_unlock(&journal->j_state_lock); } + lock_release(&handle->h_lockdep_map, 1, _THIS_IP_); + jbd_free_handle(handle); return err; } -- cgit 1.4.1 From d475fd428ce77aa2a8bc650d230e17663a4f49c3 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 15 Oct 2007 14:51:31 +0200 Subject: lockdep: per filesystem inode lock class Give each filesystem its own inode lock class. The various filesystems have different locking order wrt the inode locks; esp. the pseudo filesystems differ from the rest. Signed-off-by: Peter Zijlstra --- fs/inode.c | 12 +++++++++--- include/linux/fs.h | 5 +++++ 2 files changed, 14 insertions(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/inode.c b/fs/inode.c index 29f5068f819b..bf6adf122c68 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -142,6 +142,15 @@ static struct inode *alloc_inode(struct super_block *sb) return NULL; } + spin_lock_init(&inode->i_lock); + lockdep_set_class(&inode->i_lock, &sb->s_type->i_lock_key); + + mutex_init(&inode->i_mutex); + lockdep_set_class(&inode->i_mutex, &sb->s_type->i_mutex_key); + + init_rwsem(&inode->i_alloc_sem); + lockdep_set_class(&inode->i_alloc_sem, &sb->s_type->i_alloc_sem_key); + mapping->a_ops = &empty_aops; mapping->host = inode; mapping->flags = 0; @@ -190,8 +199,6 @@ void inode_init_once(struct inode *inode) INIT_HLIST_NODE(&inode->i_hash); INIT_LIST_HEAD(&inode->i_dentry); INIT_LIST_HEAD(&inode->i_devices); - mutex_init(&inode->i_mutex); - init_rwsem(&inode->i_alloc_sem); INIT_RADIX_TREE(&inode->i_data.page_tree, GFP_ATOMIC); rwlock_init(&inode->i_data.tree_lock); spin_lock_init(&inode->i_data.i_mmap_lock); @@ -199,7 +206,6 @@ void inode_init_once(struct inode *inode) spin_lock_init(&inode->i_data.private_lock); INIT_RAW_PRIO_TREE_ROOT(&inode->i_data.i_mmap); INIT_LIST_HEAD(&inode->i_data.i_mmap_nonlinear); - spin_lock_init(&inode->i_lock); i_size_ordered_init(inode); #ifdef CONFIG_INOTIFY INIT_LIST_HEAD(&inode->inotify_watches); diff --git a/include/linux/fs.h b/include/linux/fs.h index 16421f662a7a..0cad20e12585 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1302,8 +1302,13 @@ struct file_system_type { struct module *owner; struct file_system_type * next; struct list_head fs_supers; + struct lock_class_key s_lock_key; struct lock_class_key s_umount_key; + + struct lock_class_key i_lock_key; + struct lock_class_key i_mutex_key; + struct lock_class_key i_alloc_sem_key; }; extern int get_sb_bdev(struct file_system_type *fs_type, -- cgit 1.4.1 From 14358e6ddaed27499d7d366b3e65c3e46b39e1c4 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Sun, 14 Oct 2007 01:38:33 +0200 Subject: 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 --- fs/inode.c | 12 ++++++++++++ include/linux/fs.h | 1 + 2 files changed, 13 insertions(+) (limited to 'fs') 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); void unlock_new_inode(struct inode *inode) { +#ifdef CONFIG_DEBUG_LOCK_ALLOC + struct file_system_type *type = inode->i_sb->s_type; + /* + * ensure nobody is actually holding i_mutex + */ + mutex_destroy(&inode->i_mutex); + mutex_init(&inode->i_mutex); + if (inode->i_mode & S_IFDIR) + lockdep_set_class(&inode->i_mutex, &type->i_mutex_dir_key); + else + lockdep_set_class(&inode->i_mutex, &type->i_mutex_key); +#endif /* * This is special! We do not need the spinlock * 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 { struct lock_class_key i_lock_key; struct lock_class_key i_mutex_key; + struct lock_class_key i_mutex_dir_key; struct lock_class_key i_alloc_sem_key; }; -- cgit 1.4.1