Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1230460 > unrolled thread

Re: Multiple potential races on vma->vm_flags

Started byAndrey Konovalov <andreyknvl@google.com>
First post2015-09-22 18:50 +0200
Last post2015-09-25 21:40 +0200
Articles 9 — 5 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: Multiple potential races on vma->vm_flags Andrey Konovalov <andreyknvl@google.com> - 2015-09-22 18:50 +0200
    Re: Multiple potential races on vma->vm_flags Hugh Dickins <hughd@google.com> - 2015-09-22 21:00 +0200
      Re: Multiple potential races on vma->vm_flags Andrey Konovalov <andreyknvl@google.com> - 2015-09-22 21:50 +0200
        Re: Multiple potential races on vma->vm_flags Hugh Dickins <hughd@google.com> - 2015-09-23 03:50 +0200
          Re: Multiple potential races on vma->vm_flags "Kirill A. Shutemov" <kirill@shutemov.name> - 2015-09-23 13:50 +0200
            Re: Multiple potential races on vma->vm_flags Davidlohr Bueso <dave@stgolabs.net> - 2015-09-24 01:00 +0200
          Re: Multiple potential races on vma->vm_flags Andrey Konovalov <andreyknvl@google.com> - 2015-09-23 15:10 +0200
            Re: Multiple potential races on vma->vm_flags Sasha Levin <sasha.levin@oracle.com> - 2015-09-24 02:50 +0200
              Re: Multiple potential races on vma->vm_flags "Kirill A. Shutemov" <kirill@shutemov.name> - 2015-09-25 21:40 +0200

#1230460 — Re: Multiple potential races on vma->vm_flags

FromAndrey Konovalov <andreyknvl@google.com>
Date2015-09-22 18:50 +0200
SubjectRe: Multiple potential races on vma->vm_flags
Message-ID<qbyVs-2z7-15@gated-at.bofh.it>
If anybody comes up with a patch to fix the original issue I easily
can test it, since I'm hitting "BUG: Bad page state" in a second when
fuzzing with KTSAN and Trinity.

