lkml.org 
[lkml]   [2021]   [Sep]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH RESEND v2 2/4] mm: hugetlb: replace hugetlb_free_vmemmap_enabled with a static_key
On Sat, Sep 18, 2021 at 12:55 PM Barry Song <21cnbao@gmail.com> wrote:
>
> On Sat, Sep 18, 2021 at 12:08 AM Muchun Song <songmuchun@bytedance.com> wrote:
> >
> > The page_head_if_fake() is used throughout memory management and the
> > conditional check requires checking a global variable, although the
> > overhead of this check may be small, it increases when the memory
> > cache comes under pressure. Also, the global variable will not be
> > modified after system boot, so it is very appropriate to use static
> > key machanism.
> >
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > ---
> > include/linux/hugetlb.h | 6 +++++-
> > include/linux/page-flags.h | 6 ++++--
> > mm/hugetlb_vmemmap.c | 10 +++++-----
> > 3 files changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> > index f7ca1a3870ea..ee3ddf3d12cf 100644
> > --- a/include/linux/hugetlb.h
> > +++ b/include/linux/hugetlb.h
> > @@ -1057,7 +1057,11 @@ static inline void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr
> > #endif /* CONFIG_HUGETLB_PAGE */
> >
> > #ifdef CONFIG_HUGETLB_PAGE_FREE_VMEMMAP
> > -extern bool hugetlb_free_vmemmap_enabled;
> > +DECLARE_STATIC_KEY_MAYBE(CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON,
> > + hugetlb_free_vmemmap_enabled_key);
> > +#define hugetlb_free_vmemmap_enabled \
> > + static_key_enabled(&hugetlb_free_vmemmap_enabled_key)
> > +
> > #else
> > #define hugetlb_free_vmemmap_enabled false
> > #endif
> > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > index 7b1a918ebd43..d68d2cf30d76 100644
> > --- a/include/linux/page-flags.h
> > +++ b/include/linux/page-flags.h
> > @@ -185,7 +185,8 @@ enum pageflags {
> > #ifndef __GENERATING_BOUNDS_H
> >
> > #ifdef CONFIG_HUGETLB_PAGE_FREE_VMEMMAP
> > -extern bool hugetlb_free_vmemmap_enabled;
> > +DECLARE_STATIC_KEY_MAYBE(CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON,
> > + hugetlb_free_vmemmap_enabled_key);
> >
> > /*
> > * If the feature of freeing some vmemmap pages associated with each HugeTLB
> > @@ -204,7 +205,8 @@ extern bool hugetlb_free_vmemmap_enabled;
> > */
> > static __always_inline const struct page *page_head_if_fake(const struct page *page)
> > {
> > - if (!hugetlb_free_vmemmap_enabled)
> > + if (!static_branch_maybe(CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON,
> > + &hugetlb_free_vmemmap_enabled_key))
>
> A question bothering me is that we still have hugetlb_free_vmemmap_enabled
> defined as static_key_enabled(&hugetlb_free_vmemmap_enabled_key).
> but here you are using static_branch_maybe() with the CONFIG and refer the key
> directly.
> Do we only need one of them? Or something is wrong?
>

Yeah, we only need one. But my consideration is that we
use static_branch_maybe() for performance sensitive places.
So I do not change hugetlb_free_vmemmap_enabled
to static_branch_maybe(), this can reduce some codes
that need to be updated when the static key is enabled.
Actually, the user of hugetlb_free_vmemmap_enabled
is not performance sensitive.

Thanks.

\
 
 \ /
  Last update: 2021-09-18 12:32    [W:0.082 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site