| Message ID | 20200401225723.14164-2-pasha.tatashin@soleen.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | initialize deferred pages with interrupts enabled | expand |
On 02.04.20 00:57, Pavel Tatashin wrote: > From: Daniel Jordan <daniel.m.jordan@oracle.com> > > deferred_init_memmap() disables interrupts the entire time, so it calls > touch_nmi_watchdog() periodically to avoid soft lockup splats. Soon it > will run with interrupts enabled, at which point cond_resched() should > be used instead. > > deferred_grow_zone() makes the same watchdog calls through code shared > with deferred init but will continue to run with interrupts disabled, so > it can't call cond_resched(). > > Pull the watchdog calls up to these two places to allow the first to be > changed later, independently of the second. The frequency reduces from > twice per pageblock (init and free) to once per max order block. > > Fixes: 3a2d7fa8a3d5 ("mm: disable interrupts while initializing deferred pages") > Cc: stable@vger.kernel.org # 4.17+ > > Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com> > Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com> > --- > mm/page_alloc.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 3c4eb750a199..e8ff6a176164 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1639,7 +1639,6 @@ static void __init deferred_free_pages(unsigned long pfn, > } else if (!(pfn & nr_pgmask)) { > deferred_free_range(pfn - nr_free, nr_free); > nr_free = 1; > - touch_nmi_watchdog(); > } else { > nr_free++; > } > @@ -1669,7 +1668,6 @@ static unsigned long __init deferred_init_pages(struct zone *zone, > continue; > } else if (!page || !(pfn & nr_pgmask)) { > page = pfn_to_page(pfn); > - touch_nmi_watchdog(); > } else { > page++; > } > @@ -1809,8 +1807,10 @@ static int __init deferred_init_memmap(void *data) > * that we can avoid introducing any issues with the buddy > * allocator. > */ > - while (spfn < epfn) > + while (spfn < epfn) { > nr_pages += deferred_init_maxorder(&i, zone, &spfn, &epfn); > + touch_nmi_watchdog(); > + } > zone_empty: > pgdat_resize_unlock(pgdat, &flags); > > @@ -1894,6 +1894,7 @@ deferred_grow_zone(struct zone *zone, unsigned int order) > first_deferred_pfn = spfn; > > nr_pages += deferred_init_maxorder(&i, zone, &spfn, &epfn); > + touch_nmi_watchdog(); > > /* We should only stop along section boundaries */ > if ((first_deferred_pfn ^ spfn) < PAGES_PER_SECTION) > Reviewed-by: David Hildenbrand <david@redhat.com>
I have only now noticed that you have reposted. On Wed 01-04-20 18:57:22, Pavel Tatashin wrote: > From: Daniel Jordan <daniel.m.jordan@oracle.com> > > deferred_init_memmap() disables interrupts the entire time, so it calls > touch_nmi_watchdog() periodically to avoid soft lockup splats. Soon it > will run with interrupts enabled, at which point cond_resched() should > be used instead. > > deferred_grow_zone() makes the same watchdog calls through code shared > with deferred init but will continue to run with interrupts disabled, so > it can't call cond_resched(). > > Pull the watchdog calls up to these two places to allow the first to be > changed later, independently of the second. The frequency reduces from > twice per pageblock (init and free) to once per max order block. > > Fixes: 3a2d7fa8a3d5 ("mm: disable interrupts while initializing deferred pages") > Cc: stable@vger.kernel.org # 4.17+ This patch is not fixing anything, right? It cleans up the code to make further changes easier which is good. > Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com> > Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com> Acked-by: Michal Hocko <mhocko@suse.com> > --- > mm/page_alloc.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 3c4eb750a199..e8ff6a176164 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1639,7 +1639,6 @@ static void __init deferred_free_pages(unsigned long pfn, > } else if (!(pfn & nr_pgmask)) { > deferred_free_range(pfn - nr_free, nr_free); > nr_free = 1; > - touch_nmi_watchdog(); > } else { > nr_free++; > } > @@ -1669,7 +1668,6 @@ static unsigned long __init deferred_init_pages(struct zone *zone, > continue; > } else if (!page || !(pfn & nr_pgmask)) { > page = pfn_to_page(pfn); > - touch_nmi_watchdog(); > } else { > page++; > } > @@ -1809,8 +1807,10 @@ static int __init deferred_init_memmap(void *data) > * that we can avoid introducing any issues with the buddy > * allocator. > */ > - while (spfn < epfn) > + while (spfn < epfn) { > nr_pages += deferred_init_maxorder(&i, zone, &spfn, &epfn); > + touch_nmi_watchdog(); > + } > zone_empty: > pgdat_resize_unlock(pgdat, &flags); > > @@ -1894,6 +1894,7 @@ deferred_grow_zone(struct zone *zone, unsigned int order) > first_deferred_pfn = spfn; > > nr_pages += deferred_init_maxorder(&i, zone, &spfn, &epfn); > + touch_nmi_watchdog(); > > /* We should only stop along section boundaries */ > if ((first_deferred_pfn ^ spfn) < PAGES_PER_SECTION) > -- > 2.17.1 >
On 4/2/20 12:57 AM, Pavel Tatashin wrote: > From: Daniel Jordan <daniel.m.jordan@oracle.com> > > deferred_init_memmap() disables interrupts the entire time, so it calls > touch_nmi_watchdog() periodically to avoid soft lockup splats. Soon it > will run with interrupts enabled, at which point cond_resched() should > be used instead. > > deferred_grow_zone() makes the same watchdog calls through code shared > with deferred init but will continue to run with interrupts disabled, so > it can't call cond_resched(). > > Pull the watchdog calls up to these two places to allow the first to be > changed later, independently of the second. The frequency reduces from > twice per pageblock (init and free) to once per max order block. > > Fixes: 3a2d7fa8a3d5 ("mm: disable interrupts while initializing deferred pages") > Cc: stable@vger.kernel.org # 4.17+ > > Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com> > Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com> Acked-by: Vlastimil Babka <vbabka@suse.cz> > --- > mm/page_alloc.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 3c4eb750a199..e8ff6a176164 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1639,7 +1639,6 @@ static void __init deferred_free_pages(unsigned long pfn, > } else if (!(pfn & nr_pgmask)) { > deferred_free_range(pfn - nr_free, nr_free); > nr_free = 1; > - touch_nmi_watchdog(); > } else { > nr_free++; > } > @@ -1669,7 +1668,6 @@ static unsigned long __init deferred_init_pages(struct zone *zone, > continue; > } else if (!page || !(pfn & nr_pgmask)) { > page = pfn_to_page(pfn); > - touch_nmi_watchdog(); > } else { > page++; > } > @@ -1809,8 +1807,10 @@ static int __init deferred_init_memmap(void *data) > * that we can avoid introducing any issues with the buddy > * allocator. > */ > - while (spfn < epfn) > + while (spfn < epfn) { > nr_pages += deferred_init_maxorder(&i, zone, &spfn, &epfn); > + touch_nmi_watchdog(); > + } > zone_empty: > pgdat_resize_unlock(pgdat, &flags); > > @@ -1894,6 +1894,7 @@ deferred_grow_zone(struct zone *zone, unsigned int order) > first_deferred_pfn = spfn; > > nr_pages += deferred_init_maxorder(&i, zone, &spfn, &epfn); > + touch_nmi_watchdog(); > > /* We should only stop along section boundaries */ > if ((first_deferred_pfn ^ spfn) < PAGES_PER_SECTION) >
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 3c4eb750a199..e8ff6a176164 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1639,7 +1639,6 @@ static void __init deferred_free_pages(unsigned long pfn, } else if (!(pfn & nr_pgmask)) { deferred_free_range(pfn - nr_free, nr_free); nr_free = 1; - touch_nmi_watchdog(); } else { nr_free++; } @@ -1669,7 +1668,6 @@ static unsigned long __init deferred_init_pages(struct zone *zone, continue; } else if (!page || !(pfn & nr_pgmask)) { page = pfn_to_page(pfn); - touch_nmi_watchdog(); } else { page++; } @@ -1809,8 +1807,10 @@ static int __init deferred_init_memmap(void *data) * that we can avoid introducing any issues with the buddy * allocator. */ - while (spfn < epfn) + while (spfn < epfn) { nr_pages += deferred_init_maxorder(&i, zone, &spfn, &epfn); + touch_nmi_watchdog(); + } zone_empty: pgdat_resize_unlock(pgdat, &flags); @@ -1894,6 +1894,7 @@ deferred_grow_zone(struct zone *zone, unsigned int order) first_deferred_pfn = spfn; nr_pages += deferred_init_maxorder(&i, zone, &spfn, &epfn); + touch_nmi_watchdog(); /* We should only stop along section boundaries */ if ((first_deferred_pfn ^ spfn) < PAGES_PER_SECTION)