All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] mm: fail prefaulting if page table allocation fails
       [not found] <01fa01d1e94c$4be09210$e3a1b630$@alibaba-inc.com>
@ 2016-07-29  8:00 ` Hillf Danton
  2016-07-29  8:10   ` Kirill A. Shutemov
  0 siblings, 1 reply; 9+ messages in thread
From: Hillf Danton @ 2016-07-29  8:00 UTC (permalink / raw)
  To: 'Vegard Nossum'; +Cc: Kirill A. Shutemov, Andrew Morton, linux-mm

> 
> I ran into this:
> 
>     BUG: sleeping function called from invalid context at mm/page_alloc.c:3784
>     in_atomic(): 0, irqs_disabled(): 0, pid: 1434, name: trinity-c1
>     2 locks held by trinity-c1/1434:
>      #0:  (&mm->mmap_sem){......}, at: [<ffffffff810ce31e>] __do_page_fault+0x1ce/0x8f0
>      #1:  (rcu_read_lock){......}, at: [<ffffffff81378f86>] filemap_map_pages+0xd6/0xdd0
> 
>     CPU: 0 PID: 1434 Comm: trinity-c1 Not tainted 4.7.0+ #58
>     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
>      ffff8800b662f698 ffff8800b662f548 ffffffff81d6d001 ffffffff83a61100
>      ffff8800b662f620 ffff8800b662f610 ffffffff81373fd1 0000000041b58ab3
>      ffffffff8406ca21 ffffffff81373e4c 0000000041b58ab3 ffffffff00000008
>     Call Trace:
>      [<ffffffff81d6d001>] dump_stack+0x65/0x84
>      [<ffffffff81373fd1>] panic+0x185/0x2dd
>      [<ffffffff8118e38c>] ___might_sleep+0x51c/0x600
>      [<ffffffff8118e500>] __might_sleep+0x90/0x1a0
>      [<ffffffff81392761>] __alloc_pages_nodemask+0x5b1/0x2160
>      [<ffffffff814665ac>] alloc_pages_current+0xcc/0x370
>      [<ffffffff810d95b2>] pte_alloc_one+0x12/0x90
>      [<ffffffff814053cd>] __pte_alloc+0x1d/0x200
>      [<ffffffff8140be4e>] alloc_set_pte+0xe3e/0x14a0
>      [<ffffffff813792db>] filemap_map_pages+0x42b/0xdd0
>      [<ffffffff8140e0d5>] handle_mm_fault+0x17d5/0x28b0
>      [<ffffffff810ce460>] __do_page_fault+0x310/0x8f0
>      [<ffffffff810cec7d>] trace_do_page_fault+0x18d/0x310
>      [<ffffffff810c2177>] do_async_page_fault+0x27/0xa0
>      [<ffffffff8389e258>] async_page_fault+0x28/0x30
> 
> The important bits from the above is that filemap_map_pages() is calling
> into the page allocator while holding rcu_read_lock (sleeping is not
> allowed inside RCU read-side critical sections).
> 
> According to Kirill Shutemov, the prefaulting code in do_fault_around()
> is supposed to take care of this, but missing error handling means that
> the allocation failure can go unnoticed.
> 
Well it is fixed at this particular call site, thanks. 

On the other hand IIUC in alloc_set_pte() there is no acquiring of ptl if 
fe->pte is valid, so race still sits there. 

Would you please address both?

Hillf
> We don't need to return VM_FAULT_OOM (or any other error) here, since we
> can just let the normal fault path try again.
> 
> Fixes: 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  mm/memory.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 4425b60..0400483 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3133,6 +3133,8 @@ static int do_fault_around(struct fault_env *fe, pgoff_t start_pgoff)
> 
>  	if (pmd_none(*fe->pmd)) {
>  		fe->prealloc_pte = pte_alloc_one(fe->vma->vm_mm, fe->address);
> +		if (!fe->prealloc_pte)
> +			goto out;
>  		smp_wmb(); /* See comment in __pte_alloc() */
>  	}
> 
> --
> 1.9.1
> 
> 


--
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] 9+ messages in thread

* Re: [PATCH] mm: fail prefaulting if page table allocation fails
  2016-07-29  8:00 ` [PATCH] mm: fail prefaulting if page table allocation fails Hillf Danton
@ 2016-07-29  8:10   ` Kirill A. Shutemov
  2016-07-29  8:23     ` Hillf Danton
  0 siblings, 1 reply; 9+ messages in thread
From: Kirill A. Shutemov @ 2016-07-29  8:10 UTC (permalink / raw)
  To: Hillf Danton; +Cc: 'Vegard Nossum', Andrew Morton, linux-mm

On Fri, Jul 29, 2016 at 04:00:37PM +0800, Hillf Danton wrote:
> > 
> > I ran into this:
> > 
> >     BUG: sleeping function called from invalid context at mm/page_alloc.c:3784
> >     in_atomic(): 0, irqs_disabled(): 0, pid: 1434, name: trinity-c1
> >     2 locks held by trinity-c1/1434:
> >      #0:  (&mm->mmap_sem){......}, at: [<ffffffff810ce31e>] __do_page_fault+0x1ce/0x8f0
> >      #1:  (rcu_read_lock){......}, at: [<ffffffff81378f86>] filemap_map_pages+0xd6/0xdd0
> > 
> >     CPU: 0 PID: 1434 Comm: trinity-c1 Not tainted 4.7.0+ #58
> >     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
> >      ffff8800b662f698 ffff8800b662f548 ffffffff81d6d001 ffffffff83a61100
> >      ffff8800b662f620 ffff8800b662f610 ffffffff81373fd1 0000000041b58ab3
> >      ffffffff8406ca21 ffffffff81373e4c 0000000041b58ab3 ffffffff00000008
> >     Call Trace:
> >      [<ffffffff81d6d001>] dump_stack+0x65/0x84
> >      [<ffffffff81373fd1>] panic+0x185/0x2dd
> >      [<ffffffff8118e38c>] ___might_sleep+0x51c/0x600
> >      [<ffffffff8118e500>] __might_sleep+0x90/0x1a0
> >      [<ffffffff81392761>] __alloc_pages_nodemask+0x5b1/0x2160
> >      [<ffffffff814665ac>] alloc_pages_current+0xcc/0x370
> >      [<ffffffff810d95b2>] pte_alloc_one+0x12/0x90
> >      [<ffffffff814053cd>] __pte_alloc+0x1d/0x200
> >      [<ffffffff8140be4e>] alloc_set_pte+0xe3e/0x14a0
> >      [<ffffffff813792db>] filemap_map_pages+0x42b/0xdd0
> >      [<ffffffff8140e0d5>] handle_mm_fault+0x17d5/0x28b0
> >      [<ffffffff810ce460>] __do_page_fault+0x310/0x8f0
> >      [<ffffffff810cec7d>] trace_do_page_fault+0x18d/0x310
> >      [<ffffffff810c2177>] do_async_page_fault+0x27/0xa0
> >      [<ffffffff8389e258>] async_page_fault+0x28/0x30
> > 
> > The important bits from the above is that filemap_map_pages() is calling
> > into the page allocator while holding rcu_read_lock (sleeping is not
> > allowed inside RCU read-side critical sections).
> > 
> > According to Kirill Shutemov, the prefaulting code in do_fault_around()
> > is supposed to take care of this, but missing error handling means that
> > the allocation failure can go unnoticed.
> > 
> Well it is fixed at this particular call site, thanks. 
> 
> On the other hand IIUC in alloc_set_pte() there is no acquiring of ptl if 
> fe->pte is valid, so race still sits there. 

Could you elaborate on where you see the race? I didn't get it.

> Would you please address both?

-- 
 Kirill A. Shutemov

--
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] 9+ messages in thread

* Re: [PATCH] mm: fail prefaulting if page table allocation fails
  2016-07-29  8:10   ` Kirill A. Shutemov
@ 2016-07-29  8:23     ` Hillf Danton
  2016-07-29  8:31       ` Kirill A. Shutemov
  0 siblings, 1 reply; 9+ messages in thread
From: Hillf Danton @ 2016-07-29  8:23 UTC (permalink / raw)
  To: 'Kirill A. Shutemov'
  Cc: 'Vegard Nossum', 'Andrew Morton', linux-mm

