| Message ID | 20210917034815.80264-3-songmuchun@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Free the 2nd vmemmap page associated with each HugeTLB page | expand |
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? > return page; > > /* > diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c > index 527bcaa44a48..5b80129c684c 100644 > --- a/mm/hugetlb_vmemmap.c > +++ b/mm/hugetlb_vmemmap.c > @@ -188,9 +188,9 @@ > #define RESERVE_VMEMMAP_NR 1U > #define RESERVE_VMEMMAP_SIZE (RESERVE_VMEMMAP_NR << PAGE_SHIFT) > > -bool hugetlb_free_vmemmap_enabled __read_mostly = > - IS_ENABLED(CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON); > -EXPORT_SYMBOL(hugetlb_free_vmemmap_enabled); > +DEFINE_STATIC_KEY_MAYBE(CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON, > + hugetlb_free_vmemmap_enabled_key); > +EXPORT_SYMBOL(hugetlb_free_vmemmap_enabled_key); > > static int __init early_hugetlb_free_vmemmap_param(char *buf) > { > @@ -204,9 +204,9 @@ static int __init early_hugetlb_free_vmemmap_param(char *buf) > return -EINVAL; > > if (!strcmp(buf, "on")) > - hugetlb_free_vmemmap_enabled = true; > + static_branch_enable(&hugetlb_free_vmemmap_enabled_key); > else if (!strcmp(buf, "off")) > - hugetlb_free_vmemmap_enabled = false; > + static_branch_disable(&hugetlb_free_vmemmap_enabled_key); > else > return -EINVAL; > > -- > 2.11.0 > Thanks barry
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.
On Sat, Sep 18, 2021 at 10:31 PM Muchun Song <songmuchun@bytedance.com> wrote: > > 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. not quite sure if an unified inline API will be better, e.g. #ifdef CONFIG_SCHED_SMT extern struct static_key_false sched_smt_present; static __always_inline bool sched_smt_active(void) { return static_branch_likely(&sched_smt_present); } #else static inline bool sched_smt_active(void) { return false; } #endif but in your case, CONFIG_HUGETLB_PAGE_FREE_VMEMMAP is always true in your page_head_if_fake(). Why do we check it again? > > Thanks. Thanks barry
On Sat, Sep 18, 2021 at 7:15 PM Barry Song <21cnbao@gmail.com> wrote: > > On Sat, Sep 18, 2021 at 10:31 PM Muchun Song <songmuchun@bytedance.com> wrote: > > > > 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. > > not quite sure if an unified inline API will be better, e.g. > > #ifdef CONFIG_SCHED_SMT > extern struct static_key_false sched_smt_present; > > static __always_inline bool sched_smt_active(void) > { > return static_branch_likely(&sched_smt_present); > } > #else > static inline bool sched_smt_active(void) { return false; } > #endif Alright, I can change hugetlb_free_vmemmap_enabled to an inline function. > > but in your case, CONFIG_HUGETLB_PAGE_FREE_VMEMMAP > is always true in your page_head_if_fake(). Why do we check it > again? That is CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON not CONFIG_HUGETLB_PAGE_FREE_VMEMMAP. Thanks
On Sat, Sep 18, 2021 at 11:48 PM Muchun Song <songmuchun@bytedance.com> wrote: > > On Sat, Sep 18, 2021 at 7:15 PM Barry Song <21cnbao@gmail.com> wrote: > > > > On Sat, Sep 18, 2021 at 10:31 PM Muchun Song <songmuchun@bytedance.com> wrote: > > > > > > 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. > > > > not quite sure if an unified inline API will be better, e.g. > > > > #ifdef CONFIG_SCHED_SMT > > extern struct static_key_false sched_smt_present; > > > > static __always_inline bool sched_smt_active(void) > > { > > return static_branch_likely(&sched_smt_present); > > } > > #else > > static inline bool sched_smt_active(void) { return false; } > > #endif > > Alright, I can change hugetlb_free_vmemmap_enabled to > an inline function. > > > > > but in your case, CONFIG_HUGETLB_PAGE_FREE_VMEMMAP > > is always true in your page_head_if_fake(). Why do we check it > > again? > > That is CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON > not CONFIG_HUGETLB_PAGE_FREE_VMEMMAP. oops, sorry for missing that. > > Thanks
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)) return page; /* diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c index 527bcaa44a48..5b80129c684c 100644 --- a/mm/hugetlb_vmemmap.c +++ b/mm/hugetlb_vmemmap.c @@ -188,9 +188,9 @@ #define RESERVE_VMEMMAP_NR 1U #define RESERVE_VMEMMAP_SIZE (RESERVE_VMEMMAP_NR << PAGE_SHIFT) -bool hugetlb_free_vmemmap_enabled __read_mostly = - IS_ENABLED(CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON); -EXPORT_SYMBOL(hugetlb_free_vmemmap_enabled); +DEFINE_STATIC_KEY_MAYBE(CONFIG_HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON, + hugetlb_free_vmemmap_enabled_key); +EXPORT_SYMBOL(hugetlb_free_vmemmap_enabled_key); static int __init early_hugetlb_free_vmemmap_param(char *buf) { @@ -204,9 +204,9 @@ static int __init early_hugetlb_free_vmemmap_param(char *buf) return -EINVAL; if (!strcmp(buf, "on")) - hugetlb_free_vmemmap_enabled = true; + static_branch_enable(&hugetlb_free_vmemmap_enabled_key); else if (!strcmp(buf, "off")) - hugetlb_free_vmemmap_enabled = false; + static_branch_disable(&hugetlb_free_vmemmap_enabled_key); else return -EINVAL;
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(-)