* [PATCH] mm/page_alloc: Fix sleeping function called in case of irqsdisable.
@ 2021-07-06 3:29 qiang.zhang
2021-07-06 6:50 ` Muchun Song
2021-07-06 8:19 ` Mel Gorman
0 siblings, 2 replies; 5+ messages in thread
From: qiang.zhang @ 2021-07-06 3:29 UTC (permalink / raw)
To: mgorman, akpm, alobakin; +Cc: songmuchun, wangqing, linux-mm, linux-kernel
From: Zqiang <qiang.zhang@windriver.com>
BUG: sleeping function called from invalid context at mm/page_alloc.c:5179
in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0
.....
__dump_stack lib/dump_stack.c:79 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:96
___might_sleep.cold+0x1f1/0x237 kernel/sched/core.c:9153
prepare_alloc_pages+0x3da/0x580 mm/page_alloc.c:5179
__alloc_pages+0x12f/0x500 mm/page_alloc.c:5375
alloc_page_interleave+0x1e/0x200 mm/mempolicy.c:2147
alloc_pages+0x238/0x2a0 mm/mempolicy.c:2270
stack_depot_save+0x39d/0x4e0 lib/stackdepot.c:303
save_stack+0x15e/0x1e0 mm/page_owner.c:120
__set_page_owner+0x50/0x290 mm/page_owner.c:181
prep_new_page mm/page_alloc.c:2445 [inline]
__alloc_pages_bulk+0x8b9/0x1870 mm/page_alloc.c:5313
alloc_pages_bulk_array_node include/linux/gfp.h:557 [inline]
vm_area_alloc_pages mm/vmalloc.c:2775 [inline]
__vmalloc_area_node mm/vmalloc.c:2845 [inline]
__vmalloc_node_range+0x39d/0x960 mm/vmalloc.c:2947
__vmalloc_node mm/vmalloc.c:2996 [inline]
vzalloc+0x67/0x80 mm/vmalloc.c:3066
If the PAGE_OWNER is enabled, in __set_page_owner(), the pages will be
allocated to save calltrace info, due to the allocated action is executed
under irq disable(pagesets.lock be held), if the gfp variable contains
the flag that causes sleep, will trigger above information. the
prep_new_page() is not need to disable irq for protection, fix it through
enable irq before call prep_new_page().
Fixes: 0f87d9d30f21 ("mm/page_alloc: add an array-based interface to the bulk page allocator")
Reported-by: syzbot+0123a2b8f9e623d5b443@syzkaller.appspotmail.com
Signed-off-by: Zqiang <qiang.zhang@windriver.com>
---
mm/page_alloc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d6e94cc8066c..8d7f0f5ce891 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5309,12 +5309,14 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
}
nr_account++;
+ local_unlock_irqrestore(&pagesets.lock, flags);
prep_new_page(page, 0, gfp, 0);
if (page_list)
list_add(&page->lru, page_list);
else
page_array[nr_populated] = page;
nr_populated++;
+ local_lock_irqsave(&pagesets.lock, flags);
}
local_unlock_irqrestore(&pagesets.lock, flags);
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [External] [PATCH] mm/page_alloc: Fix sleeping function called in case of irqsdisable.
2021-07-06 3:29 [PATCH] mm/page_alloc: Fix sleeping function called in case of irqsdisable qiang.zhang
@ 2021-07-06 6:50 ` Muchun Song
2021-07-06 8:19 ` Mel Gorman
1 sibling, 0 replies; 5+ messages in thread
From: Muchun Song @ 2021-07-06 6:50 UTC (permalink / raw)
To: qiang.zhang
Cc: mgorman, Andrew Morton, alobakin, wangqing,
Linux Memory Management List, LKML
On Tue, Jul 6, 2021 at 11:29 AM <qiang.zhang@windriver.com> wrote:
>
> From: Zqiang <qiang.zhang@windriver.com>
>
> BUG: sleeping function called from invalid context at mm/page_alloc.c:5179
> in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0
> .....
> __dump_stack lib/dump_stack.c:79 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:96
> ___might_sleep.cold+0x1f1/0x237 kernel/sched/core.c:9153
> prepare_alloc_pages+0x3da/0x580 mm/page_alloc.c:5179
> __alloc_pages+0x12f/0x500 mm/page_alloc.c:5375
> alloc_page_interleave+0x1e/0x200 mm/mempolicy.c:2147
> alloc_pages+0x238/0x2a0 mm/mempolicy.c:2270
> stack_depot_save+0x39d/0x4e0 lib/stackdepot.c:303
> save_stack+0x15e/0x1e0 mm/page_owner.c:120
> __set_page_owner+0x50/0x290 mm/page_owner.c:181
> prep_new_page mm/page_alloc.c:2445 [inline]
> __alloc_pages_bulk+0x8b9/0x1870 mm/page_alloc.c:5313
> alloc_pages_bulk_array_node include/linux/gfp.h:557 [inline]
> vm_area_alloc_pages mm/vmalloc.c:2775 [inline]
> __vmalloc_area_node mm/vmalloc.c:2845 [inline]
> __vmalloc_node_range+0x39d/0x960 mm/vmalloc.c:2947
> __vmalloc_node mm/vmalloc.c:2996 [inline]
> vzalloc+0x67/0x80 mm/vmalloc.c:3066
>
> If the PAGE_OWNER is enabled, in __set_page_owner(), the pages will be
> allocated to save calltrace info, due to the allocated action is executed
> under irq disable(pagesets.lock be held), if the gfp variable contains
> the flag that causes sleep, will trigger above information. the
> prep_new_page() is not need to disable irq for protection, fix it through
> enable irq before call prep_new_page().
>
> Fixes: 0f87d9d30f21 ("mm/page_alloc: add an array-based interface to the bulk page allocator")
> Reported-by: syzbot+0123a2b8f9e623d5b443@syzkaller.appspotmail.com
> Signed-off-by: Zqiang <qiang.zhang@windriver.com>
> ---
> mm/page_alloc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index d6e94cc8066c..8d7f0f5ce891 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5309,12 +5309,14 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
> }
> nr_account++;
>
> + local_unlock_irqrestore(&pagesets.lock, flags);
> prep_new_page(page, 0, gfp, 0);
> if (page_list)
> list_add(&page->lru, page_list);
> else
> page_array[nr_populated] = page;
> nr_populated++;
> + local_lock_irqsave(&pagesets.lock, flags);
Because pagesets.lock is relocked here, the current
thread can be scheduled to another CPU. The @pcp
and @pcp_list need to be reloaded. Otherwise, it is
broken in the next loop.
Thanks.
> }
>
> local_unlock_irqrestore(&pagesets.lock, flags);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [External] [PATCH] mm/page_alloc: Fix sleeping function called in case of irqsdisable.
@ 2021-07-06 6:50 ` Muchun Song
0 siblings, 0 replies; 5+ messages in thread
From: Muchun Song @ 2021-07-06 6:50 UTC (permalink / raw)
To: qiang.zhang
Cc: mgorman, Andrew Morton, alobakin, wangqing,
Linux Memory Management List, LKML
On Tue, Jul 6, 2021 at 11:29 AM <qiang.zhang@windriver.com> wrote:
>
> From: Zqiang <qiang.zhang@windriver.com>
>
> BUG: sleeping function called from invalid context at mm/page_alloc.c:5179
> in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0
> .....
> __dump_stack lib/dump_stack.c:79 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:96
> ___might_sleep.cold+0x1f1/0x237 kernel/sched/core.c:9153
> prepare_alloc_pages+0x3da/0x580 mm/page_alloc.c:5179
> __alloc_pages+0x12f/0x500 mm/page_alloc.c:5375
> alloc_page_interleave+0x1e/0x200 mm/mempolicy.c:2147
> alloc_pages+0x238/0x2a0 mm/mempolicy.c:2270
> stack_depot_save+0x39d/0x4e0 lib/stackdepot.c:303
> save_stack+0x15e/0x1e0 mm/page_owner.c:120
> __set_page_owner+0x50/0x290 mm/page_owner.c:181
> prep_new_page mm/page_alloc.c:2445 [inline]
> __alloc_pages_bulk+0x8b9/0x1870 mm/page_alloc.c:5313
> alloc_pages_bulk_array_node include/linux/gfp.h:557 [inline]
> vm_area_alloc_pages mm/vmalloc.c:2775 [inline]
> __vmalloc_area_node mm/vmalloc.c:2845 [inline]
> __vmalloc_node_range+0x39d/0x960 mm/vmalloc.c:2947
> __vmalloc_node mm/vmalloc.c:2996 [inline]
> vzalloc+0x67/0x80 mm/vmalloc.c:3066
>
> If the PAGE_OWNER is enabled, in __set_page_owner(), the pages will be
> allocated to save calltrace info, due to the allocated action is executed
> under irq disable(pagesets.lock be held), if the gfp variable contains
> the flag that causes sleep, will trigger above information. the
> prep_new_page() is not need to disable irq for protection, fix it through
> enable irq before call prep_new_page().
>
> Fixes: 0f87d9d30f21 ("mm/page_alloc: add an array-based interface to the bulk page allocator")
> Reported-by: syzbot+0123a2b8f9e623d5b443@syzkaller.appspotmail.com
> Signed-off-by: Zqiang <qiang.zhang@windriver.com>
> ---
> mm/page_alloc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index d6e94cc8066c..8d7f0f5ce891 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5309,12 +5309,14 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
> }
> nr_account++;
>
> + local_unlock_irqrestore(&pagesets.lock, flags);
> prep_new_page(page, 0, gfp, 0);
> if (page_list)
> list_add(&page->lru, page_list);
> else
> page_array[nr_populated] = page;
> nr_populated++;
> + local_lock_irqsave(&pagesets.lock, flags);
Because pagesets.lock is relocked here, the current
thread can be scheduled to another CPU. The @pcp
and @pcp_list need to be reloaded. Otherwise, it is
broken in the next loop.
Thanks.
> }
>
> local_unlock_irqrestore(&pagesets.lock, flags);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [External] [PATCH] mm/page_alloc: Fix sleeping function called in case of irqsdisable.
2021-07-06 6:50 ` Muchun Song
(?)
@ 2021-07-06 7:16 ` Zhang, Qiang
-1 siblings, 0 replies; 5+ messages in thread
From: Zhang, Qiang @ 2021-07-06 7:16 UTC (permalink / raw)
To: Muchun Song
Cc: mgorman, Andrew Morton, alobakin, wangqing,
Linux Memory Management List, LKML
________________________________________
From: Muchun Song <songmuchun@bytedance.com>
Sent: Tuesday, 6 July 2021 14:50
To: Zhang, Qiang
Cc: mgorman@techsingularity.net; Andrew Morton; alobakin@pm.me; wangqing@vivo.com; Linux Memory Management List; LKML
Subject: Re: [External] [PATCH] mm/page_alloc: Fix sleeping function called in case of irqsdisable.
[Please note: This e-mail is from an EXTERNAL e-mail address]
On Tue, Jul 6, 2021 at 11:29 AM <qiang.zhang@windriver.com> wrote:
>
> From: Zqiang <qiang.zhang@windriver.com>
>
> BUG: sleeping function called from invalid context at mm/page_alloc.c:5179
> in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0
> .....
> __dump_stack lib/dump_stack.c:79 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:96
> ___might_sleep.cold+0x1f1/0x237 kernel/sched/core.c:9153
> prepare_alloc_pages+0x3da/0x580 mm/page_alloc.c:5179
> __alloc_pages+0x12f/0x500 mm/page_alloc.c:5375
> alloc_page_interleave+0x1e/0x200 mm/mempolicy.c:2147
> alloc_pages+0x238/0x2a0 mm/mempolicy.c:2270
> stack_depot_save+0x39d/0x4e0 lib/stackdepot.c:303
> save_stack+0x15e/0x1e0 mm/page_owner.c:120
> __set_page_owner+0x50/0x290 mm/page_owner.c:181
> prep_new_page mm/page_alloc.c:2445 [inline]
> __alloc_pages_bulk+0x8b9/0x1870 mm/page_alloc.c:5313
> alloc_pages_bulk_array_node include/linux/gfp.h:557 [inline]
> vm_area_alloc_pages mm/vmalloc.c:2775 [inline]
> __vmalloc_area_node mm/vmalloc.c:2845 [inline]
> __vmalloc_node_range+0x39d/0x960 mm/vmalloc.c:2947
> __vmalloc_node mm/vmalloc.c:2996 [inline]
> vzalloc+0x67/0x80 mm/vmalloc.c:3066
>
> If the PAGE_OWNER is enabled, in __set_page_owner(), the pages will be
> allocated to save calltrace info, due to the allocated action is executed
> under irq disable(pagesets.lock be held), if the gfp variable contains
> the flag that causes sleep, will trigger above information. the
> prep_new_page() is not need to disable irq for protection, fix it through
> enable irq before call prep_new_page().
>
> Fixes: 0f87d9d30f21 ("mm/page_alloc: add an array-based interface to the bulk page allocator")
> Reported-by: syzbot+0123a2b8f9e623d5b443@syzkaller.appspotmail.com
> Signed-off-by: Zqiang <qiang.zhang@windriver.com>
> ---
> mm/page_alloc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index d6e94cc8066c..8d7f0f5ce891 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5309,12 +5309,14 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
> }
> nr_account++;
>
> + local_unlock_irqrestore(&pagesets.lock, flags);
> prep_new_page(page, 0, gfp, 0);
> if (page_list)
> list_add(&page->lru, page_list);
> else
> page_array[nr_populated] = page;
> nr_populated++;
> + local_lock_irqsave(&pagesets.lock, flags);
>Because pagesets.lock is relocked here, the current
>thread can be scheduled to another CPU. The @pcp
>and @pcp_list need to be reloaded. Otherwise, it is
>broken in the next loop.
>
Thanks Muchun,
I ignored this problem, I will resend.
>Thanks.
> }
>
> local_unlock_irqrestore(&pagesets.lock, flags);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/page_alloc: Fix sleeping function called in case of irqsdisable.
2021-07-06 3:29 [PATCH] mm/page_alloc: Fix sleeping function called in case of irqsdisable qiang.zhang
2021-07-06 6:50 ` Muchun Song
@ 2021-07-06 8:19 ` Mel Gorman
1 sibling, 0 replies; 5+ messages in thread
From: Mel Gorman @ 2021-07-06 8:19 UTC (permalink / raw)
To: qiang.zhang; +Cc: akpm, alobakin, songmuchun, wangqing, linux-mm, linux-kernel
On Tue, Jul 06, 2021 at 11:29:07AM +0800, qiang.zhang@windriver.com wrote:
> From: Zqiang <qiang.zhang@windriver.com>
>
> BUG: sleeping function called from invalid context at mm/page_alloc.c:5179
> in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0
> .....
> __dump_stack lib/dump_stack.c:79 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:96
> ___might_sleep.cold+0x1f1/0x237 kernel/sched/core.c:9153
> prepare_alloc_pages+0x3da/0x580 mm/page_alloc.c:5179
> __alloc_pages+0x12f/0x500 mm/page_alloc.c:5375
> alloc_page_interleave+0x1e/0x200 mm/mempolicy.c:2147
> alloc_pages+0x238/0x2a0 mm/mempolicy.c:2270
> stack_depot_save+0x39d/0x4e0 lib/stackdepot.c:303
> save_stack+0x15e/0x1e0 mm/page_owner.c:120
> __set_page_owner+0x50/0x290 mm/page_owner.c:181
> prep_new_page mm/page_alloc.c:2445 [inline]
> __alloc_pages_bulk+0x8b9/0x1870 mm/page_alloc.c:5313
> alloc_pages_bulk_array_node include/linux/gfp.h:557 [inline]
> vm_area_alloc_pages mm/vmalloc.c:2775 [inline]
> __vmalloc_area_node mm/vmalloc.c:2845 [inline]
> __vmalloc_node_range+0x39d/0x960 mm/vmalloc.c:2947
> __vmalloc_node mm/vmalloc.c:2996 [inline]
> vzalloc+0x67/0x80 mm/vmalloc.c:3066
>
> If the PAGE_OWNER is enabled, in __set_page_owner(), the pages will be
> allocated to save calltrace info, due to the allocated action is executed
> under irq disable(pagesets.lock be held), if the gfp variable contains
> the flag that causes sleep, will trigger above information. the
> prep_new_page() is not need to disable irq for protection, fix it through
> enable irq before call prep_new_page().
>
> Fixes: 0f87d9d30f21 ("mm/page_alloc: add an array-based interface to the bulk page allocator")
> Reported-by: syzbot+0123a2b8f9e623d5b443@syzkaller.appspotmail.com
> Signed-off-by: Zqiang <qiang.zhang@windriver.com>
This will hurt the performance of the bulk allocator a lot because it'll
no longer batch the IRQ disable/enabling and be similar to simply using
the single page allocator. It also impacts the performance even if page
owner tracking is off or disabled in kconfig
Given that this is related to page owner tracking, a more straight
forward basic fix is the following untested patch
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3b97e17806be..4f96081727f2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5224,6 +5224,18 @@ unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
if (unlikely(nr_pages <= 0))
return 0;
+#ifdef CONFIG_PAGE_OWNER
+ /*
+ * If page owner tracking is enabled then prep_new_page may need to
+ * allocate space and then sleep with the pagesets.lock lock held.
+ * Releasing/reacquiring the lock on each page would offset much
+ * of the benefit of bulk page allocation so simply return 1 page
+ * when tracking page owners.
+ */
+ if (static_branch_unlikely(&page_owner_inited))
+ goto failed;
+#endif
+
/*
* Skip populated array elements to determine if any pages need
* to be allocated before disabling IRQs.
There are other ways it could be done. Given it's struct page pointers,
the least significant bit could be used to track pages that need to be
prepped and then iterate through the list/array to prep newly allocated
pages. It's not clear it's worth the complexity though unless there are
really interesting use cases where page owner is enabled in production.
--
Mel Gorman
SUSE Labs
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-06 8:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-06 3:29 [PATCH] mm/page_alloc: Fix sleeping function called in case of irqsdisable qiang.zhang
2021-07-06 6:50 ` [External] " Muchun Song
2021-07-06 6:50 ` Muchun Song
2021-07-06 7:16 ` Zhang, Qiang
2021-07-06 8:19 ` Mel Gorman
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.