> 
> On Fri, Jul 29, 2016 at 04:00:37PM +0800, Hillf Danton wrote:
> > >
> > > I ran into this:
> > >
> > >     BUG: sleeping function called from invalid context at mm/page_alloc.c:3784
> > >     in_atomic(): 0, irqs_disabled(): 0, pid: 1434, name: trinity-c1
> > >     2 locks held by trinity-c1/1434:
> > >      #0:  (&mm->mmap_sem){......}, at: [<ffffffff810ce31e>] __do_page_fault+0x1ce/0x8f0
> > >      #1:  (rcu_read_lock){......}, at: [<ffffffff81378f86>] filemap_map_pages+0xd6/0xdd0
> > >
> > >     CPU: 0 PID: 1434 Comm: trinity-c1 Not tainted 4.7.0+ #58
> > >     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
> > >      ffff8800b662f698 ffff8800b662f548 ffffffff81d6d001 ffffffff83a61100
> > >      ffff8800b662f620 ffff8800b662f610 ffffffff81373fd1 0000000041b58ab3
> > >      ffffffff8406ca21 ffffffff81373e4c 0000000041b58ab3 ffffffff00000008
> > >     Call Trace:
> > >      [<ffffffff81d6d001>] dump_stack+0x65/0x84
> > >      [<ffffffff81373fd1>] panic+0x185/0x2dd
> > >      [<ffffffff8118e38c>] ___might_sleep+0x51c/0x600
> > >      [<ffffffff8118e500>] __might_sleep+0x90/0x1a0
> > >      [<ffffffff81392761>] __alloc_pages_nodemask+0x5b1/0x2160
> > >      [<ffffffff814665ac>] alloc_pages_current+0xcc/0x370
> > >      [<ffffffff810d95b2>] pte_alloc_one+0x12/0x90
> > >      [<ffffffff814053cd>] __pte_alloc+0x1d/0x200
> > >      [<ffffffff8140be4e>] alloc_set_pte+0xe3e/0x14a0
> > >      [<ffffffff813792db>] filemap_map_pages+0x42b/0xdd0
> > >      [<ffffffff8140e0d5>] handle_mm_fault+0x17d5/0x28b0
> > >      [<ffffffff810ce460>] __do_page_fault+0x310/0x8f0
> > >      [<ffffffff810cec7d>] trace_do_page_fault+0x18d/0x310
> > >      [<ffffffff810c2177>] do_async_page_fault+0x27/0xa0
> > >      [<ffffffff8389e258>] async_page_fault+0x28/0x30
> > >
> > > The important bits from the above is that filemap_map_pages() is calling
> > > into the page allocator while holding rcu_read_lock (sleeping is not
> > > allowed inside RCU read-side critical sections).
> > >
> > > According to Kirill Shutemov, the prefaulting code in do_fault_around()
> > > is supposed to take care of this, but missing error handling means that
> > > the allocation failure can go unnoticed.
> > >
> > Well it is fixed at this particular call site, thanks.
> >
> > On the other hand IIUC in alloc_set_pte() there is no acquiring of ptl if
> > fe->pte is valid, so race still sits there.
> 
> Could you elaborate on where you see the race? I didn't get it.
> 
In filemap_map_pages()
	CPU0					CPU1
	trylock_page at offset_A		trylock_page at offset_A
						goto offset_A+1
	if (!fe->pte) {
		alloc pte
		map pte
		lock pte
	}
	handle offset_A with ptl held		handle offset_A+1 without acquiring ptl

Hillf

--
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] 9+ messages in thread

