linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] mm: mmap: annotate vm_lock_anon_vma locking properly for lockdep
@ 2013-01-05 11:11 Sedat Dilek
  0 siblings, 0 replies; 3+ messages in thread
From: Sedat Dilek @ 2013-01-05 11:11 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-mm, LKML

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>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/2] mm: mmap: annotate vm_lock_anon_vma locking properly for lockdep
  2013-01-05  0:35   ` [PATCH 2/2] mm: mmap: annotate vm_lock_anon_vma locking properly for lockdep Jiri Kosina
@ 2013-01-11 17:00     ` Borislav Petkov
  0 siblings, 0 replies; 3+ messages in thread
From: Borislav Petkov @ 2013-01-11 17:00 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Rik van Riel, Ingo Molnar, Peter Zijlstra, Andrew Morton,
	linux-kernel, linux-mm

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>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] mm: mmap: annotate vm_lock_anon_vma locking properly for lockdep
  2013-01-05  0:34 ` [PATCH 1/2] lockdep, rwsem: provide down_write_nest_lock() Jiri Kosina
@ 2013-01-05  0:35   ` Jiri Kosina
  2013-01-11 17:00     ` Borislav Petkov
  0 siblings, 1 reply; 3+ messages in thread
From: Jiri Kosina @ 2013-01-05  0:35 UTC (permalink / raw)
  To: Rik van Riel, Ingo Molnar, Peter Zijlstra, Andrew Morton
  Cc: linux-kernel, linux-mm

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>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-01-11 17:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-05 11:11 [PATCH 2/2] mm: mmap: annotate vm_lock_anon_vma locking properly for lockdep Sedat Dilek
  -- strict thread matches above, loose matches on Subject: below --
2013-01-04 12:49 3.8-rc2: lockdep is complaining about mm_take_all_locks() Jiri Kosina
2013-01-05  0:34 ` [PATCH 1/2] lockdep, rwsem: provide down_write_nest_lock() Jiri Kosina
2013-01-05  0:35   ` [PATCH 2/2] mm: mmap: annotate vm_lock_anon_vma locking properly for lockdep Jiri Kosina
2013-01-11 17:00     ` Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox