Re: [External] Re: [PATCH v8 12/12] mm/hugetlb: Optimize the code with the help of the compiler

[Muchun Song <songmuchun@xxxxxxxxxxxxx>]

On Thu, Dec 10, 2020 at 7:39 PM Oscar Salvador <osalvador@xxxxxxx> wrote:
>
> On 2020-12-10 04:55, Muchun Song wrote:
> > We cannot optimize if a "struct page" crosses page boundaries. If
> > it is true, we can optimize the code with the help of a compiler.
> > When free_vmemmap_pages_per_hpage() returns zero, most functions are
> > optimized by the compiler.
>
> As I said earlier, I would squash this patch with patch#10 and
> remove the !is_power_of_2 check in hugetlb_vmemmap_init and leave
> only the check for the boot parameter.
> That should be enough.

Yeah, you are right. I just want the compiler to do optimization.

>
> >  static inline bool is_hugetlb_free_vmemmap_enabled(void)
> >  {
> > -     return hugetlb_free_vmemmap_enabled;
> > +     return hugetlb_free_vmemmap_enabled &&
> > +            is_power_of_2(sizeof(struct page));
>
> Why? hugetlb_free_vmemmap_enabled can only become true
> if the is_power_of_2 check succeeds in early_hugetlb_free_vmemmap_param.
> The "is_power_of_2" check here can go.
>
> > diff --git a/mm/hugetlb_vmemmap.h b/mm/hugetlb_vmemmap.h
> > index 0a1c0d33a316..5f5e90c81cd2 100644
> > --- a/mm/hugetlb_vmemmap.h
> > +++ b/mm/hugetlb_vmemmap.h
> > @@ -21,7 +21,7 @@ void free_huge_page_vmemmap(struct hstate *h, struct
> > page *head);
> >   */
> >  static inline unsigned int free_vmemmap_pages_per_hpage(struct hstate
> > *h)
> >  {
> > -     return h->nr_free_vmemmap_pages;
> > +     return h->nr_free_vmemmap_pages && is_power_of_2(sizeof(struct
> > page));
>
> If hugetlb_free_vmemmap_enabled is false, hugetlb_vmemmap_init() leaves
> h->nr_free_vmemmap_pages unset to 0, so no need for the is_power_of_2
> check here.

Yeah, you are right. But if we do this check can make the code simple.

For example, here is a code snippet.

void func(void)
{
        if (free_vmemmap_pages_per_hpage())
                return;
        /* Do something */
}

With this patch, the func will be optimized to null when is_power_of_2
returns false.

void func(void)
{
}

Without this patch, the compiler cannot do this optimization.

Thanks.

>
>
> --
> Oscar Salvador
> SUSE L3



-- 
Yours,
Muchun