* Re: [PATCH] mm: fail prefaulting if page table allocation fails
  2016-07-29  8:23     ` Hillf Danton
@ 2016-07-29  8:31       ` Kirill A. Shutemov
  2016-07-29  8:40         ` Hillf Danton
  0 siblings, 1 reply; 9+ messages in thread
From: Kirill A. Shutemov @ 2016-07-29  8:31 UTC (permalink / raw)
  To: Hillf Danton
  Cc: 'Kirill A. Shutemov', 'Vegard Nossum',
	'Andrew Morton',
	linux-mm

On Fri, Jul 29, 2016 at 04:23:33PM +0800, Hillf Danton wrote:
> > 
> > On Fri, Jul 29, 2016 at 04:00:37PM +0800, Hillf Danton wrote:
> > > >
> > > > I ran into this:
> > > >
> > > >     BUG: sleeping function called from invalid context at mm/page_alloc.c:3784
> > > >     in_atomic(): 0, irqs_disabled(): 0, pid: 1434, name: trinity-c1
> > > >     2 locks held by trinity-c1/1434:
> > > >      #0:  (&mm->mmap_sem){......}, at: [<ffffffff810ce31e>] __do_page_fault+0x1ce/0x8f0
> > > >      #1:  (rcu_read_lock){......}, at: [<ffffffff81378f86>] filemap_map_pages+0xd6/0xdd0
> > > >
> > > >     CPU: 0 PID: 1434 Comm: trinity-c1 Not tainted 4.7.0+ #58
> > > >     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
> > > >      ffff8800b662f698 ffff8800b662f548 ffffffff81d6d001 ffffffff83a61100
> > > >      ffff8800b662f620 ffff8800b662f610 ffffffff81373fd1 0000000041b58ab3
> > > >      ffffffff8406ca21 ffffffff81373e4c 0000000041b58ab3 ffffffff00000008
> > > >     Call Trace:
> > > >      [<ffffffff81d6d001>] dump_stack+0x65/0x84
> > > >      [<ffffffff81373fd1>] panic+0x185/0x2dd
> > > >      [<ffffffff8118e38c>] ___might_sleep+0x51c/0x600
> > > >      [<ffffffff8118e500>] __might_sleep+0x90/0x1a0
> > > >      [<ffffffff81392761>] __alloc_pages_nodemask+0x5b1/0x2160
> > > >      [<ffffffff814665ac>] alloc_pages_current+0xcc/0x370
> > > >      [<ffffffff810d95b2>] pte_alloc_one+0x12/0x90
> > > >      [<ffffffff814053cd>] __pte_alloc+0x1d/0x200
> > > >      [<ffffffff8140be4e>] alloc_set_pte+0xe3e/0x14a0
> > > >      [<ffffffff813792db>] filemap_map_pages+0x42b/0xdd0
> > > >      [<ffffffff8140e0d5>] handle_mm_fault+0x17d5/0x28b0
> > > >      [<ffffffff810ce460>] __do_page_fault+0x310/0x8f0
> > > >      [<ffffffff810cec7d>] trace_do_page_fault+0x18d/0x310
> > > >      [<ffffffff810c2177>] do_async_page_fault+0x27/0xa0
> > > >      [<ffffffff8389e258>] async_page_fault+0x28/0x30
> > > >
> > > > The important bits from the above is that filemap_map_pages() is calling
> > > > into the page allocator while holding rcu_read_lock (sleeping is not
> > > > allowed inside RCU read-side critical sections).
> > > >
> > > > According to Kirill Shutemov, the prefaulting code in do_fault_around()
> > > > is supposed to take care of this, but missing error handling means that
> > > > the allocation failure can go unnoticed.
> > > >
> > > Well it is fixed at this particular call site, thanks.
> > >
> > > On the other hand IIUC in alloc_set_pte() there is no acquiring of ptl if
> > > fe->pte is valid, so race still sits there.
> > 
> > Could you elaborate on where you see the race? I didn't get it.
> > 
> In filemap_map_pages()
> 	CPU0					CPU1
> 	trylock_page at offset_A		trylock_page at offset_A
> 						goto offset_A+1
> 	if (!fe->pte) {
> 		alloc pte
> 		map pte
> 		lock pte
> 	}
> 	handle offset_A with ptl held		handle offset_A+1 without acquiring ptl

I still don't see where's the problem.

On the seond iteration (for offset_A+1), CPU1 would go into
alloc_set_pte() and as its fe->pte is NULL pte_alloc_one_map() would map
and lock the pte table allocated by CPU0.

-- 
 Kirill A. Shutemov

--
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] 9+ messages in thread

* RE: [PATCH] mm: fail prefaulting if page table allocation fails
  2016-07-29  8:31       ` Kirill A. Shutemov