On Tue, Sep 15, 2015 at 9:01 PM, Kirill A. Shutemov
<kirill@shutemov.name> wrote:
> On Tue, Sep 15, 2015 at 01:36:45PM -0400, Sasha Levin wrote:
>> On 09/11/2015 09:27 PM, Hugh Dickins wrote:
>> > I'm inclined to echo Vlastimil's comment from earlier in the thread:
>> > sounds like an overkill, unless we find something more serious than this.
>>
>> I've modified my tests to stress the exit path of processes with many vmas,
>
> Could you share the test?
>
>> and hit the following NULL ptr deref (not sure if it's related to the original issue):
>>
>> [1181047.935563] kasan: GPF could be caused by NULL-ptr deref or user memory accessgeneral protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC KASAN
>> [1181047.937223] Modules linked in:
>> [1181047.937772] CPU: 4 PID: 21912 Comm: trinity-c341 Not tainted 4.3.0-rc1-next-20150914-sasha-00043-geddd763-dirty #2554
>> [1181047.939387] task: ffff8804195c8000 ti: ffff880433f00000 task.ti: ffff880433f00000
>> [1181047.940533] RIP: unmap_vmas (mm/memory.c:1337)
>
> Is it "struct mm_struct *mm = vma->vm_mm;"?
>
> --
>  Kirill A. Shutemov
--
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/

[toc] | [next] | [standalone]

#1230790

FromHugh Dickins <hughd@google.com>
Date2015-09-22 21:00 +0200
Message-ID<qbAXi-5tC-79@gated-at.bofh.it>
In reply to#1230460
On Tue, 22 Sep 2015, Andrey Konovalov wrote:
> If anybody comes up with a patch to fix the original issue I easily
> can test it, since I'm hitting "BUG: Bad page state" in a second when
> fuzzing with KTSAN and Trinity.

This "BUG: Bad page state" sounds more serious, but I cannot track down
your report of it: please repost - thanks - though on seeing it, I may
well end up with no ideas.

Hugh
--
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/

[toc] | [prev] | [next] | [standalone]

#1230884

FromAndrey Konovalov <andreyknvl@google.com>
Date2015-09-22 21:50 +0200
Message-ID<qbBJG-6EK-49@gated-at.bofh.it>
In reply to#1230790
On Tue, Sep 22, 2015 at 8:54 PM, Hugh Dickins <hughd@google.com> wrote:
> On Tue, 22 Sep 2015, Andrey Konovalov wrote:
>> If anybody comes up with a patch to fix the original issue I easily
>> can test it, since I'm hitting "BUG: Bad page state" in a second when
>> fuzzing with KTSAN and Trinity.
>
> This "BUG: Bad page state" sounds more serious, but I cannot track down
> your report of it: please repost - thanks - though on seeing it, I may
> well end up with no ideas.

The report is below.

I get it after a few seconds of running Trinity on a kernel with KTSAN
and targeting mlock, munlock and madvise syscalls.
Sasha also observed a very similar crash a while ago
(https://lkml.org/lkml/2014/11/6/1055).
I didn't manage to reproduce this in a kernel build without KTSAN though.
The idea was that data races KTSAN reports might be the explanation of
these crashes.

BUG: Bad page state in process trinity-c15  pfn:281999
page:ffffea000a066640 count:0 mapcount:0 mapping:          (null) index:0xd
flags: 0x20000000028000c(referenced|uptodate|swapbacked|mlocked)
page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set
bad because of flags:
flags: 0x200000(mlocked)
Modules linked in:
CPU: 3 PID: 11190 Comm: trinity-c15 Not tainted 4.2.0-tsan #1295
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffffffff821c3b70 0000000000000000 0000000100004741 ffff8800b857f948
 ffffffff81e9926c 0000000000000003 ffffea000a066640 ffff8800b857f978
 ffffffff811ce045 ffffffff821c3b70 ffffea000a066640 0000000000000001
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff81e9926c>] dump_stack+0x63/0x81 lib/dump_stack.c:50
 [<ffffffff811ce045>] bad_page+0x115/0x1a0 mm/page_alloc.c:409
 [<     inline     >] free_pages_check mm/page_alloc.c:731
 [<ffffffff811cf3b8>] free_pages_prepare+0x2f8/0x330 mm/page_alloc.c:922
 [<ffffffff811d2911>] free_hot_cold_page+0x51/0x2b0 mm/page_alloc.c:1908
 [<ffffffff811d2bcf>] free_hot_cold_page_list+0x5f/0x100
mm/page_alloc.c:1956 (discriminator 3)
 [<ffffffff811dd9c1>] release_pages+0x151/0x300 mm/swap.c:967
 [<ffffffff811de723>] __pagevec_release+0x43/0x60 mm/swap.c:984
 [<     inline     >] pagevec_release include/linux/pagevec.h:69
 [<ffffffff811ef36a>] shmem_undo_range+0x4fa/0x9d0 mm/shmem.c:446
 [<ffffffff811ef86f>] shmem_truncate_range+0x2f/0x60 mm/shmem.c:540
 [<ffffffff811f15d5>] shmem_fallocate+0x555/0x6e0 mm/shmem.c:2086
 [<ffffffff812568d0>] vfs_fallocate+0x1e0/0x310 fs/open.c:303
 [<     inline     >] madvise_remove mm/madvise.c:326
 [<     inline     >] madvise_vma mm/madvise.c:378
 [<     inline     >] SYSC_madvise mm/madvise.c:528
 [<ffffffff81225548>] SyS_madvise+0x378/0x760 mm/madvise.c:459
 [<ffffffff8124ef36>] ? kt_atomic64_store+0x76/0x130 mm/ktsan/sync_atomic.c:161
 [<ffffffff81ea8691>] entry_SYSCALL_64_fastpath+0x31/0x95
arch/x86/entry/entry_64.S:188
Disabling lock debugging due to kernel taint

>
> Hugh
--
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/

[toc] | [prev] | [next] | [standalone]

#1231090

FromHugh Dickins <hughd@google.com>
Date2015-09-23 03:50 +0200
Message-ID<qbHm2-6iQ-1@gated-at.bofh.it>
In reply to#1230884
On Tue, 22 Sep 2015, Andrey Konovalov wrote:
> On Tue, Sep 22, 2015 at 8:54 PM, Hugh Dickins <hughd@google.com> wrote:
> > On Tue, 22 Sep 2015, Andrey Konovalov wrote:
> >> If anybody comes up with a patch to fix the original issue I easily
> >> can test it, since I'm hitting "BUG: Bad page state" in a second when
> >> fuzzing with KTSAN and Trinity.
> >
> > This "BUG: Bad page state" sounds more serious, but I cannot track down
> > your report of it: please repost - thanks - though on seeing it, I may
> > well end up with no ideas.
> 
> The report is below.

Thanks.

> 
> I get it after a few seconds of running Trinity on a kernel with KTSAN
> and targeting mlock, munlock and madvise syscalls.
> Sasha also observed a very similar crash a while ago
> (https://lkml.org/lkml/2014/11/6/1055).
> I didn't manage to reproduce this in a kernel build without KTSAN though.
> The idea was that data races KTSAN reports might be the explanation of
> these crashes.
> 
> BUG: Bad page state in process trinity-c15  pfn:281999
> page:ffffea000a066640 count:0 mapcount:0 mapping:          (null) index:0xd
> flags: 0x20000000028000c(referenced|uptodate|swapbacked|mlocked)
> page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set
> bad because of flags:
> flags: 0x200000(mlocked)
> Modules linked in:
> CPU: 3 PID: 11190 Comm: trinity-c15 Not tainted 4.2.0-tsan #1295
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>  ffffffff821c3b70 0000000000000000 0000000100004741 ffff8800b857f948
>  ffffffff81e9926c 0000000000000003 ffffea000a066640 ffff8800b857f978
>  ffffffff811ce045 ffffffff821c3b70 ffffea000a066640 0000000000000001
> Call Trace:
>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>  [<ffffffff81e9926c>] dump_stack+0x63/0x81 lib/dump_stack.c:50
>  [<ffffffff811ce045>] bad_page+0x115/0x1a0 mm/page_alloc.c:409
>  [<     inline     >] free_pages_check mm/page_alloc.c:731
>  [<ffffffff811cf3b8>] free_pages_prepare+0x2f8/0x330 mm/page_alloc.c:922
>  [<ffffffff811d2911>] free_hot_cold_page+0x51/0x2b0 mm/page_alloc.c:1908
>  [<ffffffff811d2bcf>] free_hot_cold_page_list+0x5f/0x100
> mm/page_alloc.c:1956 (discriminator 3)
>  [<ffffffff811dd9c1>] release_pages+0x151/0x300 mm/swap.c:967
>  [<ffffffff811de723>] __pagevec_release+0x43/0x60 mm/swap.c:984
>  [<     inline     >] pagevec_release include/linux/pagevec.h:69
>  [<ffffffff811ef36a>] shmem_undo_range+0x4fa/0x9d0 mm/shmem.c:446
>  [<ffffffff811ef86f>] shmem_truncate_range+0x2f/0x60 mm/shmem.c:540
>  [<ffffffff811f15d5>] shmem_fallocate+0x555/0x6e0 mm/shmem.c:2086
>  [<ffffffff812568d0>] vfs_fallocate+0x1e0/0x310 fs/open.c:303
>  [<     inline     >] madvise_remove mm/madvise.c:326
>  [<     inline     >] madvise_vma mm/madvise.c:378
>  [<     inline     >] SYSC_madvise mm/madvise.c:528
>  [<ffffffff81225548>] SyS_madvise+0x378/0x760 mm/madvise.c:459
>  [<ffffffff8124ef36>] ? kt_atomic64_store+0x76/0x130 mm/ktsan/sync_atomic.c:161
>  [<ffffffff81ea8691>] entry_SYSCALL_64_fastpath+0x31/0x95
> arch/x86/entry/entry_64.S:188
> Disabling lock debugging due to kernel taint

This is totally untested, and one of you may quickly prove me wrong;
but I went in to fix your "Bad page state (mlocked)" by holding pte
lock across the down_read_trylock of mmap_sem in try_to_unmap_one(),
then couldn't see why it would need mmap_sem at all, given how mlock
and munlock first assert intention by setting or clearing VM_LOCKED
in vm_flags, then work their way up the vma, taking pte locks.

Calling mlock_vma_page() under pte lock may look suspicious
at first: but what it does is similar to clear_page_mlock(),
which we regularly call under pte lock from page_remove_rmap().

I'd rather wait to hear whether this appears to work in practice,
and whether you agree that it should work in theory, before writing
the proper description.  I'd love to lose that down_read_trylock.

You mention how Sasha hit the "Bad page state (mlocked)" back in
November: that was one of the reasons we reverted Davidlohr's
i_mmap_lock_read to i_mmap_lock_write in unmap_mapping_range(),
without understanding why it was needed.  Yes, it would lock out
a concurrent try_to_unmap(), whose setting of PageMlocked was not
sufficiently serialized by the down_read_trylock of mmap_sem.

But I don't remember the other reasons for that revert (and
haven't looked very hard as yet): anyone else remember?

Not-yet-Signed-off-by: Hugh Dickins <hughd@google.com>
---

 mm/rmap.c |   32 +++++++-------------------------
 1 file changed, 7 insertions(+), 25 deletions(-)

--- 4.3-rc2/mm/rmap.c	2015-09-12 18:30:20.857039763 -0700
+++ linux/mm/rmap.c	2015-09-22 17:47:43.489096676 -0700
@@ -1314,9 +1314,12 @@ static int try_to_unmap_one(struct page
 	 * skipped over this mm) then we should reactivate it.
 	 */
 	if (!(flags & TTU_IGNORE_MLOCK)) {
-		if (vma->vm_flags & VM_LOCKED)
-			goto out_mlock;
-
+		if (vma->vm_flags & VM_LOCKED) {
+	 		/* Holding pte lock, we do *not* need mmap_sem here */
+			mlock_vma_page(page);
+			ret = SWAP_MLOCK;
+			goto out_unmap;
+		}
 		if (flags & TTU_MUNLOCK)
 			goto out_unmap;
 	}
@@ -1419,31 +1422,10 @@ static int try_to_unmap_one(struct page
 
 out_unmap:
 	pte_unmap_unlock(pte, ptl);
-	if (ret != SWAP_FAIL && !(flags & TTU_MUNLOCK))
+	if (ret != SWAP_FAIL && ret != SWAP_MLOCK && !(flags & TTU_MUNLOCK))
 		mmu_notifier_invalidate_page(mm, address);
 out:
 	return ret;
-
-out_mlock:
-	pte_unmap_unlock(pte, ptl);
-
-
-	/*
-	 * We need mmap_sem locking, Otherwise VM_LOCKED check makes
-	 * unstable result and race. Plus, We can't wait here because
-	 * we now hold anon_vma->rwsem or mapping->i_mmap_rwsem.
-	 * if trylock failed, the page remain in evictable lru and later
-	 * vmscan could retry to move the page to unevictable lru if the
-	 * page is actually mlocked.
-	 */
-	if (down_read_trylock(&vma->vm_mm->mmap_sem)) {
-		if (vma->vm_flags & VM_LOCKED) {
-			mlock_vma_page(page);
-			ret = SWAP_MLOCK;
-		}
-		up_read(&vma->vm_mm->mmap_sem);
-	}
-	return ret;
 }
 
 bool is_vma_temporary_stack(struct vm_area_struct *vma)
--
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/

[toc] | [prev] | [next] | [standalone]

#1231383

From"Kirill A. Shutemov" <kirill@shutemov.name>
Date2015-09-23 13:50 +0200
Message-ID<qbQIH-33j-43@gated-at.bofh.it>
In reply to#1231090
On Tue, Sep 22, 2015 at 06:39:52PM -0700, Hugh Dickins wrote:
> On Tue, 22 Sep 2015, Andrey Konovalov wrote:
> > On Tue, Sep 22, 2015 at 8:54 PM, Hugh Dickins <hughd@google.com> wrote:
> > > On Tue, 22 Sep 2015, Andrey Konovalov wrote:
> > >> If anybody comes up with a patch to fix the original issue I easily
> > >> can test it, since I'm hitting "BUG: Bad page state" in a second when
> > >> fuzzing with KTSAN and Trinity.
> > >
> > > This "BUG: Bad page state" sounds more serious, but I cannot track down
> > > your report of it: please repost - thanks - though on seeing it, I may
> > > well end up with no ideas.
> > 
> > The report is below.
> 
> Thanks.
> 
> > 
> > I get it after a few seconds of running Trinity on a kernel with KTSAN
> > and targeting mlock, munlock and madvise syscalls.
> > Sasha also observed a very similar crash a while ago
> > (https://lkml.org/lkml/2014/11/6/1055).
> > I didn't manage to reproduce this in a kernel build without KTSAN though.
> > The idea was that data races KTSAN reports might be the explanation of
> > these crashes.
> > 
> > BUG: Bad page state in process trinity-c15  pfn:281999
> > page:ffffea000a066640 count:0 mapcount:0 mapping:          (null) index:0xd
> > flags: 0x20000000028000c(referenced|uptodate|swapbacked|mlocked)
> > page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set
> > bad because of flags:
> > flags: 0x200000(mlocked)
> > Modules linked in:
> > CPU: 3 PID: 11190 Comm: trinity-c15 Not tainted 4.2.0-tsan #1295
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> >  ffffffff821c3b70 0000000000000000 0000000100004741 ffff8800b857f948
> >  ffffffff81e9926c 0000000000000003 ffffea000a066640 ffff8800b857f978
> >  ffffffff811ce045 ffffffff821c3b70 ffffea000a066640 0000000000000001
> > Call Trace:
> >  [<     inline     >] __dump_stack lib/dump_stack.c:15
> >  [<ffffffff81e9926c>] dump_stack+0x63/0x81 lib/dump_stack.c:50
> >  [<ffffffff811ce045>] bad_page+0x115/0x1a0 mm/page_alloc.c:409
> >  [<     inline     >] free_pages_check mm/page_alloc.c:731
> >  [<ffffffff811cf3b8>] free_pages_prepare+0x2f8/0x330 mm/page_alloc.c:922
> >  [<ffffffff811d2911>] free_hot_cold_page+0x51/0x2b0 mm/page_alloc.c:1908
> >  [<ffffffff811d2bcf>] free_hot_cold_page_list+0x5f/0x100
> > mm/page_alloc.c:1956 (discriminator 3)
> >  [<ffffffff811dd9c1>] release_pages+0x151/0x300 mm/swap.c:967
> >  [<ffffffff811de723>] __pagevec_release+0x43/0x60 mm/swap.c:984
> >  [<     inline     >] pagevec_release include/linux/pagevec.h:69
> >  [<ffffffff811ef36a>] shmem_undo_range+0x4fa/0x9d0 mm/shmem.c:446
> >  [<ffffffff811ef86f>] shmem_truncate_range+0x2f/0x60 mm/shmem.c:540
> >  [<ffffffff811f15d5>] shmem_fallocate+0x555/0x6e0 mm/shmem.c:2086
> >  [<ffffffff812568d0>] vfs_fallocate+0x1e0/0x310 fs/open.c:303
> >  [<     inline     >] madvise_remove mm/madvise.c:326
> >  [<     inline     >] madvise_vma mm/madvise.c:378
> >  [<     inline     >] SYSC_madvise mm/madvise.c:528
> >  [<ffffffff81225548>] SyS_madvise+0x378/0x760 mm/madvise.c:459
> >  [<ffffffff8124ef36>] ? kt_atomic64_store+0x76/0x130 mm/ktsan/sync_atomic.c:161
> >  [<ffffffff81ea8691>] entry_SYSCALL_64_fastpath+0x31/0x95
> > arch/x86/entry/entry_64.S:188
> > Disabling lock debugging due to kernel taint
> 
> This is totally untested, and one of you may quickly prove me wrong;
> but I went in to fix your "Bad page state (mlocked)" by holding pte
> lock across the down_read_trylock of mmap_sem in try_to_unmap_one(),
> then couldn't see why it would need mmap_sem at all, given how mlock
> and munlock first assert intention by setting or clearing VM_LOCKED
> in vm_flags, then work their way up the vma, taking pte locks.
> 
> Calling mlock_vma_page() under pte lock may look suspicious
> at first: but what it does is similar to clear_page_mlock(),
> which we regularly call under pte lock from page_remove_rmap().

Indeed. Looks fishy. But probably will work.

That was not obvious for me what makes clearing VM_LOCKED visible in
try_to_unmap_one() without mmap_sem.

After looking some more it seems we would rely on page_check_address()
in try_to_unmap_one() having acquire semantics and follow_page_mask() in
munlock_vma_pages_range() having release semantics.

But I would prefer to have something more explicit.

That's mess.

> 
> I'd rather wait to hear whether this appears to work in practice,
> and whether you agree that it should work in theory, before writing
> the proper description.  I'd love to lose that down_read_trylock.
> 
> You mention how Sasha hit the "Bad page state (mlocked)" back in
> November: that was one of the reasons we reverted Davidlohr's
> i_mmap_lock_read to i_mmap_lock_write in unmap_mapping_range(),
> without understanding why it was needed.  Yes, it would lock out
> a concurrent try_to_unmap(), whose setting of PageMlocked was not
> sufficiently serialized by the down_read_trylock of mmap_sem.
> 
> But I don't remember the other reasons for that revert (and
> haven't looked very hard as yet): anyone else remember?

I hoped Davidlohr will come back with something after the revert, but it
never happend. I think the reverted patch was responsible for most of
scalability boost from rwsem for i_mmap_lock...

-- 
 Kirill A. Shutemov
--
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/

[toc] | [prev] | [next] | [standalone]

#1231776

FromDavidlohr Bueso <dave@stgolabs.net>
Date2015-09-24 01:00 +0200
Message-ID<qc1b3-1s3-1@gated-at.bofh.it>
In reply to#1231383
On Wed, 23 Sep 2015, Kirill A. Shutemov wrote:

>On Tue, Sep 22, 2015 at 06:39:52PM -0700, Hugh Dickins wrote:
>>[...]
>> I'd rather wait to hear whether this appears to work in practice,
>> and whether you agree that it should work in theory, before writing
>> the proper description.  I'd love to lose that down_read_trylock.
>>
>> You mention how Sasha hit the "Bad page state (mlocked)" back in
>> November: that was one of the reasons we reverted Davidlohr's
>> i_mmap_lock_read to i_mmap_lock_write in unmap_mapping_range(),
>> without understanding why it was needed.  Yes, it would lock out
>> a concurrent try_to_unmap(), whose setting of PageMlocked was not
>> sufficiently serialized by the down_read_trylock of mmap_sem.
>>
>> But I don't remember the other reasons for that revert (and
>> haven't looked very hard as yet): anyone else remember?

Yeah, I don't think this was ever resolved, but ultimately the patch
got reverted[1] because it exposed issues in the form of bad pages
(shmem, vmsplice) and corrupted vm_flags while in untrack_pfn() causing,
for example, vm_file to dissapear.

>I hoped Davidlohr will come back with something after the revert, but it
>never happend. I think the reverted patch was responsible for most of
>scalability boost from rwsem for i_mmap_lock...

Actually no, the change that got reverted was something we got in very
last minute, just because it made sense and had the blessing of some
key people. The main winner of the series was migration (rmap), which
later Hugh addressed more specifically for unmapped pages:

https://lkml.org/lkml/2014/11/30/349

So I really didn't care about the reverted patch, and therefore was never
on my radar.

[1] https://lkml.org/lkml/2014/12/22/375

Thanks,
Davidlohr
--
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/

[toc] | [prev] | [next] | [standalone]

#1231428

FromAndrey Konovalov <andreyknvl@google.com>
Date2015-09-23 15:10 +0200
Message-ID<qbRY6-51S-11@gated-at.bofh.it>
In reply to#1231090
On Wed, Sep 23, 2015 at 3:39 AM, Hugh Dickins <hughd@google.com> wrote:
> This is totally untested, and one of you may quickly prove me wrong;
> but I went in to fix your "Bad page state (mlocked)" by holding pte
> lock across the down_read_trylock of mmap_sem in try_to_unmap_one(),
> then couldn't see why it would need mmap_sem at all, given how mlock
> and munlock first assert intention by setting or clearing VM_LOCKED
> in vm_flags, then work their way up the vma, taking pte locks.
>
> Calling mlock_vma_page() under pte lock may look suspicious
> at first: but what it does is similar to clear_page_mlock(),
> which we regularly call under pte lock from page_remove_rmap().
>
> I'd rather wait to hear whether this appears to work in practice,
> and whether you agree that it should work in theory, before writing
> the proper description.  I'd love to lose that down_read_trylock.

No, unfortunately it doesn't work, I still see "Bad page state (mlocked)".

It seems that your patch doesn't fix the race from the report below, since pte
lock is not taken when 'vma->vm_flags &= ~VM_LOCKED;' (mlock.c:425)
is being executed. (Line numbers are from kernel with your patch applied.)

===
ThreadSanitizer: data-race in munlock_vma_pages_range

Write at 0xffff880282a93290 of size 8 by thread 2546 on CPU 2:
 [<ffffffff81211009>] munlock_vma_pages_range+0x59/0x3e0 mm/mlock.c:425
 [<     inline     >] munlock_vma_pages_all mm/internal.h:252
 [<ffffffff81215d03>] exit_mmap+0x163/0x190 mm/mmap.c:2824
 [<ffffffff81085635>] mmput+0x65/0x190 kernel/fork.c:708
 [<     inline     >] exit_mm kernel/exit.c:437
 [<ffffffff8108c2a7>] do_exit+0x457/0x1400 kernel/exit.c:733
 [<ffffffff8108ef3f>] do_group_exit+0x7f/0x140 kernel/exit.c:874
 [<ffffffff810a03a5>] get_signal+0x375/0xa70 kernel/signal.c:2353
 [<ffffffff8100619c>] do_signal+0x2c/0xad0 arch/x86/kernel/signal.c:704
 [<ffffffff81006cbd>] do_notify_resume+0x7d/0x80 arch/x86/kernel/signal.c:749
 [<ffffffff81ea87a4>] int_signal+0x12/0x17 arch/x86/entry/entry_64.S:329

Previous read at 0xffff880282a93290 of size 8 by thread 2545 on CPU 1:
 [<ffffffff8121bc1a>] try_to_unmap_one+0x6a/0x450 mm/rmap.c:1208
 [<     inline     >] rmap_walk_file mm/rmap.c:1522
 [<ffffffff8121d1a7>] rmap_walk+0x147/0x450 mm/rmap.c:1541
 [<ffffffff8121d962>] try_to_munlock+0xa2/0xc0 mm/rmap.c:1405
 [<ffffffff81210640>] __munlock_isolated_page+0x30/0x60 mm/mlock.c:129
 [<ffffffff81210af6>] __munlock_pagevec+0x236/0x3f0 mm/mlock.c:331
 [<ffffffff81211330>] munlock_vma_pages_range+0x380/0x3e0 mm/mlock.c:476
 [<     inline     >] munlock_vma_pages_all mm/internal.h:252
 [<ffffffff81215d03>] exit_mmap+0x163/0x190 mm/mmap.c:2824
 [<ffffffff81085635>] mmput+0x65/0x190 kernel/fork.c:708
 [<     inline     >] exit_mm kernel/exit.c:437
 [<ffffffff8108c2a7>] do_exit+0x457/0x1400 kernel/exit.c:733
 [<ffffffff8108ef3f>] do_group_exit+0x7f/0x140 kernel/exit.c:874
 [<ffffffff810a03a5>] get_signal+0x375/0xa70 kernel/signal.c:2353
 [<ffffffff8100619c>] do_signal+0x2c/0xad0 arch/x86/kernel/signal.c:704
 [<ffffffff81006cbd>] do_notify_resume+0x7d/0x80 arch/x86/kernel/signal.c:749
 [<ffffffff81ea87a4>] int_signal+0x12/0x17 arch/x86/entry/entry_64.S:329
===
--
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/

[toc] | [prev] | [next] | [standalone]

#1231797

FromSasha Levin <sasha.levin@oracle.com>
Date2015-09-24 02:50 +0200
Message-ID<qc2Tw-3XT-3@gated-at.bofh.it>
In reply to#1231428
On 09/23/2015 09:08 AM, Andrey Konovalov wrote:
> On Wed, Sep 23, 2015 at 3:39 AM, Hugh Dickins <hughd@google.com> wrote:
>> > This is totally untested, and one of you may quickly prove me wrong;
>> > but I went in to fix your "Bad page state (mlocked)" by holding pte
>> > lock across the down_read_trylock of mmap_sem in try_to_unmap_one(),
>> > then couldn't see why it would need mmap_sem at all, given how mlock
>> > and munlock first assert intention by setting or clearing VM_LOCKED
>> > in vm_flags, then work their way up the vma, taking pte locks.
>> >
>> > Calling mlock_vma_page() under pte lock may look suspicious
>> > at first: but what it does is similar to clear_page_mlock(),
>> > which we regularly call under pte lock from page_remove_rmap().
>> >
>> > I'd rather wait to hear whether this appears to work in practice,
>> > and whether you agree that it should work in theory, before writing
>> > the proper description.  I'd love to lose that down_read_trylock.
> No, unfortunately it doesn't work, I still see "Bad page state (mlocked)".
> 
> It seems that your patch doesn't fix the race from the report below, since pte
> lock is not taken when 'vma->vm_flags &= ~VM_LOCKED;' (mlock.c:425)
> is being executed. (Line numbers are from kernel with your patch applied.)

I've fired up my HZ_10000 patch, and this seems to be a real race that is
somewhat easy to reproduce under those conditions.

Here's a fresh backtrace from my VMs:

[1935109.882343] BUG: Bad page state in process trinity-subchil  pfn:3ca200
[1935109.884000] page:ffffea000f288000 count:0 mapcount:0 mapping:          (null) index:0x1e00 compound_mapcount: 0
[1935109.885772] flags: 0x22fffff80144008(uptodate|head|swapbacked|mlocked)
[1935109.887174] page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set
[1935109.888197] bad because of flags:
[1935109.888759] flags: 0x100000(mlocked)
[1935109.889525] Modules linked in:
[1935109.890165] CPU: 8 PID: 2615 Comm: trinity-subchil Not tainted 4.3.0-rc2-next-20150923-sasha-00079-gec04207-dirty #2569
[1935109.891876]  1ffffffff6445448 00000000e5dca494 ffff8803f7657708 ffffffffa70402da
[1935109.893504]  ffffea000f288000 ffff8803f7657738 ffffffffa56e522b 022fffff80144008
[1935109.894947]  ffffea000f288020 ffffea000f288000 00000000ffffffff ffff8803f76577a8
[1935109.896413] Call Trace:
[1935109.899102]  [<ffffffffa70402da>] dump_stack+0x4e/0x84
[1935109.899821]  [<ffffffffa56e522b>] bad_page+0x17b/0x210
[1935109.900469]  [<ffffffffa56e85a8>] free_pages_prepare+0xb48/0x1110
[1935109.902127]  [<ffffffffa56ee0d1>] __free_pages_ok+0x21/0x260
[1935109.904435]  [<ffffffffa56ee373>] free_compound_page+0x63/0x80
[1935109.905614]  [<ffffffffa581b51e>] free_transhuge_page+0x6e/0x80
[1935109.906752]  [<ffffffffa5709f76>] __put_compound_page+0x76/0xa0
[1935109.907884]  [<ffffffffa570a475>] release_pages+0x4d5/0x9f0
[1935109.913027]  [<ffffffffa5769bea>] tlb_flush_mmu_free+0x8a/0x120
[1935109.913957]  [<ffffffffa576f993>] unmap_page_range+0xe73/0x1460
[1935109.915737]  [<ffffffffa57700a6>] unmap_single_vma+0x126/0x2f0
[1935109.916646]  [<ffffffffa577270d>] unmap_vmas+0xdd/0x190
[1935109.917454]  [<ffffffffa5790361>] exit_mmap+0x221/0x430
[1935109.921176]  [<ffffffffa5366da1>] mmput+0xb1/0x240
[1935109.921919]  [<ffffffffa537b3b2>] do_exit+0x732/0x27c0
[1935109.928561]  [<ffffffffa537d599>] do_group_exit+0xf9/0x300
[1935109.929786]  [<ffffffffa537d7bd>] SyS_exit_group+0x1d/0x20
[1935109.930617]  [<ffffffffaf59fbf6>] entry_SYSCALL_64_fastpath+0x16/0x7a


Thanks,
Sasha
--
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/

[toc] | [prev] | [next] | [standalone]

#1233042

From"Kirill A. Shutemov" <kirill@shutemov.name>
Date2015-09-25 21:40 +0200
Message-ID<qcH0C-2u4-33@gated-at.bofh.it>
In reply to#1231797
On Wed, Sep 23, 2015 at 08:42:26PM -0400, Sasha Levin wrote:
> On 09/23/2015 09:08 AM, Andrey Konovalov wrote:
> > On Wed, Sep 23, 2015 at 3:39 AM, Hugh Dickins <hughd@google.com> wrote:
> >> > This is totally untested, and one of you may quickly prove me wrong;
> >> > but I went in to fix your "Bad page state (mlocked)" by holding pte
> >> > lock across the down_read_trylock of mmap_sem in try_to_unmap_one(),
> >> > then couldn't see why it would need mmap_sem at all, given how mlock
> >> > and munlock first assert intention by setting or clearing VM_LOCKED
> >> > in vm_flags, then work their way up the vma, taking pte locks.
> >> >
> >> > Calling mlock_vma_page() under pte lock may look suspicious
> >> > at first: but what it does is similar to clear_page_mlock(),
> >> > which we regularly call under pte lock from page_remove_rmap().
> >> >
> >> > I'd rather wait to hear whether this appears to work in practice,
> >> > and whether you agree that it should work in theory, before writing
> >> > the proper description.  I'd love to lose that down_read_trylock.
> > No, unfortunately it doesn't work, I still see "Bad page state (mlocked)".
> > 
> > It seems that your patch doesn't fix the race from the report below, since pte
> > lock is not taken when 'vma->vm_flags &= ~VM_LOCKED;' (mlock.c:425)
> > is being executed. (Line numbers are from kernel with your patch applied.)
> 
> I've fired up my HZ_10000 patch,

Can we make HZ_10000 thing into upstream? Under KERNEL_DEBUG, or
something?

> and this seems to be a real race that is
> somewhat easy to reproduce under those conditions.
> 
> Here's a fresh backtrace from my VMs:
> 
> [1935109.882343] BUG: Bad page state in process trinity-subchil  pfn:3ca200
> [1935109.884000] page:ffffea000f288000 count:0 mapcount:0 mapping:          (null) index:0x1e00 compound_mapcount: 0
> [1935109.885772] flags: 0x22fffff80144008(uptodate|head|swapbacked|mlocked)
> [1935109.887174] page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set
> [1935109.888197] bad because of flags:
> [1935109.888759] flags: 0x100000(mlocked)
> [1935109.889525] Modules linked in:
> [1935109.890165] CPU: 8 PID: 2615 Comm: trinity-subchil Not tainted 4.3.0-rc2-next-20150923-sasha-00079-gec04207-dirty #2569
> [1935109.891876]  1ffffffff6445448 00000000e5dca494 ffff8803f7657708 ffffffffa70402da
> [1935109.893504]  ffffea000f288000 ffff8803f7657738 ffffffffa56e522b 022fffff80144008
> [1935109.894947]  ffffea000f288020 ffffea000f288000 00000000ffffffff ffff8803f76577a8
> [1935109.896413] Call Trace:
> [1935109.899102]  [<ffffffffa70402da>] dump_stack+0x4e/0x84
> [1935109.899821]  [<ffffffffa56e522b>] bad_page+0x17b/0x210
> [1935109.900469]  [<ffffffffa56e85a8>] free_pages_prepare+0xb48/0x1110
> [1935109.902127]  [<ffffffffa56ee0d1>] __free_pages_ok+0x21/0x260
> [1935109.904435]  [<ffffffffa56ee373>] free_compound_page+0x63/0x80
> [1935109.905614]  [<ffffffffa581b51e>] free_transhuge_page+0x6e/0x80
> [1935109.906752]  [<ffffffffa5709f76>] __put_compound_page+0x76/0xa0
> [1935109.907884]  [<ffffffffa570a475>] release_pages+0x4d5/0x9f0
> [1935109.913027]  [<ffffffffa5769bea>] tlb_flush_mmu_free+0x8a/0x120
> [1935109.913957]  [<ffffffffa576f993>] unmap_page_range+0xe73/0x1460
> [1935109.915737]  [<ffffffffa57700a6>] unmap_single_vma+0x126/0x2f0
> [1935109.916646]  [<ffffffffa577270d>] unmap_vmas+0xdd/0x190
> [1935109.917454]  [<ffffffffa5790361>] exit_mmap+0x221/0x430
> [1935109.921176]  [<ffffffffa5366da1>] mmput+0xb1/0x240
> [1935109.921919]  [<ffffffffa537b3b2>] do_exit+0x732/0x27c0
> [1935109.928561]  [<ffffffffa537d599>] do_group_exit+0xf9/0x300
> [1935109.929786]  [<ffffffffa537d7bd>] SyS_exit_group+0x1d/0x20
> [1935109.930617]  [<ffffffffaf59fbf6>] entry_SYSCALL_64_fastpath+0x16/0x7a

Would it make any difference if you'll add mmap_sem protection in
exit_mmap?

-- 
 Kirill A. Shutemov
--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web