[PATCH] mm/mmap.c: Remove redundant vma looping

[PATCH] mm/mmap.c: Remove redundant vma looping

[Chen Gang]

[-- Attachment #1: Type: text/plain, Size: 965 bytes --]

>From 36dbcc145819655682f80efd49e72b01515b4e9a Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5i5j@gmail.com>
Date: Sun, 4 Oct 2015 03:22:41 +0800
Subject: [PATCH] mm/mmap.c: Remove redundant vma looping

vma->vm_file->f_mapping and vma->anon_vma are shared with the same vma
looping, so merge them.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 mm/mmap.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 8393580..f7c1631 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3201,9 +3201,7 @@ int mm_take_all_locks(struct mm_struct *mm)
 			goto out_unlock;
 		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)
-- 
1.9.3


Chen Gang

Open, share, and attitude like air, water, and life which God blessed
 		 	   		  

[-- Attachment #2: 0001-mm-mmap.c-Remove-redundant-vma-looping.patch --]
[-- Type: application/octet-stream, Size: 836 bytes --]

From 36dbcc145819655682f80efd49e72b01515b4e9a Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5i5j@gmail.com>
Date: Sun, 4 Oct 2015 03:22:41 +0800
Subject: [PATCH] mm/mmap.c: Remove redundant vma looping

vma->vm_file->f_mapping and vma->anon_vma are shared with the same vma
looping, so merge them.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 mm/mmap.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 8393580..f7c1631 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3201,9 +3201,7 @@ int mm_take_all_locks(struct mm_struct *mm)
 			goto out_unlock;
 		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)
-- 
1.9.3

Re: [PATCH] mm/mmap.c: Remove redundant vma looping

[Richard Weinberger]

On Sat, Oct 3, 2015 at 9:38 PM, Chen Gang <xili_gchen_5257@hotmail.com> wrote:
> From 36dbcc145819655682f80efd49e72b01515b4e9a Mon Sep 17 00:00:00 2001
> From: Chen Gang <gang.chen.5i5j@gmail.com>
> Date: Sun, 4 Oct 2015 03:22:41 +0800
> Subject: [PATCH] mm/mmap.c: Remove redundant vma looping
>
> vma->vm_file->f_mapping and vma->anon_vma are shared with the same vma
> looping, so merge them.
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  mm/mmap.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 8393580..f7c1631 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -3201,9 +3201,7 @@ int mm_take_all_locks(struct mm_struct *mm)
>                         goto out_unlock;
>                 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)

With that change you're reintroducing an issue.
Please see:
commit 7cd5a02f54f4c9d16cf7fdffa2122bc73bb09b43
Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date:   Mon Aug 11 09:30:25 2008 +0200

    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: [<ffffffff802996cc>]
mm_take_all_locks+0xc2/0xea

    but task is already holding lock:
     (&anon_vma->lock){----}, at: [<ffffffff8029967a>]
mm_take_all_locks+0x70/0xea

    which lock already depends on the new lock.


git blame often explains funky code. :-)

-- 
Thanks,
//richard

--
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] mm/mmap.c: Remove redundant vma looping

[Chen Gang]

On 10/4/15 04:09, Richard Weinberger wrote:
> With that change you're reintroducing an issue.
> Please see:
> commit 7cd5a02f54f4c9d16cf7fdffa2122bc73bb09b43
> Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date:   Mon Aug 11 09:30:25 2008 +0200
> 
>     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: [<ffffffff802996cc>]
> mm_take_all_locks+0xc2/0xea
> 
>     but task is already holding lock:
>      (&anon_vma->lock){----}, at: [<ffffffff8029967a>]
> mm_take_all_locks+0x70/0xea
> 
>     which lock already depends on the new lock.
>

Oh, really. Thanks.

> 
> git blame often explains funky code. :-)
> 

Next, I shall check the git log before make patches, each time. :-)

Theoretically, the lock and unlock need to be symmetric, if we have to
lock f_mapping all firstly, then lock all anon_vma, probably, we also
need to unlock anon_vma all, then unlock all f_mapping.


Thanks.
-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

--
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] mm/mmap.c: Remove redundant vma looping

[Andrea Arcangeli]

Hello Chen,

On Sun, Oct 04, 2015 at 12:55:29PM +0800, Chen Gang wrote:
> Theoretically, the lock and unlock need to be symmetric, if we have to
> lock f_mapping all firstly, then lock all anon_vma, probably, we also
> need to unlock anon_vma all, then unlock all f_mapping.

They don't need to be symmetric because the unlocking order doesn't
matter. To avoid lock inversion deadlocks it is enough to enforce the
lock order.

--
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] mm/mmap.c: Remove redundant vma looping

[Chen Gang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 726 bytes --]

> From: aarcange@redhat.com
>
> Hello Chen,
>
> On Sun, Oct 04, 2015 at 12:55:29PM +0800, Chen Gang wrote:
>> Theoretically, the lock and unlock need to be symmetric, if we have to
>> lock f_mapping all firstly, then lock all anon_vma, probably, we also
>> need to unlock anon_vma all, then unlock all f_mapping.
>
> They don't need to be symmetric because the unlocking order doesn't
> matter. To avoid lock inversion deadlocks it is enough to enforce the
> lock order.

OK, thanks. I shall continue to find another patches. :-)

--
Chen Gang

Open, share, and attitude like air, water, and life which God blessed
 		 	   		  N‹§²æìr¸›zǧu©ž²Æ {\b­†éì¹»\x1c®&Þ–)îÆi¢žØ^n‡r¶‰šŽŠÝ¢j$½§$¢¸\x05¢¹¨­è§~Š'.)îÄÃ,yèm¶ŸÿÃ\f%Š{±šj+ƒðèž×¦j)Z†·Ÿ