@ 2016-07-29  8:40         ` Hillf Danton
  0 siblings, 0 replies; 9+ messages in thread
From: Hillf Danton @ 2016-07-29  8:40 UTC (permalink / raw)
  To: 'Kirill A. Shutemov'
  Cc: 'Kirill A. Shutemov', 'Vegard Nossum',
	'Andrew Morton',
	linux-mm

> > In filemap_map_pages()
> > 	CPU0					CPU1
> > 	trylock_page at offset_A		trylock_page at offset_A
> > 						goto offset_A+1
> > 	if (!fe->pte) {
> > 		alloc pte
> > 		map pte
> > 		lock pte
> > 	}
> > 	handle offset_A with ptl held		handle offset_A+1 without acquiring ptl
> 
> I still don't see where's the problem.
> 
> On the seond iteration (for offset_A+1), CPU1 would go into
> alloc_set_pte() and as its fe->pte is NULL pte_alloc_one_map() would map
> and lock the pte table allocated by CPU0.
> 
Ah, you are right. I missed the 2nd fe!

thanks
Hillf

--
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] 9+ messages in thread

* Re: [PATCH] mm: fail prefaulting if page table allocation fails
  2016-07-28 12:15 ` Vegard Nossum
@ 2016-07-28 12:21   ` Kirill A. Shutemov
  -1 siblings, 0 replies; 9+ messages in thread
From: Kirill A. Shutemov @ 2016-07-28 12:21 UTC (permalink / raw)
  To: Vegard Nossum; +Cc: linux-mm, akpm, linux-kernel, Kirill A. Shutemov

On Thu, Jul 28, 2016 at 02:15:07PM +0200, Vegard Nossum wrote:
> I ran into this:
> 
>     BUG: sleeping function called from invalid context at mm/page_alloc.c:3784
>     in_atomic(): 0, irqs_disabled(): 0, pid: 1434, name: trinity-c1
>     2 locks held by trinity-c1/1434:
>      #0:  (&mm->mmap_sem){......}, at: [<ffffffff810ce31e>] __do_page_fault+0x1ce/0x8f0
>      #1:  (rcu_read_lock){......}, at: [<ffffffff81378f86>] filemap_map_pages+0xd6/0xdd0
> 
>     CPU: 0 PID: 1434 Comm: trinity-c1 Not tainted 4.7.0+ #58
>     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
>      ffff8800b662f698 ffff8800b662f548 ffffffff81d6d001 ffffffff83a61100
>      ffff8800b662f620 ffff8800b662f610 ffffffff81373fd1 0000000041b58ab3
>      ffffffff8406ca21 ffffffff81373e4c 0000000041b58ab3 ffffffff00000008
>     Call Trace:
>      [<ffffffff81d6d001>] dump_stack+0x65/0x84
>      [<ffffffff81373fd1>] panic+0x185/0x2dd
>      [<ffffffff8118e38c>] ___might_sleep+0x51c/0x600
>      [<ffffffff8118e500>] __might_sleep+0x90/0x1a0
>      [<ffffffff81392761>] __alloc_pages_nodemask+0x5b1/0x2160
>      [<ffffffff814665ac>] alloc_pages_current+0xcc/0x370
>      [<ffffffff810d95b2>] pte_alloc_one+0x12/0x90
>      [<ffffffff814053cd>] __pte_alloc+0x1d/0x200
>      [<ffffffff8140be4e>] alloc_set_pte+0xe3e/0x14a0
>      [<ffffffff813792db>] filemap_map_pages+0x42b/0xdd0
>      [<ffffffff8140e0d5>] handle_mm_fault+0x17d5/0x28b0
>      [<ffffffff810ce460>] __do_page_fault+0x310/0x8f0
>      [<ffffffff810cec7d>] trace_do_page_fault+0x18d/0x310
>      [<ffffffff810c2177>] do_async_page_fault+0x27/0xa0
>      [<ffffffff8389e258>] async_page_fault+0x28/0x30
> 
> The important bits from the above is that filemap_map_pages() is calling
> into the page allocator while holding rcu_read_lock (sleeping is not
> allowed inside RCU read-side critical sections).
> 
> According to Kirill Shutemov, the prefaulting code in do_fault_around()
> is supposed to take care of this, but missing error handling means that
> the allocation failure can go unnoticed.
> 
> We don't need to return VM_FAULT_OOM (or any other error) here, since we
> can just let the normal fault path try again.
> 
> Fixes: 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Worth noticing that it's failure of order-0 allocation and unlikely to
ever happen under normal workloads without fault injection for page
allocator.

> ---
>  mm/memory.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 4425b60..0400483 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3133,6 +3133,8 @@ static int do_fault_around(struct fault_env *fe, pgoff_t start_pgoff)
>  
>  	if (pmd_none(*fe->pmd)) {
>  		fe->prealloc_pte = pte_alloc_one(fe->vma->vm_mm, fe->address);
> +		if (!fe->prealloc_pte)
> +			goto out;
>  		smp_wmb(); /* See comment in __pte_alloc() */
>  	}
>  
> -- 
> 1.9.1
> 
> --
> 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>

-- 
 Kirill A. Shutemov

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

* Re: [PATCH] mm: fail prefaulting if page table allocation fails
@ 2016-07-28 12:21   ` Kirill A. Shutemov
  0 siblings, 0 replies; 9+ messages in thread
