Re: [PATCH RFC] mm: fix regression with deferred struct page init

[Juergen Gross]

On 20.06.19 17:17, Alexander Duyck wrote:
> On Thu, 2019-06-20 at 11:40 +0200, Juergen Gross wrote:
> > Commit 0e56acae4b4dd4a9 ("mm: initialize MAX_ORDER_NR_PAGES at a time
> > instead of doing larger sections") is causing a regression on some
> > systems when the kernel is booted as Xen dom0.
> >
> > The system will just hang in early boot.
> >
> > Reason is an endless loop in get_page_from_freelist() in case the first
> > zone looked at has no free memory. deferred_grow_zone() is always
> > returning true due to the following code snipplet:
> >
> >    /* If the zone is empty somebody else may have cleared out the zone */
> >    if (!deferred_init_mem_pfn_range_in_zone(&i, zone, &spfn, &epfn,
> >                                             first_deferred_pfn)) {
> >            pgdat->first_deferred_pfn = ULONG_MAX;
> >            pgdat_resize_unlock(pgdat, &flags);
> >            return true;
> >    }
> >
> > This in turn results in the loop as get_page_from_freelist() is
> > assuming forward progress can be made by doing some more struct page
> > initialization.
> >
> > Fixes: 0e56acae4b4dd4a9 ("mm: initialize MAX_ORDER_NR_PAGES at a time instead of doing larger sections")
> > ---
> > This patch makes my system boot again as Xen dom0, but I'm not really
> > sure it is the correct way to do it, hence the RFC.
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> >   mm/page_alloc.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index d66bc8abe0af..6ee754b5cd92 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1826,7 +1826,7 @@ deferred_grow_zone(struct zone *zone, unsigned int order)
> >   						 first_deferred_pfn)) {
> >   		pgdat->first_deferred_pfn = ULONG_MAX;
> >   		pgdat_resize_unlock(pgdat, &flags);
> > -		return true;
> > +		return false;
> >   	}
> >   
> >   	/*
>
> The one change I might make to this would be to do:
> 	return first_deferred_pfn != ULONG_MAX;
>
> That way in the event the previous caller did free up the last of the
> pages and empty the zone just before we got here then we will try one more
> time. Otherwise if it was already done before we got here we exit.

Thanks for the constructive feedback!

Will send a non-RFC variant soon.


Juergen