| Message ID | 20180627214447.260804-1-cannonmatthews@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/27/2018 02:44 PM, Cannon Matthews wrote: > When booting with very large numbers of gigantic (i.e. 1G) pages, the > operations in the loop of gather_bootmem_prealloc, and specifically > prep_compound_gigantic_page, takes a very long time, and can cause a > softlockup if enough pages are requested at boot. > > For example booting with 3844 1G pages requires prepping Wow! I wish I had a system with that much memory to test. :) > (set_compound_head, init the count) over 1 billion 4K tail pages, which > takes considerable time. This should also apply to reserving the same > amount of memory as 2M pages, as the same number of struct pages > are affected in either case. Actually, this change would not apply to 2M (on x86) pages. The hugetlbfs initialization code is a bit confusing, but alloc_bootmem_huge_page and gather_bootmem_prealloc are only exercised in the case where huge page order >= MAX_ORDER. Allocation and initialization of 2M pages happens after the normal memory allocators are setup via the routine hugetlb_hstate_alloc_pages. And, there is already a cond_resched in that loop today. Note that 'else if' in the for loop of hugetlb_hstate_alloc_pages. This allows the same routine to be called for early gigantic page allocations using the bootmem allocator, and later normal (2M) allocations using the normal memory allocators. To me, this is a source of confusion and is something I plan to clean up in the future. > Add a cond_resched() to the outer loop in gather_bootmem_prealloc() to > prevent this lockup. > > Tested: Booted with softlockup_panic=1 hugepagesz=1G hugepages=3844 and > no softlockup is reported, and the hugepages are reported as > successfully setup. > > Signed-off-by: Cannon Matthews <cannonmatthews@google.com> My only suggestion would be to remove the mention of 2M pages in the commit message. Thanks for adding this. Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
On Wed, 27 Jun 2018 16:27:24 -0700 Mike Kravetz <mike.kravetz@oracle.com> wrote: > My only suggestion would be to remove the mention of 2M pages in the > commit message. Thanks for adding this. I have removed that sentence. > Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com> Thanks again.
On Wed 27-06-18 14:44:47, Cannon Matthews wrote: > When booting with very large numbers of gigantic (i.e. 1G) pages, the > operations in the loop of gather_bootmem_prealloc, and specifically > prep_compound_gigantic_page, takes a very long time, and can cause a > softlockup if enough pages are requested at boot. > > For example booting with 3844 1G pages requires prepping > (set_compound_head, init the count) over 1 billion 4K tail pages, which > takes considerable time. This should also apply to reserving the same > amount of memory as 2M pages, as the same number of struct pages > are affected in either case. > > Add a cond_resched() to the outer loop in gather_bootmem_prealloc() to > prevent this lockup. > > Tested: Booted with softlockup_panic=1 hugepagesz=1G hugepages=3844 and > no softlockup is reported, and the hugepages are reported as > successfully setup. > > Signed-off-by: Cannon Matthews <cannonmatthews@google.com> Acked-by: Michal Hocko <mhocko@suse.com> Thanks! > --- > mm/hugetlb.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index a963f2034dfc..d38273c32d3b 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -2169,6 +2169,7 @@ static void __init gather_bootmem_prealloc(void) > */ > if (hstate_is_gigantic(h)) > adjust_managed_page_count(page, 1 << h->order); > + cond_resched(); > } > } > > -- > 2.18.0.rc2.346.g013aa6912e-goog
Thanks for the quick turnaround. Good to know about the how the 2M code path differs, I have been trying to trace through some of this and it's easy to get lost between which applies to which size. Thanks! On Thu, Jun 28, 2018 at 12:03 PM Michal Hocko <mhocko@kernel.org> wrote: > > On Wed 27-06-18 14:44:47, Cannon Matthews wrote: > > When booting with very large numbers of gigantic (i.e. 1G) pages, the > > operations in the loop of gather_bootmem_prealloc, and specifically > > prep_compound_gigantic_page, takes a very long time, and can cause a > > softlockup if enough pages are requested at boot. > > > > For example booting with 3844 1G pages requires prepping > > (set_compound_head, init the count) over 1 billion 4K tail pages, which > > takes considerable time. This should also apply to reserving the same > > amount of memory as 2M pages, as the same number of struct pages > > are affected in either case. > > > > Add a cond_resched() to the outer loop in gather_bootmem_prealloc() to > > prevent this lockup. > > > > Tested: Booted with softlockup_panic=1 hugepagesz=1G hugepages=3844 and > > no softlockup is reported, and the hugepages are reported as > > successfully setup. > > > > Signed-off-by: Cannon Matthews <cannonmatthews@google.com> > > Acked-by: Michal Hocko <mhocko@suse.com> > > Thanks! > > > --- > > mm/hugetlb.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > > index a963f2034dfc..d38273c32d3b 100644 > > --- a/mm/hugetlb.c > > +++ b/mm/hugetlb.c > > @@ -2169,6 +2169,7 @@ static void __init gather_bootmem_prealloc(void) > > */ > > if (hstate_is_gigantic(h)) > > adjust_managed_page_count(page, 1 << h->order); > > + cond_resched(); > > } > > } > > > > -- > > 2.18.0.rc2.346.g013aa6912e-goog > > -- > Michal Hocko > SUSE Labs
On Thu 28-06-18 15:16:46, Cannon Matthews wrote: > Thanks for the quick turnaround. > > Good to know about the how the 2M code path differs, I have been > trying to trace through some of this and it's easy to get lost between > which applies to which size. Yeah, GB hugetlb pages implementation has been hacked into the existing hugetlb code in a quite ugly way. We have done some cleanups since then but there is still a lot of room for improvements.
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index a963f2034dfc..d38273c32d3b 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -2169,6 +2169,7 @@ static void __init gather_bootmem_prealloc(void) */ if (hstate_is_gigantic(h)) adjust_managed_page_count(page, 1 << h->order); + cond_resched(); } }
When booting with very large numbers of gigantic (i.e. 1G) pages, the operations in the loop of gather_bootmem_prealloc, and specifically prep_compound_gigantic_page, takes a very long time, and can cause a softlockup if enough pages are requested at boot. For example booting with 3844 1G pages requires prepping (set_compound_head, init the count) over 1 billion 4K tail pages, which takes considerable time. This should also apply to reserving the same amount of memory as 2M pages, as the same number of struct pages are affected in either case. Add a cond_resched() to the outer loop in gather_bootmem_prealloc() to prevent this lockup. Tested: Booted with softlockup_panic=1 hugepagesz=1G hugepages=3844 and no softlockup is reported, and the hugepages are reported as successfully setup. Signed-off-by: Cannon Matthews <cannonmatthews@google.com> --- mm/hugetlb.c | 1 + 1 file changed, 1 insertion(+)