From: Kirill A. Shutemov @ 2016-07-28 12:21 UTC (permalink / raw)
  To: Vegard Nossum; +Cc: linux-mm, akpm, linux-kernel, Kirill A. Shutemov

On Thu, Jul 28, 2016 at 02:15:07PM +0200, Vegard Nossum wrote:
> I ran into this:
> 
>     BUG: sleeping function called from invalid context at mm/page_alloc.c:3784
>     in_atomic(): 0, irqs_disabled(): 0, pid: 1434, name: trinity-c1
>     2 locks held by trinity-c1/1434:
>      #0:  (&mm->mmap_sem){......}, at: [<ffffffff810ce31e>] __do_page_fault+0x1ce/0x8f0
>      #1:  (rcu_read_lock){......}, at: [<ffffffff81378f86>] filemap_map_pages+0xd6/0xdd0
> 
>     CPU: 0 PID: 1434 Comm: trinity-c1 Not tainted 4.7.0+ #58
>     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
>      ffff8800b662f698 ffff8800b662f548 ffffffff81d6d001 ffffffff83a61100
>      ffff8800b662f620 ffff8800b662f610 ffffffff81373fd1 0000000041b58ab3
>      ffffffff8406ca21 ffffffff81373e4c 0000000041b58ab3 ffffffff00000008
>     Call Trace:
>      [<ffffffff81d6d001>] dump_stack+0x65/0x84
>      [<ffffffff81373fd1>] panic+0x185/0x2dd
>      [<ffffffff8118e38c>] ___might_sleep+0x51c/0x600
>      [<ffffffff8118e500>] __might_sleep+0x90/0x1a0
>      [<ffffffff81392761>] __alloc_pages_nodemask+0x5b1/0x2160
>      [<ffffffff814665ac>] alloc_pages_current+0xcc/0x370
>      [<ffffffff810d95b2>] pte_alloc_one+0x12/0x90
>      [<ffffffff814053cd>] __pte_alloc+0x1d/0x200
>      [<ffffffff8140be4e>] alloc_set_pte+0xe3e/0x14a0
>      [<ffffffff813792db>] filemap_map_pages+0x42b/0xdd0
>      [<ffffffff8140e0d5>] handle_mm_fault+0x17d5/0x28b0
>      [<ffffffff810ce460>] __do_page_fault+0x310/0x8f0
>      [<ffffffff810cec7d>] trace_do_page_fault+0x18d/0x310
>      [<ffffffff810c2177>] do_async_page_fault+0x27/0xa0
>      [<ffffffff8389e258>] async_page_fault+0x28/0x30
> 
> The important bits from the above is that filemap_map_pages() is calling
> into the page allocator while holding rcu_read_lock (sleeping is not
> allowed inside RCU read-side critical sections).
> 
> According to Kirill Shutemov, the prefaulting code in do_fault_around()
> is supposed to take care of this, but missing error handling means that
> the allocation failure can go unnoticed.
> 
> We don't need to return VM_FAULT_OOM (or any other error) here, since we
> can just let the normal fault path try again.
> 
> Fixes: 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Worth noticing that it's failure of order-0 allocation and unlikely to
ever happen under normal workloads without fault injection for page
allocator.

