3.8-rc2: lockdep is complaining about mm_take_all_locks()

3.8-rc2: lockdep is complaining about mm_take_all_locks()

[Jiri Kosina]

This is almost certainly because

commit 5a505085f043e8380f83610f79642853c051e2f1
Author: Ingo Molnar <mingo@kernel.org>
Date:   Sun Dec 2 19:56:46 2012 +0000

    mm/rmap: Convert the struct anon_vma::mutex to an rwsem

did this to mm_take_all_locks():

	-               mutex_lock_nest_lock(&anon_vma->root->mutex, &mm->mmap_sem);
	+               down_write(&anon_vma->root->rwsem);

killing the lockdep annotation that has been there since 

commit 454ed842d55740160334efc9ad56cfef54ed37bc
Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date:   Mon Aug 11 09:30:25 2008 +0200

    lockdep: annotate mm_take_all_locks()

The locking is obviously correct due to mmap_sem being held throughout the 
whole operation, but I am not completely sure how to annotate this 
properly for lockdep in down_write() case though. Ingo, please?



 =============================================
 [ INFO: possible recursive locking detected ]
 3.8.0-rc2-00036-g5f73896 #171 Not tainted
 ---------------------------------------------
 qemu-kvm/2315 is trying to acquire lock:
  (&anon_vma->rwsem){+.+...}, at: [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0
 
 but task is already holding lock:
  (&anon_vma->rwsem){+.+...}, at: [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0
 
 other info that might help us debug this:
  Possible unsafe locking scenario:
 
        CPU0
        ----
   lock(&anon_vma->rwsem);
   lock(&anon_vma->rwsem);
 
  *** DEADLOCK ***
 
  May be due to missing lock nesting notation
 
 4 locks held by qemu-kvm/2315:
  #0:  (&mm->mmap_sem){++++++}, at: [<ffffffff81177f1c>] do_mmu_notifier_register+0xfc/0x170
  #1:  (mm_all_locks_mutex){+.+...}, at: [<ffffffff8115d436>] mm_take_all_locks+0x36/0x1b0
  #2:  (&mapping->i_mmap_mutex){+.+...}, at: [<ffffffff8115d4c9>] mm_take_all_locks+0xc9/0x1b0
  #3:  (&anon_vma->rwsem){+.+...}, at: [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0
 
 stack backtrace:
 Pid: 2315, comm: qemu-kvm Not tainted 3.8.0-rc2-00036-g5f73896 #171
 Call Trace:
  [<ffffffff810afea2>] print_deadlock_bug+0xf2/0x100
  [<ffffffff810b1a76>] validate_chain+0x4f6/0x720
  [<ffffffff810b1ff9>] __lock_acquire+0x359/0x580
  [<ffffffff810b0e7d>] ? trace_hardirqs_on_caller+0x12d/0x1b0
  [<ffffffff810b2341>] lock_acquire+0x121/0x190
  [<ffffffff8115d549>] ? mm_take_all_locks+0x149/0x1b0
  [<ffffffff815a12bf>] down_write+0x3f/0x70
  [<ffffffff8115d549>] ? mm_take_all_locks+0x149/0x1b0
  [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0
  [<ffffffff81177e88>] do_mmu_notifier_register+0x68/0x170
  [<ffffffff81177fae>] mmu_notifier_register+0xe/0x10
  [<ffffffffa04bd6ab>] kvm_create_vm+0x22b/0x330 [kvm]
  [<ffffffffa04bd8a8>] kvm_dev_ioctl+0xf8/0x1a0 [kvm]
  [<ffffffff811a45bd>] do_vfs_ioctl+0x9d/0x350
  [<ffffffff815ad215>] ? sysret_check+0x22/0x5d
  [<ffffffff811a4901>] sys_ioctl+0x91/0xb0
  [<ffffffff815ad1e9>] system_call_fastpath+0x16/0x1b

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: 3.8-rc2: lockdep is complaining about mm_take_all_locks()

[Jiri Kosina]

On Fri, 4 Jan 2013, Jiri Kosina wrote:

> This is almost certainly because
> 
> commit 5a505085f043e8380f83610f79642853c051e2f1
> Author: Ingo Molnar <mingo@kernel.org>
> Date:   Sun Dec 2 19:56:46 2012 +0000
> 
>     mm/rmap: Convert the struct anon_vma::mutex to an rwsem
> 
> did this to mm_take_all_locks():
> 
> 	-               mutex_lock_nest_lock(&anon_vma->root->mutex, &mm->mmap_sem);
> 	+               down_write(&anon_vma->root->rwsem);
> 
> killing the lockdep annotation that has been there since 
> 
> commit 454ed842d55740160334efc9ad56cfef54ed37bc
> Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date:   Mon Aug 11 09:30:25 2008 +0200
> 
>     lockdep: annotate mm_take_all_locks()
> 
> The locking is obviously correct due to mmap_sem being held throughout the 
> whole operation, but I am not completely sure how to annotate this 
> properly for lockdep in down_write() case though. Ingo, please?

OK, I think the only solution is to introduce down_read_nest_lock(). I 
will prepare a patch.

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] lockdep, rwsem: provide down_write_nest_lock()

[Jiri Kosina]

down_write_nest_lock() provides means to annotate locking scenario where 
an outter lock is guaranteed to serialize the order nested locks are being 
acquired.

This is an analogy to already existing mutex_lock_nest_lock() and
spin_lock_nest_lock().

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 include/linux/lockdep.h |    3 +++
 include/linux/rwsem.h   |    9 +++++++++
 kernel/rwsem.c          |   10 ++++++++++
 3 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 00e4637..2bca44b 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -524,14 +524,17 @@ static inline void print_irqtrace_events(struct task_struct *curr)
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 # ifdef CONFIG_PROVE_LOCKING
 #  define rwsem_acquire(l, s, t, i)		lock_acquire(l, s, t, 0, 2, NULL, i)
+#  define rwsem_acquire_nest(l, s, t, n, i)	lock_acquire(l, s, t, 0, 2, n, i)
 #  define rwsem_acquire_read(l, s, t, i)	lock_acquire(l, s, t, 1, 2, NULL, i)
 # else
 #  define rwsem_acquire(l, s, t, i)		lock_acquire(l, s, t, 0, 1, NULL, i)
+#  define rwsem_acquire_nest(l, s, t, n, i)	lock_acquire(l, s, t, 0, 1, n, i)
 #  define rwsem_acquire_read(l, s, t, i)	lock_acquire(l, s, t, 1, 1, NULL, i)
 # endif
 # define rwsem_release(l, n, i)			lock_release(l, n, i)
 #else
 # define rwsem_acquire(l, s, t, i)		do { } while (0)
+# define rwsem_acquire_nest(l, s, t, n, i)	do { } while (0)
 # define rwsem_acquire_read(l, s, t, i)		do { } while (0)
 # define rwsem_release(l, n, i)			do { } while (0)
 #endif
diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h
index 54bd7cd..413cc11 100644
--- a/include/linux/rwsem.h
+++ b/include/linux/rwsem.h
@@ -125,8 +125,17 @@ extern void downgrade_write(struct rw_semaphore *sem);
  */
 extern void down_read_nested(struct rw_semaphore *sem, int subclass);
 extern void down_write_nested(struct rw_semaphore *sem, int subclass);
+extern void _down_write_nest_lock(struct rw_semaphore *sem, struct lockdep_map *nest_lock);
+
+# define down_write_nest_lock(sem, nest_lock)			\
+do {								\
+	typecheck(struct lockdep_map *, &(nest_lock)->dep_map);	\
+	_down_write_nest_lock(sem, &(nest_lock)->dep_map);	\
+} while (0);
+
 #else
 # define down_read_nested(sem, subclass)		down_read(sem)
+# define down_write_nest_lock(sem, nest_lock)	down_read(sem)
 # define down_write_nested(sem, subclass)	down_write(sem)
 #endif
 
diff --git a/kernel/rwsem.c b/kernel/rwsem.c
index 6850f53..b3c6c3f 100644
--- a/kernel/rwsem.c
+++ b/kernel/rwsem.c
@@ -116,6 +116,16 @@ void down_read_nested(struct rw_semaphore *sem, int subclass)
 
 EXPORT_SYMBOL(down_read_nested);
 
+void _down_write_nest_lock(struct rw_semaphore *sem, struct lockdep_map *nest)
+{
+	might_sleep();
+	rwsem_acquire_nest(&sem->dep_map, 0, 0, nest, _RET_IP_);
+
+	LOCK_CONTENDED(sem, __down_write_trylock, __down_write);
+}
+
+EXPORT_SYMBOL(_down_write_nest_lock);
+
 void down_write_nested(struct rw_semaphore *sem, int subclass)
 {
 	might_sleep();
-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] mm: mmap: annotate vm_lock_anon_vma locking properly for lockdep

[Jiri Kosina]

Commit 5a505085f04 ("mm/rmap: Convert the struct anon_vma::mutex to an 
rwsem") turned anon_vma mutex to rwsem.

However, the properly annotated nested locking in mm_take_all_locks() has 
been converted from

	mutex_lock_nest_lock(&anon_vma->root->mutex, &mm->mmap_sem);

to

	down_write(&anon_vma->root->rwsem);

which is incomplete, and causes the false positive report from lockdep below.

Annotate the fact that mmap_sem is used as an outter lock to serialize taking
of all the anon_vma rwsems at once no matter the order, using the
down_write_nest_lock() primitive.

This patch fixes this lockdep report:

 =============================================
 [ INFO: possible recursive locking detected ]
 3.8.0-rc2-00036-g5f73896 #171 Not tainted
 ---------------------------------------------
 qemu-kvm/2315 is trying to acquire lock:
  (&anon_vma->rwsem){+.+...}, at: [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0

 but task is already holding lock:
  (&anon_vma->rwsem){+.+...}, at: [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0

 other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(&anon_vma->rwsem);
   lock(&anon_vma->rwsem);

  *** DEADLOCK ***

  May be due to missing lock nesting notation

 4 locks held by qemu-kvm/2315:
  #0:  (&mm->mmap_sem){++++++}, at: [<ffffffff81177f1c>] do_mmu_notifier_register+0xfc/0x170
  #1:  (mm_all_locks_mutex){+.+...}, at: [<ffffffff8115d436>] mm_take_all_locks+0x36/0x1b0
  #2:  (&mapping->i_mmap_mutex){+.+...}, at: [<ffffffff8115d4c9>] mm_take_all_locks+0xc9/0x1b0
  #3:  (&anon_vma->rwsem){+.+...}, at: [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0

 stack backtrace:
 Pid: 2315, comm: qemu-kvm Not tainted 3.8.0-rc2-00036-g5f73896 #171
 Call Trace:
  [<ffffffff810afea2>] print_deadlock_bug+0xf2/0x100
  [<ffffffff810b1a76>] validate_chain+0x4f6/0x720
  [<ffffffff810b1ff9>] __lock_acquire+0x359/0x580
  [<ffffffff810b0e7d>] ? trace_hardirqs_on_caller+0x12d/0x1b0
  [<ffffffff810b2341>] lock_acquire+0x121/0x190
  [<ffffffff8115d549>] ? mm_take_all_locks+0x149/0x1b0
  [<ffffffff815a12bf>] down_write+0x3f/0x70
  [<ffffffff8115d549>] ? mm_take_all_locks+0x149/0x1b0
  [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0
  [<ffffffff81177e88>] do_mmu_notifier_register+0x68/0x170
  [<ffffffff81177fae>] mmu_notifier_register+0xe/0x10
  [<ffffffffa04bd6ab>] kvm_create_vm+0x22b/0x330 [kvm]
  [<ffffffffa04bd8a8>] kvm_dev_ioctl+0xf8/0x1a0 [kvm]
  [<ffffffff811a45bd>] do_vfs_ioctl+0x9d/0x350
  [<ffffffff815ad215>] ? sysret_check+0x22/0x5d
  [<ffffffff811a4901>] sys_ioctl+0x91/0xb0
  [<ffffffff815ad1e9>] system_call_fastpath+0x16/0x1b

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 mm/mmap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index f54b235..35730ee 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2886,7 +2886,7 @@ static void vm_lock_anon_vma(struct mm_struct *mm, struct anon_vma *anon_vma)
 		 * The LSB of head.next can't change from under us
 		 * because we hold the mm_all_locks_mutex.
 		 */
-		down_write(&anon_vma->root->rwsem);
+		down_write_nest_lock(&anon_vma->root->rwsem, &mm->mmap_sem);
 		/*
 		 * We can safely modify head.next after taking the
 		 * anon_vma->root->rwsem. If some other vma in this mm shares
-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] mm: mmap: annotate vm_lock_anon_vma locking properly for lockdep

[Borislav Petkov]

Ok, I see this still on -rc3, also with kvm.

Will run your patches and verify them.

Thanks.

On Sat, Jan 05, 2013 at 01:35:45AM +0100, Jiri Kosina wrote:
> Commit 5a505085f04 ("mm/rmap: Convert the struct anon_vma::mutex to an 
> rwsem") turned anon_vma mutex to rwsem.
> 
> However, the properly annotated nested locking in mm_take_all_locks() has 
> been converted from
> 
> 	mutex_lock_nest_lock(&anon_vma->root->mutex, &mm->mmap_sem);
> 
> to
> 
> 	down_write(&anon_vma->root->rwsem);
> 
> which is incomplete, and causes the false positive report from lockdep below.
> 
> Annotate the fact that mmap_sem is used as an outter lock to serialize taking
> of all the anon_vma rwsems at once no matter the order, using the
> down_write_nest_lock() primitive.
> 
> This patch fixes this lockdep report:
> 
>  =============================================
>  [ INFO: possible recursive locking detected ]
>  3.8.0-rc2-00036-g5f73896 #171 Not tainted
>  ---------------------------------------------
>  qemu-kvm/2315 is trying to acquire lock:
>   (&anon_vma->rwsem){+.+...}, at: [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0
> 
>  but task is already holding lock:
>   (&anon_vma->rwsem){+.+...}, at: [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0
> 
>  other info that might help us debug this:
>   Possible unsafe locking scenario:
> 
>         CPU0
>         ----
>    lock(&anon_vma->rwsem);
>    lock(&anon_vma->rwsem);
> 
>   *** DEADLOCK ***
> 
>   May be due to missing lock nesting notation
> 
>  4 locks held by qemu-kvm/2315:
>   #0:  (&mm->mmap_sem){++++++}, at: [<ffffffff81177f1c>] do_mmu_notifier_register+0xfc/0x170
>   #1:  (mm_all_locks_mutex){+.+...}, at: [<ffffffff8115d436>] mm_take_all_locks+0x36/0x1b0
>   #2:  (&mapping->i_mmap_mutex){+.+...}, at: [<ffffffff8115d4c9>] mm_take_all_locks+0xc9/0x1b0
>   #3:  (&anon_vma->rwsem){+.+...}, at: [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0
> 
>  stack backtrace:
>  Pid: 2315, comm: qemu-kvm Not tainted 3.8.0-rc2-00036-g5f73896 #171
>  Call Trace:
>   [<ffffffff810afea2>] print_deadlock_bug+0xf2/0x100
>   [<ffffffff810b1a76>] validate_chain+0x4f6/0x720
>   [<ffffffff810b1ff9>] __lock_acquire+0x359/0x580
>   [<ffffffff810b0e7d>] ? trace_hardirqs_on_caller+0x12d/0x1b0
>   [<ffffffff810b2341>] lock_acquire+0x121/0x190
>   [<ffffffff8115d549>] ? mm_take_all_locks+0x149/0x1b0
>   [<ffffffff815a12bf>] down_write+0x3f/0x70
>   [<ffffffff8115d549>] ? mm_take_all_locks+0x149/0x1b0
>   [<ffffffff8115d549>] mm_take_all_locks+0x149/0x1b0
>   [<ffffffff81177e88>] do_mmu_notifier_register+0x68/0x170
>   [<ffffffff81177fae>] mmu_notifier_register+0xe/0x10
>   [<ffffffffa04bd6ab>] kvm_create_vm+0x22b/0x330 [kvm]
>   [<ffffffffa04bd8a8>] kvm_dev_ioctl+0xf8/0x1a0 [kvm]
>   [<ffffffff811a45bd>] do_vfs_ioctl+0x9d/0x350
>   [<ffffffff815ad215>] ? sysret_check+0x22/0x5d
>   [<ffffffff811a4901>] sys_ioctl+0x91/0xb0
>   [<ffffffff815ad1e9>] system_call_fastpath+0x16/0x1b
> 
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
>  mm/mmap.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index f54b235..35730ee 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2886,7 +2886,7 @@ static void vm_lock_anon_vma(struct mm_struct *mm, struct anon_vma *anon_vma)
>  		 * The LSB of head.next can't change from under us
>  		 * because we hold the mm_all_locks_mutex.
>  		 */
> -		down_write(&anon_vma->root->rwsem);
> +		down_write_nest_lock(&anon_vma->root->rwsem, &mm->mmap_sem);
>  		/*
>  		 * We can safely modify head.next after taking the
>  		 * anon_vma->root->rwsem. If some other vma in this mm shares
> -- 
> Jiri Kosina
> SUSE Labs
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] mm: mmap: annotate vm_lock_anon_vma locking properly for lockdep

[Sedat Dilek]

Hi Jiri,

see my other reply to "[PATCH 1/2] lockdep, rwsem: provide
down_write_nest_lock()" in [1]...

...and feel free to add...

     Tested-by: Sedat Dilek <sedat.dilek@gmail.com>

Thanks!

Regards,
- Sedat -

[1] http://marc.info/?l=linux-mm&m=135738411200967&w=2

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>