Re: [External] Re: [PATCH v8 10/12] mm/hugetlb: Introduce nr_free_vmemmap_pages in the struct hstate

[Muchun Song]

On Thu, Dec 10, 2020 at 7:40 PM Oscar Salvador <osalvador@xxxxxxx> 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