From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DF28EC433FE for ; Sat, 18 Sep 2021 11:15:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BD2BB61251 for ; Sat, 18 Sep 2021 11:15:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238903AbhIRLQf (ORCPT ); Sat, 18 Sep 2021 07:16:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58730 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235543AbhIRLQc (ORCPT ); Sat, 18 Sep 2021 07:16:32 -0400 Received: from mail-ed1-x535.google.com (mail-ed1-x535.google.com [IPv6:2a00:1450:4864:20::535]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EA155C061574; Sat, 18 Sep 2021 04:15:08 -0700 (PDT) Received: by mail-ed1-x535.google.com with SMTP id c22so40018548edn.12; Sat, 18 Sep 2021 04:15:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hhBkqFl9K1TvuEPZxNRIrEeIc4UVq6Zdqs07eDkUW5Y=; b=DMe+5i2O8BkVUhUwEcr5rpW2w02cGac1v9nLYWXMVnF6r9GjHW8xTD31282esI9/F5 hahlmbR4IfxLKWBMqDYMPC1TR+I5qCQZFLM+AMDMcM2jfFM0zJnX+AVT1JfGCINA6213 LbrWCkIGVM67C3+M2Emiv/2wuI9KCs4G8G8lRwzYgz8jK7evDExt592lTkLp8RrmJ7wH rjFS9Q7OiTmJQKG5HHnJ83BNYMw8vmxCgEJS8oujAlAzJPehErDJP8ONYIKyOPvmhMIb 9Ck/CDOpfScZ2ZRIDlMYCyJ4tIJbsawt0MsFpIpE/lxWeTYG0tVE94pJstBvRrfbD/LI QUkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hhBkqFl9K1TvuEPZxNRIrEeIc4UVq6Zdqs07eDkUW5Y=; b=oH9AbNTJAy4SP5q7daXx9T7w1OcMjOVFZtT6wjPDAr8NlxJsdlZCLMwCOOQXFn7as3 NMvfGOJEtQxTSlRt+qL6e2ZCCruPl6cdDZU/VMg6UIq//cze/hAovxzRZK6TplcMn2ct OOuKrC6MIZ8KKkcwyM0BoAW9cWrpwi9nmjmajvBpi1/goT3Le3RsbF03coKHRBP+euOE K5aXrAon2OJnyXW1icJcWLDCF4NWSmz6FX2N1BMUP2WkAukDkS69/aImBUhJkLGDQECw KP4+OkcrEHp8oPdp4KidUd7etm3obPt/MhjQgQ7mnv5OLVD9XQlLrKvLkIS3kN/oV96J U6Gw== X-Gm-Message-State: AOAM531ZyJJqvFRxq30IYJbomV3JT1NAiyun0XKhgSfJUeSS/MNfQdWM xdsf1w2nnFNtoW6HcQQbLW/01evxHpD2MSJhfII= X-Google-Smtp-Source: ABdhPJwKdMhl8Jf4wHEUO3SuwBmvjG+48GphqJaicwEstAJlhgICwp/s15WSokVWIyN0XFwO7fmUKYXaOx2EWvgiUu0= X-Received: by 2002:a17:906:dbe5:: with SMTP id yd5mr17297002ejb.134.1631963707450; Sat, 18 Sep 2021 04:15:07 -0700 (PDT) MIME-Version: 1.0 References: <20210917034815.80264-1-songmuchun@bytedance.com> <20210917034815.80264-3-songmuchun@bytedance.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Sat, 18 Sep 2021 23:14:56 +1200 Message-ID: Subject: Re: [PATCH RESEND v2 2/4] mm: hugetlb: replace hugetlb_free_vmemmap_enabled with a static_key To: Muchun Song Cc: Mike Kravetz , Andrew Morton , Oscar Salvador , Michal Hocko , Barry Song , David Hildenbrand , Chen Huang , "Bodeddula, Balasubramaniam" , Jonathan Corbet , Matthew Wilcox , Xiongchun duan , fam.zheng@bytedance.com, Muchun Song , Qi Zheng , linux-doc@vger.kernel.org, LKML , Linux-MM Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Sep 18, 2021 at 10:31 PM Muchun Song 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 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 > > > --- > > > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7B410C433F5 for ; Sat, 18 Sep 2021 11:15:10 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 12D85611C8 for ; Sat, 18 Sep 2021 11:15:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 12D85611C8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 8D0D46B0071; Sat, 18 Sep 2021 07:15:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 87E9F6B0072; Sat, 18 Sep 2021 07:15:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 76D956B0073; Sat, 18 Sep 2021 07:15:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0109.hostedemail.com [216.40.44.109]) by kanga.kvack.org (Postfix) with ESMTP id 669F36B0071 for ; Sat, 18 Sep 2021 07:15:09 -0400 (EDT) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 22B7D2DD7A for ; Sat, 18 Sep 2021 11:15:09 +0000 (UTC) X-FDA: 78600437538.03.D98B40B Received: from mail-ed1-f54.google.com (mail-ed1-f54.google.com [209.85.208.54]) by imf21.hostedemail.com (Postfix) with ESMTP id CCFE9D030D78 for ; Sat, 18 Sep 2021 11:15:08 +0000 (UTC) Received: by mail-ed1-f54.google.com with SMTP id g21so40123718edw.4 for ; Sat, 18 Sep 2021 04:15:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hhBkqFl9K1TvuEPZxNRIrEeIc4UVq6Zdqs07eDkUW5Y=; b=DMe+5i2O8BkVUhUwEcr5rpW2w02cGac1v9nLYWXMVnF6r9GjHW8xTD31282esI9/F5 hahlmbR4IfxLKWBMqDYMPC1TR+I5qCQZFLM+AMDMcM2jfFM0zJnX+AVT1JfGCINA6213 LbrWCkIGVM67C3+M2Emiv/2wuI9KCs4G8G8lRwzYgz8jK7evDExt592lTkLp8RrmJ7wH rjFS9Q7OiTmJQKG5HHnJ83BNYMw8vmxCgEJS8oujAlAzJPehErDJP8ONYIKyOPvmhMIb 9Ck/CDOpfScZ2ZRIDlMYCyJ4tIJbsawt0MsFpIpE/lxWeTYG0tVE94pJstBvRrfbD/LI QUkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hhBkqFl9K1TvuEPZxNRIrEeIc4UVq6Zdqs07eDkUW5Y=; b=ul9W3OuTljdDrsygD7VD57XzXIpIiX3qplGlL9xTek5uc3vLD/yTMO1lDTBMI0p5IX rs7lbnCSnDxKAyoVN+VMJVmI2UGlnNwv252brG0kyIGhPOFSjaGlahg/G+DDRCN6OBbE sAgoHtioDhw4aLDf4T5bCUpzalaAF51V9yejPm3peORyUbbq32EvghXxKvgVaSM1f3bp 47v6q8E7if+7n3tHurFZMx25Pqe3aDHYJXYc7l//3P7AxAyDmYTnIp7tEOWpqnteH3fG fHnXKGzcFWZzvk+a4HIwizYzD/vBidAVPaeqSGKldE7iFzktvjDDJcAdb9XSu7y2a5Bx mbsw== X-Gm-Message-State: AOAM533r81CMdRUgL/v5bq9sXaA0ZzkfatUCIXHvIYo7YyDVxfI+uvES SPGc1xIHM7A5BCSmO27Af4oUBH8smJVTVVAdW7Y= X-Google-Smtp-Source: ABdhPJwKdMhl8Jf4wHEUO3SuwBmvjG+48GphqJaicwEstAJlhgICwp/s15WSokVWIyN0XFwO7fmUKYXaOx2EWvgiUu0= X-Received: by 2002:a17:906:dbe5:: with SMTP id yd5mr17297002ejb.134.1631963707450; Sat, 18 Sep 2021 04:15:07 -0700 (PDT) MIME-Version: 1.0 References: <20210917034815.80264-1-songmuchun@bytedance.com> <20210917034815.80264-3-songmuchun@bytedance.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Sat, 18 Sep 2021 23:14:56 +1200 Message-ID: Subject: Re: [PATCH RESEND v2 2/4] mm: hugetlb: replace hugetlb_free_vmemmap_enabled with a static_key To: Muchun Song Cc: Mike Kravetz , Andrew Morton , Oscar Salvador , Michal Hocko , Barry Song , David Hildenbrand , Chen Huang , "Bodeddula, Balasubramaniam" , Jonathan Corbet , Matthew Wilcox , Xiongchun duan , fam.zheng@bytedance.com, Muchun Song , Qi Zheng , linux-doc@vger.kernel.org, LKML , Linux-MM Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: CCFE9D030D78 X-Stat-Signature: 9ya1ib118m4e8s38f5peg5x4rdtgru3k Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=DMe+5i2O; spf=pass (imf21.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.208.54 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-HE-Tag: 1631963708-296880 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat, Sep 18, 2021 at 10:31 PM Muchun Song 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 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 > > > --- > > > 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