mm/page_alloc: avoid hard lockups in __alloc_pages_bulk()

Commit Message

Zhang, Qiang July 10, 2021, 11:29 a.m. UTC

From: Zqiang <[e-mail]>

The __alloc_pages_bulk() mainly used for batch allocation of
order-0 pages, in the case of holding pagesets.lock, if too
many pages are required, maybe trigger hard lockup watchdog.

Signed-off-by: Zqiang <[e-mail]>
---
 mm/page_alloc.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Andrew Morton July 10, 2021, 6:46 p.m. UTC | #1

On Sat, 10 Jul 2021 19:29:29 +0800 [e-mail] wrote:

> From: Zqiang <[e-mail]>
> 
> The __alloc_pages_bulk() mainly used for batch allocation of
> order-0 pages, in the case of holding pagesets.lock, if too
> many pages are required, maybe trigger hard lockup watchdog.

Ouch.  Has this been observed in testing?  If so, can you please share
the kernel debug output from that event?

Matthew Wilcox July 10, 2021, 9:10 p.m. UTC | #2

On Sat, Jul 10, 2021 at 11:46:13AM -0700, Andrew Morton wrote:
> On Sat, 10 Jul 2021 19:29:29 +0800 [e-mail] wrote:
> 
> > From: Zqiang <[e-mail]>
> > 
> > The __alloc_pages_bulk() mainly used for batch allocation of
> > order-0 pages, in the case of holding pagesets.lock, if too
> > many pages are required, maybe trigger hard lockup watchdog.
> 
> Ouch.  Has this been observed in testing?  If so, can you please share
> the kernel debug output from that event?

This should be fixed in the caller by asking for fewer pages.
The NFS and vmalloc cases have already been fixed for this.

Mel Gorman July 15, 2021, 8:35 a.m. UTC | #3

On Sat, Jul 10, 2021 at 10:57:53PM +0000, Zhang, Qiang wrote:
> ________________________________
> ??????: Matthew Wilcox <[e-mail]>
> ????????: ??????, ???? 11, 2021 05:10
> ??????: Andrew Morton
> ????: Zhang, Qiang; [e-mail]; [e-mail]; [e-mail]
> ????: Re: [PATCH] mm/page_alloc: avoid hard lockups in __alloc_pages_bulk()
> 
> [Please note: This e-mail is from an EXTERNAL e-mail address]
> 
> On Sat, Jul 10, 2021 at 11:46:13AM -0700, Andrew Morton wrote:
> > On Sat, 10 Jul 2021 19:29:29 +0800 [e-mail] wrote:
> >
> > > From: Zqiang <[e-mail]>
> > >
> > > The __alloc_pages_bulk() mainly used for batch allocation of
> > > order-0 pages, in the case of holding pagesets.lock, if too
> > > many pages are required, maybe trigger hard lockup watchdog.
> >
> > Ouch.  Has this been observed in testing?  If so, can you please share
> > the kernel debug output from that event?
> 
> >This should be fixed in the caller by asking for fewer pages.
> >The NFS and vmalloc cases have already been fixed for this.
> 
> The NFS and vmalloc cases haven  been fixed??
> I don??t see if there is any information about that?
> 

AFAIK, NFS simply doesn't ask for a large enough number of pages to be
of concern. For vmalloc, it's somewhat theoritical that it can happen
for anything other than a stress test but this exists
https://lore.kernel.org/r/[e-mail]

I had no objection to the patch but didn't feel strongly enough to say
anything about it either given that it was triggered artifically.

Patch

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d6e94cc8066c..1127db25507f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5315,6 +5315,8 @@  unsigned long __alloc_pages_bulk(gfp_t gfp, int preferred_nid,
 		else
 			page_array[nr_populated] = page;
 		nr_populated++;
+
+		touch_nmi_watchdog();
 	}
 
 	local_unlock_irqrestore(&pagesets.lock, flags);