> ---
>  mm/memory.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 4425b60..0400483 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3133,6 +3133,8 @@ static int do_fault_around(struct fault_env *fe, pgoff_t start_pgoff)
>  
>  	if (pmd_none(*fe->pmd)) {
>  		fe->prealloc_pte = pte_alloc_one(fe->vma->vm_mm, fe->address);
> +		if (!fe->prealloc_pte)
> +			goto out;
>  		smp_wmb(); /* See comment in __pte_alloc() */
>  	}
>  
> -- 
> 1.9.1
> 
> --
> 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>

-- 
 Kirill A. Shutemov

--
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] 9+ messages in thread

* [PATCH] mm: fail prefaulting if page table allocation fails
@ 2016-07-28 12:15 ` Vegard Nossum
  0 siblings, 0 replies; 9+ messages in thread
From: Vegard Nossum @ 2016-07-28 12:15 UTC (permalink / raw)
  To: linux-mm, akpm; +Cc: linux-kernel, Vegard Nossum, Kirill A. Shutemov

I ran into this:

    BUG: sleeping function called from invalid context at mm/page_alloc.c:3784
    in_atomic(): 0, irqs_disabled(): 0, pid: 1434, name: trinity-c1
    2 locks held by trinity-c1/1434:
     #0:  (&mm->mmap_sem){......}, at: [<ffffffff810ce31e>] __do_page_fault+0x1ce/0x8f0
     #1:  (rcu_read_lock){......}, at: [<ffffffff81378f86>] filemap_map_pages+0xd6/0xdd0

    CPU: 0 PID: 1434 Comm: trinity-c1 Not tainted 4.7.0+ #58
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
     ffff8800b662f698 ffff8800b662f548 ffffffff81d6d001 ffffffff83a61100
     ffff8800b662f620 ffff8800b662f610 ffffffff81373fd1 0000000041b58ab3
     ffffffff8406ca21 ffffffff81373e4c 0000000041b58ab3 ffffffff00000008
    Call Trace:
     [<ffffffff81d6d001>] dump_stack+0x65/0x84
     [<ffffffff81373fd1>] panic+0x185/0x2dd
     [<ffffffff8118e38c>] ___might_sleep+0x51c/0x600
     [<ffffffff8118e500>] __might_sleep+0x90/0x1a0
     [<ffffffff81392761>] __alloc_pages_nodemask+0x5b1/0x2160
     [<ffffffff814665ac>] alloc_pages_current+0xcc/0x370
     [<ffffffff810d95b2>] pte_alloc_one+0x12/0x90
     [<ffffffff814053cd>] __pte_alloc+0x1d/0x200
     [<ffffffff8140be4e>] alloc_set_pte+0xe3e/0x14a0
     [<ffffffff813792db>] filemap_map_pages+0x42b/0xdd0
     [<ffffffff8140e0d5>] handle_mm_fault+0x17d5/0x28b0
     [<ffffffff810ce460>] __do_page_fault+0x310/0x8f0
     [<ffffffff810cec7d>] trace_do_page_fault+0x18d/0x310
     [<ffffffff810c2177>] do_async_page_fault+0x27/0xa0
     [<ffffffff8389e258>] async_page_fault+0x28/0x30

The important bits from the above is that filemap_map_pages() is calling
into the page allocator while holding rcu_read_lock (sleeping is not
allowed inside RCU read-side critical sections).

According to Kirill Shutemov, the prefaulting code in do_fault_around()
is supposed to take care of this, but missing error handling means that
the allocation failure can go unnoticed.

We don't need to return VM_FAULT_OOM (or any other error) here, since we
can just let the normal fault path try again.

Fixes: 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 mm/memory.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index 4425b60..0400483 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3133,6 +3133,8 @@ static int do_fault_around(struct fault_env *fe, pgoff_t start_pgoff)
 
 	if (pmd_none(*fe->pmd)) {
 		fe->prealloc_pte = pte_alloc_one(fe->vma->vm_mm, fe->address);
+		if (!fe->prealloc_pte)
+			goto out;
 		smp_wmb(); /* See comment in __pte_alloc() */
 	}
 
-- 
1.9.1

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

* [PATCH] mm: fail prefaulting if page table allocation fails
@ 2016-07-28 12:15 ` Vegard Nossum
  0 siblings, 0 replies; 9+ messages in thread
