| Message ID | 20170727061828.11406-1-aneesh.kumar@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Thu 27-07-17 11:48:26, Aneesh Kumar K.V wrote: > For ppc64, we want to call this function when we are not running as guest. What does this mean? > Also, if we failed to allocate hugepages, let the user know. > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > --- > include/linux/hugetlb.h | 1 + > mm/hugetlb.c | 5 ++++- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h > index 0ed8e41aaf11..8bbbd37ab105 100644 > --- a/include/linux/hugetlb.h > +++ b/include/linux/hugetlb.h > @@ -358,6 +358,7 @@ int huge_add_to_page_cache(struct page *page, struct address_space *mapping, > pgoff_t idx); > > /* arch callback */ > +int __init __alloc_bootmem_huge_page(struct hstate *h); > int __init alloc_bootmem_huge_page(struct hstate *h); > > void __init hugetlb_bad_size(void); > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index bc48ee783dd9..a3a7a7e6339e 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -2083,7 +2083,9 @@ struct page *alloc_huge_page_noerr(struct vm_area_struct *vma, > return page; > } > > -int __weak alloc_bootmem_huge_page(struct hstate *h) > +int alloc_bootmem_huge_page(struct hstate *h) > + __attribute__ ((weak, alias("__alloc_bootmem_huge_page"))); > +int __alloc_bootmem_huge_page(struct hstate *h) > { > struct huge_bootmem_page *m; > int nr_nodes, node; > @@ -2104,6 +2106,7 @@ int __weak alloc_bootmem_huge_page(struct hstate *h) > goto found; > } > } > + pr_info("Failed to allocate hugepage of size %ld\n", huge_page_size(h)); > return 0; > > found: > -- > 2.13.3 > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
* Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> [170727 02:18]: > For ppc64, we want to call this function when we are not running as guest. > Also, if we failed to allocate hugepages, let the user know. > [...] > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index bc48ee783dd9..a3a7a7e6339e 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -2083,7 +2083,9 @@ struct page *alloc_huge_page_noerr(struct vm_area_struct *vma, > return page; > } > > -int __weak alloc_bootmem_huge_page(struct hstate *h) > +int alloc_bootmem_huge_page(struct hstate *h) > + __attribute__ ((weak, alias("__alloc_bootmem_huge_page"))); > +int __alloc_bootmem_huge_page(struct hstate *h) > { > struct huge_bootmem_page *m; > int nr_nodes, node; > @@ -2104,6 +2106,7 @@ int __weak alloc_bootmem_huge_page(struct hstate *h) > goto found; > } > } > + pr_info("Failed to allocate hugepage of size %ld\n", huge_page_size(h)); > return 0; > > found: There is already a call to warn the user in the hugetlb_hstate_alloc_pages function. If you look there, you will see that the huge_page_size was translated into a more user friendly format and the count prior to the failure is included. What call path are you trying to cover? Also, you may want your print to be a pr_warn since it is a failure? Thanks, Liam
On 07/27/2017 08:55 PM, Liam R. Howlett wrote: > * Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> [170727 02:18]: >> For ppc64, we want to call this function when we are not running as guest. >> Also, if we failed to allocate hugepages, let the user know. >> > [...] >> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >> index bc48ee783dd9..a3a7a7e6339e 100644 >> --- a/mm/hugetlb.c >> +++ b/mm/hugetlb.c >> @@ -2083,7 +2083,9 @@ struct page *alloc_huge_page_noerr(struct vm_area_struct *vma, >> return page; >> } >> >> -int __weak alloc_bootmem_huge_page(struct hstate *h) >> +int alloc_bootmem_huge_page(struct hstate *h) >> + __attribute__ ((weak, alias("__alloc_bootmem_huge_page"))); >> +int __alloc_bootmem_huge_page(struct hstate *h) >> { >> struct huge_bootmem_page *m; >> int nr_nodes, node; >> @@ -2104,6 +2106,7 @@ int __weak alloc_bootmem_huge_page(struct hstate *h) >> goto found; >> } >> } >> + pr_info("Failed to allocate hugepage of size %ld\n", huge_page_size(h)); >> return 0; >> >> found: > > There is already a call to warn the user in the > hugetlb_hstate_alloc_pages function. If you look there, you will see > that the huge_page_size was translated into a more user friendly format > and the count prior to the failure is included. What call path are you > trying to cover? Also, you may want your print to be a pr_warn since it > is a failure? > Sorry I missed that in the recent kernel. I wrote the above before the mentioned changes was done. I will drop the pr_info from the patch. Thanks -aneesh
On 07/27/2017 06:31 PM, Michal Hocko wrote: > On Thu 27-07-17 11:48:26, Aneesh Kumar K.V wrote: >> For ppc64, we want to call this function when we are not running as guest. > > What does this mean? > ppc64 guest (aka LPAR) support a different mechanism for hugetlb allocation/reservation. The LPAR management application called HMC can be used to reserve a set of hugepages and we pass the details of reserved pages via device tree to the guest. You can find the details in htab_dt_scan_hugepage_blocks() . We do the memblock_reserve of the range and later in the boot sequence, we just add the reserved range to huge_boot_pages. For baremetal config (when we are not running as guest) we want to follow what other architecture does, that is look at the command line and do memblock allocation. Hence the need to call generic function __alloc_bootmem_huge_page() in that case. I can add all these details in to the commit message if that makes it easy ? -aneesh
* Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> [170727 12:12]: > > > On 07/27/2017 08:55 PM, Liam R. Howlett wrote: > > * Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> [170727 02:18]: > > > For ppc64, we want to call this function when we are not running as guest. > > > Also, if we failed to allocate hugepages, let the user know. > > > > > [...] > > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > > > index bc48ee783dd9..a3a7a7e6339e 100644 > > > --- a/mm/hugetlb.c > > > +++ b/mm/hugetlb.c > > > @@ -2083,7 +2083,9 @@ struct page *alloc_huge_page_noerr(struct vm_area_struct *vma, > > > return page; > > > } > > > -int __weak alloc_bootmem_huge_page(struct hstate *h) > > > +int alloc_bootmem_huge_page(struct hstate *h) > > > + __attribute__ ((weak, alias("__alloc_bootmem_huge_page"))); > > > +int __alloc_bootmem_huge_page(struct hstate *h) > > > { > > > struct huge_bootmem_page *m; > > > int nr_nodes, node; > > > @@ -2104,6 +2106,7 @@ int __weak alloc_bootmem_huge_page(struct hstate *h) > > > goto found; > > > } > > > } > > > + pr_info("Failed to allocate hugepage of size %ld\n", huge_page_size(h)); > > > return 0; > > > found: > > > > There is already a call to warn the user in the > > hugetlb_hstate_alloc_pages function. If you look there, you will see > > that the huge_page_size was translated into a more user friendly format > > and the count prior to the failure is included. What call path are you > > trying to cover? Also, you may want your print to be a pr_warn since it > > is a failure? > > > > Sorry I missed that in the recent kernel. I wrote the above before the > mentioned changes was done. I will drop the pr_info from the patch. Okay, thanks. I didn't think there was a code path that was missed on boot. Cheers, Liam
On Thu 27-07-17 21:50:35, Aneesh Kumar K.V wrote: > > > On 07/27/2017 06:31 PM, Michal Hocko wrote: > >On Thu 27-07-17 11:48:26, Aneesh Kumar K.V wrote: > >>For ppc64, we want to call this function when we are not running as guest. > > > >What does this mean? > > > > ppc64 guest (aka LPAR) support a different mechanism for hugetlb > allocation/reservation. The LPAR management application called HMC can be > used to reserve a set of hugepages and we pass the details of reserved pages > via device tree to the guest. You can find the details in > htab_dt_scan_hugepage_blocks() . We do the memblock_reserve of the range and > later in the boot sequence, we just add the reserved range to > huge_boot_pages. > > For baremetal config (when we are not running as guest) we want to follow > what other architecture does, that is look at the command line and do > memblock allocation. Hence the need to call generic function > __alloc_bootmem_huge_page() in that case. > > I can add all these details in to the commit message if that makes it easy ? It certainly helped me to understand the context much better. Thanks! As you are patching a generic code this would be appropriate IMHO.
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h index 0ed8e41aaf11..8bbbd37ab105 100644 --- a/include/linux/hugetlb.h +++ b/include/linux/hugetlb.h @@ -358,6 +358,7 @@ int huge_add_to_page_cache(struct page *page, struct address_space *mapping, pgoff_t idx); /* arch callback */ +int __init __alloc_bootmem_huge_page(struct hstate *h); int __init alloc_bootmem_huge_page(struct hstate *h); void __init hugetlb_bad_size(void); diff --git a/mm/hugetlb.c b/mm/hugetlb.c index bc48ee783dd9..a3a7a7e6339e 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -2083,7 +2083,9 @@ struct page *alloc_huge_page_noerr(struct vm_area_struct *vma, return page; } -int __weak alloc_bootmem_huge_page(struct hstate *h) +int alloc_bootmem_huge_page(struct hstate *h) + __attribute__ ((weak, alias("__alloc_bootmem_huge_page"))); +int __alloc_bootmem_huge_page(struct hstate *h) { struct huge_bootmem_page *m; int nr_nodes, node; @@ -2104,6 +2106,7 @@ int __weak alloc_bootmem_huge_page(struct hstate *h) goto found; } } + pr_info("Failed to allocate hugepage of size %ld\n", huge_page_size(h)); return 0; found:
For ppc64, we want to call this function when we are not running as guest. Also, if we failed to allocate hugepages, let the user know. Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> --- include/linux/hugetlb.h | 1 + mm/hugetlb.c | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-)