Messages in this thread | | | From | Muchun Song <> | Date | Thu, 10 Dec 2020 20:32:06 +0800 | Subject | Re: [External] Re: [PATCH v8 10/12] mm/hugetlb: Introduce nr_free_vmemmap_pages in the struct hstate |
| |
On Thu, Dec 10, 2020 at 7:40 PM Oscar Salvador <osalvador@suse.de> wrote: > > On Thu, Dec 10, 2020 at 11:55:24AM +0800, Muchun Song wrote: > > +void __init hugetlb_vmemmap_init(struct hstate *h) > > +{ > > + unsigned int nr_pages = pages_per_huge_page(h); > > + unsigned int vmemmap_pages; > > + > > + /* We cannot optimize if a "struct page" crosses page boundaries. */ > > + if (!is_power_of_2(sizeof(struct page))) > > + return; > > + > > + if (!hugetlb_free_vmemmap_enabled) > > + return; > > I think it would make sense to squash the last patch and this one. > As per the last patch, if "struct page" is not power of 2, > early_hugetlb_free_vmemmap_param() does not set > hugetlb_free_vmemmap_enabled, so the "!is_power_of_2" check from above > would become useless here. > We know that in order for hugetlb_free_vmemmap_enabled to become true, > the is_power_of_2 must have succeed early on when calling the early_ > function.
Yeah, you are right. But if is_power_of_2 returns false. The compiler can optimize this function to null. If we remove the check, it prevents the compiler from optimizing the code of hugetlb_vmemmap_init(). So I think leaving it here makes sense. Right?
> > -- > Oscar Salvador > SUSE L3
-- Yours, Muchun
| |