From: Vegard Nossum @ 2016-07-28 12:15 UTC (permalink / raw)
  To: linux-mm, akpm; +Cc: linux-kernel, Vegard Nossum, Kirill A. Shutemov

I ran into this:

    BUG: sleeping function called from invalid context at mm/page_alloc.c:3784
    in_atomic(): 0, irqs_disabled(): 0, pid: 1434, name: trinity-c1
    2 locks held by trinity-c1/1434:
     #0:  (&mm->mmap_sem){......}, at: [<ffffffff810ce31e>] __do_page_fault+0x1ce/0x8f0
     #1:  (rcu_read_lock){......}, at: [<ffffffff81378f86>] filemap_map_pages+0xd6/0xdd0

    CPU: 0 PID: 1434 Comm: trinity-c1 Not tainted 4.7.0+ #58
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
     ffff8800b662f698 ffff8800b662f548 ffffffff81d6d001 ffffffff83a61100
     ffff8800b662f620 ffff8800b662f610 ffffffff81373fd1 0000000041b58ab3
     ffffffff8406ca21 ffffffff81373e4c 0000000041b58ab3 ffffffff00000008
    Call Trace:
     [<ffffffff81d6d001>] dump_stack+0x65/0x84
     [<ffffffff81373fd1>] panic+0x185/0x2dd
     [<ffffffff8118e38c>] ___might_sleep+0x51c/0x600
     [<ffffffff8118e500>] __might_sleep+0x90/0x1a0
     [<ffffffff81392761>] __alloc_pages_nodemask+0x5b1/0x2160
     [<ffffffff814665ac>] alloc_pages_current+0xcc/0x370
     [<ffffffff810d95b2>] pte_alloc_one+0x12/0x90
     [<ffffffff814053cd>] __pte_alloc+0x1d/0x200
     [<ffffffff8140be4e>] alloc_set_pte+0xe3e/0x14a0
     [<ffffffff813792db>] filemap_map_pages+0x42b/0xdd0
     [<ffffffff8140e0d5>] handle_mm_fault+0x17d5/0x28b0
     [<ffffffff810ce460>] __do_page_fault+0x310/0x8f0
     [<ffffffff810cec7d>] trace_do_page_fault+0x18d/0x310
     [<ffffffff810c2177>] do_async_page_fault+0x27/0xa0
     [<ffffffff8389e258>] async_page_fault+0x28/0x30

The important bits from the above is that filemap_map_pages() is calling
into the page allocator while holding rcu_read_lock (sleeping is not
allowed inside RCU read-side critical sections).

According to Kirill Shutemov, the prefaulting code in do_fault_around()
is supposed to take care of this, but missing error handling means that
the allocation failure can go unnoticed.

We don't need to return VM_FAULT_OOM (or any other error) here, since we
can just let the normal fault path try again.

Fixes: 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 mm/memory.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index 4425b60..0400483 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3133,6 +3133,8 @@ static int do_fault_around(struct fault_env *fe, pgoff_t start_pgoff)
 
 	if (pmd_none(*fe->pmd)) {
 		fe->prealloc_pte = pte_alloc_one(fe->vma->vm_mm, fe->address);
+		if (!fe->prealloc_pte)
+			goto out;
 		smp_wmb(); /* See comment in __pte_alloc() */
 	}
 
-- 
1.9.1

--
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] 9+ messages in thread

end of thread, other threads:[~2016-07-29  8:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <01fa01d1e94c$4be09210$e3a1b630$@alibaba-inc.com>
2016-07-29  8:00 ` [PATCH] mm: fail prefaulting if page table allocation fails Hillf Danton
2016-07-29  8:10   ` Kirill A. Shutemov
2016-07-29  8:23     ` Hillf Danton
2016-07-29  8:31       ` Kirill A. Shutemov
2016-07-29  8:40         ` Hillf Danton
2016-07-28 12:15 Vegard Nossum
2016-07-28 12:15 ` Vegard Nossum
2016-07-28 12:21 ` Kirill A. Shutemov
2016-07-28 12:21   ` Kirill A. Shutemov

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.