From 454ed842d55740160334efc9ad56cfef54ed37bc Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 11 Aug 2008 09:30:25 +0200 Subject: lockdep: annotate mm_take_all_locks() The nesting is correct due to holding mmap_sem, use the new annotation to annotate this. Signed-off-by: Peter Zijlstra Signed-off-by: Ingo Molnar --- mm/mmap.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'mm') diff --git a/mm/mmap.c b/mm/mmap.c index 245c3d69067b..5d09d08a4120 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -2273,14 +2273,14 @@ int install_special_mapping(struct mm_struct *mm, static DEFINE_MUTEX(mm_all_locks_mutex); -static void vm_lock_anon_vma(struct anon_vma *anon_vma) +static void vm_lock_anon_vma(struct mm_struct *mm, struct anon_vma *anon_vma) { if (!test_bit(0, (unsigned long *) &anon_vma->head.next)) { /* * The LSB of head.next can't change from under us * because we hold the mm_all_locks_mutex. */ - spin_lock(&anon_vma->lock); + spin_lock_nest_lock(&anon_vma->lock, &mm->mmap_sem); /* * We can safely modify head.next after taking the * anon_vma->lock. If some other vma in this mm shares @@ -2296,7 +2296,7 @@ static void vm_lock_anon_vma(struct anon_vma *anon_vma) } } -static void vm_lock_mapping(struct address_space *mapping) +static void vm_lock_mapping(struct mm_struct *mm, struct address_space *mapping) { if (!test_bit(AS_MM_ALL_LOCKS, &mapping->flags)) { /* @@ -2310,7 +2310,7 @@ static void vm_lock_mapping(struct address_space *mapping) */ if (test_and_set_bit(AS_MM_ALL_LOCKS, &mapping->flags)) BUG(); - spin_lock(&mapping->i_mmap_lock); + spin_lock_nest_lock(&mapping->i_mmap_lock, &mm->mmap_sem); } } @@ -2359,9 +2359,9 @@ int mm_take_all_locks(struct mm_struct *mm) if (signal_pending(current)) goto out_unlock; if (vma->anon_vma) - vm_lock_anon_vma(vma->anon_vma); + vm_lock_anon_vma(mm, vma->anon_vma); if (vma->vm_file && vma->vm_file->f_mapping) - vm_lock_mapping(vma->vm_file->f_mapping); + vm_lock_mapping(mm, vma->vm_file->f_mapping); } ret = 0; -- cgit v1.2.2 From 7cd5a02f54f4c9d16cf7fdffa2122bc73bb09b43 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 11 Aug 2008 09:30:25 +0200 Subject: mm: fix mm_take_all_locks() locking order Lockdep spotted: ======================================================= [ INFO: possible circular locking dependency detected ] 2.6.27-rc1 #270 ------------------------------------------------------- qemu-kvm/2033 is trying to acquire lock: (&inode->i_data.i_mmap_lock){----}, at: [] mm_take_all_locks+0xc2/0xea but task is already holding lock: (&anon_vma->lock){----}, at: [] mm_take_all_locks+0x70/0xea which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&anon_vma->lock){----}: [] __lock_acquire+0x11be/0x14d2 [] lock_acquire+0x5e/0x7a [] _spin_lock+0x3b/0x47 [] vma_adjust+0x200/0x444 [] split_vma+0x12f/0x146 [] mprotect_fixup+0x13c/0x536 [] sys_mprotect+0x1a9/0x21e [] system_call_fastpath+0x16/0x1b [] 0xffffffffffffffff -> #0 (&inode->i_data.i_mmap_lock){----}: [] __lock_acquire+0xedb/0x14d2 [] lock_release_non_nested+0x1c2/0x219 [] lock_release+0x127/0x14a [] _spin_unlock+0x1e/0x50 [] mm_drop_all_locks+0x7f/0xb0 [] do_mmu_notifier_register+0xe2/0x112 [] mmu_notifier_register+0xe/0x10 [] kvm_dev_ioctl+0x11e/0x287 [kvm] [] vfs_ioctl+0x2a/0x78 [] do_vfs_ioctl+0x257/0x274 [] sys_ioctl+0x55/0x78 [] system_call_fastpath+0x16/0x1b [] 0xffffffffffffffff other info that might help us debug this: 5 locks held by qemu-kvm/2033: #0: (&mm->mmap_sem){----}, at: [] do_mmu_notifier_register+0x55/0x112 #1: (mm_all_locks_mutex){--..}, at: [] mm_take_all_locks+0x34/0xea #2: (&anon_vma->lock){----}, at: [] mm_take_all_locks+0x70/0xea #3: (&anon_vma->lock){----}, at: [] mm_take_all_locks+0x70/0xea #4: (&anon_vma->lock){----}, at: [] mm_take_all_locks+0x70/0xea stack backtrace: Pid: 2033, comm: qemu-kvm Not tainted 2.6.27-rc1 #270 Call Trace: [] print_circular_bug_tail+0xb8/0xc3 [] __lock_acquire+0xedb/0x14d2 [] ? add_lock_to_list+0x7e/0xad [] ? mm_take_all_locks+0x70/0xea [] ? mm_take_all_locks+0x70/0xea [] lock_release_non_nested+0x1c2/0x219 [] ? mm_take_all_locks+0xc2/0xea [] ? mm_take_all_locks+0xc2/0xea [] ? trace_hardirqs_on_caller+0x4d/0x115 [] ? mm_drop_all_locks+0x7f/0xb0 [] lock_release+0x127/0x14a [] _spin_unlock+0x1e/0x50 [] mm_drop_all_locks+0x7f/0xb0 [] do_mmu_notifier_register+0xe2/0x112 [] mmu_notifier_register+0xe/0x10 [] kvm_dev_ioctl+0x11e/0x287 [kvm] [] ? file_has_perm+0x83/0x8e [] vfs_ioctl+0x2a/0x78 [] do_vfs_ioctl+0x257/0x274 [] sys_ioctl+0x55/0x78 [] system_call_fastpath+0x16/0x1b Which the locking hierarchy in mm/rmap.c confirms as valid. Fix this by first taking all the mapping->i_mmap_lock instances and then take all anon_vma->lock instances. Signed-off-by: Peter Zijlstra Acked-by: Hugh Dickins Signed-off-by: Ingo Molnar --- mm/mmap.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'mm') diff --git a/mm/mmap.c b/mm/mmap.c index 5d09d08a4120..32a287b631d4 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -2358,11 +2358,17 @@ int mm_take_all_locks(struct mm_struct *mm) for (vma = mm->mmap; vma; vma = vma->vm_next) { if (signal_pending(current)) goto out_unlock; - if (vma->anon_vma) - vm_lock_anon_vma(mm, vma->anon_vma); if (vma->vm_file && vma->vm_file->f_mapping) vm_lock_mapping(mm, vma->vm_file->f_mapping); } + + for (vma = mm->mmap; vma; vma = vma->vm_next) { + if (signal_pending(current)) + goto out_unlock; + if (vma->anon_vma) + vm_lock_anon_vma(mm, vma->anon_vma); + } + ret = 0; out_unlock: -- cgit v1.2.2