* [PATCH 0/1] userfaultfd: shmem: avoid a lockup resulting from corrupted page->flags
@ 2017-01-16 18:04 Andrea Arcangeli
2017-01-16 18:04 ` [PATCH 1/1] " Andrea Arcangeli
0 siblings, 1 reply; 3+ messages in thread
From: Andrea Arcangeli @ 2017-01-16 18:04 UTC (permalink / raw)
To: linux-mm, Andrew Morton
Cc: Michael Rapoport, Dr. David Alan Gilbert, Mike Kravetz,
Pavel Emelyanov, Hillf Danton
Hello,
The userfaultfd_shmem selftest is currently locking up in D state
pretty quickly on current -mm and on the aa.git userfault branch based
on upstream.
Nothing changed on the userfault side of things and I surely let it
run the shmem stress test a while without noticing issues. I initially
thought some other recent change in shmem broke something. After
finishing reviewing all lock/unlock_page I figured it had to be
something else as nobody was holding the lock and surprisingly lockdep
never complained about locking errors.
Something must have changed that gives the lookup more concurrency and
exposed this problem in the page->flags non atomic update that
corrupts the PG_lock bit.
Good thing the userfault selftest is very aggressive at reproducing
any sort of SMP race conditions by starting 3 threads per CPU.
This fix solves the lockup for me. Mike can you verify on your setup
that reproduced this originally?
On a side note: the fix for the false positive SIGBUS is already
included in -mm and it happened to apply clean at the end. I would
suggest to apply that at the top of the userfault queue as it's the
only "fix" in queue for the upstream userfault code (even if not
particularly concerning and unnoticeable in real workloads). This new
fix is not relevant for upstream, but for -mm only.
Thanks.
Andrea Arcangeli (1):
userfaultfd: shmem: avoid a lockup resulting from corrupted
page->flags
mm/shmem.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--
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 1/1] userfaultfd: shmem: avoid a lockup resulting from corrupted page->flags
2017-01-16 18:04 [PATCH 0/1] userfaultfd: shmem: avoid a lockup resulting from corrupted page->flags Andrea Arcangeli
@ 2017-01-16 18:04 ` Andrea Arcangeli
2017-01-19 9:50 ` Hillf Danton
0 siblings, 1 reply; 3+ messages in thread
From: Andrea Arcangeli @ 2017-01-16 18:04 UTC (permalink / raw)
To: linux-mm, Andrew Morton
Cc: Michael Rapoport, Dr. David Alan Gilbert, Mike Kravetz,
Pavel Emelyanov, Hillf Danton
Use the non atomic version of __SetPageUptodate while the page is
still private and not visible to lookup operations. Using the non
atomic version after the page is already visible to lookups is unsafe
as there would be concurrent lock_page operation modifying the
page->flags while it runs.
This solves a lockup in find_lock_entry with the userfaultfd_shmem
selftest.
userfaultfd_shm D14296 691 1 0x00000004
Call Trace:
? __schedule+0x311/0xb60
schedule+0x3d/0x90
schedule_timeout+0x228/0x420
? mark_held_locks+0x71/0x90
? ktime_get+0x134/0x170
? kvm_clock_read+0x25/0x30
? kvm_clock_get_cycles+0x9/0x10
? ktime_get+0xd6/0x170
? __delayacct_blkio_start+0x1f/0x30
io_schedule_timeout+0xa4/0x110
? trace_hardirqs_on+0xd/0x10
__lock_page+0x12d/0x170
? add_to_page_cache_lru+0xe0/0xe0
find_lock_entry+0xa4/0x190
shmem_getpage_gfp+0xb9/0xc30
? alloc_set_pte+0x56e/0x610
? radix_tree_next_chunk+0xf6/0x2d0
shmem_fault+0x70/0x1c0
? filemap_map_pages+0x3bd/0x530
__do_fault+0x21/0x150
handle_mm_fault+0xec9/0x1490
__do_page_fault+0x20d/0x520
trace_do_page_fault+0x61/0x270
do_async_page_fault+0x19/0x80
async_page_fault+0x25/0x30
Reported-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---
mm/shmem.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/mm/shmem.c b/mm/shmem.c
index b1ecd07..873b847 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2247,6 +2247,7 @@ int shmem_mcopy_atomic_pte(struct mm_struct *dst_mm,
VM_BUG_ON(PageLocked(page) || PageSwapBacked(page));
__SetPageLocked(page);
__SetPageSwapBacked(page);
+ __SetPageUptodate(page);
ret = mem_cgroup_try_charge(page, dst_mm, gfp, &memcg, false);
if (ret)
@@ -2271,8 +2272,6 @@ int shmem_mcopy_atomic_pte(struct mm_struct *dst_mm,
if (!pte_none(*dst_pte))
goto out_release_uncharge_unlock;
- __SetPageUptodate(page);
-
lru_cache_add_anon(page);
spin_lock(&info->lock);
--
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 related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] userfaultfd: shmem: avoid a lockup resulting from corrupted page->flags
2017-01-16 18:04 ` [PATCH 1/1] " Andrea Arcangeli
@ 2017-01-19 9:50 ` Hillf Danton
0 siblings, 0 replies; 3+ messages in thread
From: Hillf Danton @ 2017-01-19 9:50 UTC (permalink / raw)
To: 'Andrea Arcangeli', linux-mm, 'Andrew Morton'
Cc: 'Michael Rapoport', 'Dr. David Alan Gilbert',
'Mike Kravetz', 'Pavel Emelyanov'
On Tuesday, January 17, 2017 2:04 AM Andrea Arcangeli wrote:
>
> Use the non atomic version of __SetPageUptodate while the page is
> still private and not visible to lookup operations. Using the non
> atomic version after the page is already visible to lookups is unsafe
> as there would be concurrent lock_page operation modifying the
> page->flags while it runs.
>
> This solves a lockup in find_lock_entry with the userfaultfd_shmem
> selftest.
>
> userfaultfd_shm D14296 691 1 0x00000004
> Call Trace:
> ? __schedule+0x311/0xb60
> schedule+0x3d/0x90
> schedule_timeout+0x228/0x420
> ? mark_held_locks+0x71/0x90
> ? ktime_get+0x134/0x170
> ? kvm_clock_read+0x25/0x30
> ? kvm_clock_get_cycles+0x9/0x10
> ? ktime_get+0xd6/0x170
> ? __delayacct_blkio_start+0x1f/0x30
> io_schedule_timeout+0xa4/0x110
> ? trace_hardirqs_on+0xd/0x10
> __lock_page+0x12d/0x170
> ? add_to_page_cache_lru+0xe0/0xe0
> find_lock_entry+0xa4/0x190
> shmem_getpage_gfp+0xb9/0xc30
> ? alloc_set_pte+0x56e/0x610
> ? radix_tree_next_chunk+0xf6/0x2d0
> shmem_fault+0x70/0x1c0
> ? filemap_map_pages+0x3bd/0x530
> __do_fault+0x21/0x150
> handle_mm_fault+0xec9/0x1490
> __do_page_fault+0x20d/0x520
> trace_do_page_fault+0x61/0x270
> do_async_page_fault+0x19/0x80
> async_page_fault+0x25/0x30
>
> Reported-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
> ---
Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com>
> mm/shmem.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index b1ecd07..873b847 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2247,6 +2247,7 @@ int shmem_mcopy_atomic_pte(struct mm_struct *dst_mm,
> VM_BUG_ON(PageLocked(page) || PageSwapBacked(page));
> __SetPageLocked(page);
> __SetPageSwapBacked(page);
> + __SetPageUptodate(page);
>
> ret = mem_cgroup_try_charge(page, dst_mm, gfp, &memcg, false);
> if (ret)
> @@ -2271,8 +2272,6 @@ int shmem_mcopy_atomic_pte(struct mm_struct *dst_mm,
> if (!pte_none(*dst_pte))
> goto out_release_uncharge_unlock;
>
> - __SetPageUptodate(page);
> -
> lru_cache_add_anon(page);
>
> spin_lock(&info->lock);
--
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:[~2017-01-19 9:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-16 18:04 [PATCH 0/1] userfaultfd: shmem: avoid a lockup resulting from corrupted page->flags Andrea Arcangeli
2017-01-16 18:04 ` [PATCH 1/1] " Andrea Arcangeli
2017-01-19 9:50 ` Hillf Danton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.