linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hugh Dickins <hughd@google.com>
To: Mike Rapoport <rppt@kernel.org>
Cc: Andrea Arcangeli <aarcange@redhat.com>,
	linux-kernel@vger.kernel.org, Alan Cox <alan@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andy Lutomirski <luto@kernel.org>,
	Christopher Lameter <cl@linux.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Idan Yaniv <idan.yaniv@ibm.com>,
	James Bottomley <jejb@linux.ibm.com>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Matthew Wilcox <willy@infradead.org>,
	Peter Zijlstra <peterz@infradead.org>,
	"Reshetova, Elena" <elena.reshetova@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Tycho Andersen <tycho@tycho.ws>,
	linux-api@vger.kernel.org, linux-mm@kvack.org,
	Mike Rapoport <rppt@linux.ibm.com>
Subject: Re: [RFC PATCH v2 1/5] mm: make HPAGE_PxD_{SHIFT,MASK,SIZE} always available
Date: Mon, 6 Jul 2020 22:07:34 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LSU.2.11.2007062153000.2793@eggly.anvils> (raw)
In-Reply-To: <20200706172051.19465-2-rppt@kernel.org>

On Mon, 6 Jul 2020, Mike Rapoport wrote:
> From: Mike Rapoport <rppt@linux.ibm.com>
> 
> The definitions of shift, mask and size for the second and the third level
> of the leaf pages are available only when CONFIG_TRANSPARENT_HUGEPAGE is
> set. Otherwise they evaluate to BUILD_BUG().
> 
> There is no explanation neither in the code nor in the changelog why the
> usage of, e.g. HPAGE_PMD_SIZE should be only allowed with THP and forbidden
> otherwise while the definitions of HPAGE_PMD_SIZE and HPAGE_PUD_SIZE
> express the sizes better than ambiguous HPAGE_SIZE.
> 
> Make HPAGE_PxD_{SHIFT,MASK,SIZE} definitions available unconditionally.

Adding Andrea to Cc, he's the one who structured it that way,
and should be consulted.

I'm ambivalent myself.  Many's the time I've been irritated by the
BUILD_BUG() in HPAGE_etc, and it's responsible for very many #ifdef
CONFIG_TRANSPARENT_HUGEPAGEs or IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)s
that you find uglily scattered around the source.

But that's the point of it: it's warning when you write code peculiar
to THP, that is going to bloat the build of kernels without any THP.

So although I've often been tempted to do as you suggest, I've always
ended up respecting Andrea's intention, and worked around it instead
(sometimes with #ifdef or IS_ENABLED(), sometimes with
PMD_{SHIFT,MASK_SIZE}, sometimes with a local definition).

Hugh

> 
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
>  include/linux/huge_mm.h | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 71f20776b06c..1f4b44a76e31 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -115,7 +115,6 @@ extern struct kobj_attribute shmem_enabled_attr;
>  #define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
>  #define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
>  
> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  #define HPAGE_PMD_SHIFT PMD_SHIFT
>  #define HPAGE_PMD_SIZE	((1UL) << HPAGE_PMD_SHIFT)
>  #define HPAGE_PMD_MASK	(~(HPAGE_PMD_SIZE - 1))
> @@ -124,6 +123,8 @@ extern struct kobj_attribute shmem_enabled_attr;
>  #define HPAGE_PUD_SIZE	((1UL) << HPAGE_PUD_SHIFT)
>  #define HPAGE_PUD_MASK	(~(HPAGE_PUD_SIZE - 1))
>  
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +
>  extern unsigned long transparent_hugepage_flags;
>  
>  /*
> @@ -316,13 +317,6 @@ static inline struct list_head *page_deferred_list(struct page *page)
>  }
>  
>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
> -#define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
> -#define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
> -#define HPAGE_PMD_SIZE ({ BUILD_BUG(); 0; })
> -
> -#define HPAGE_PUD_SHIFT ({ BUILD_BUG(); 0; })
> -#define HPAGE_PUD_MASK ({ BUILD_BUG(); 0; })
> -#define HPAGE_PUD_SIZE ({ BUILD_BUG(); 0; })
>  
>  static inline int hpage_nr_pages(struct page *page)
>  {
> -- 
> 2.26.2

  reply	other threads:[~2020-07-07  5:07 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06 17:20 [RFC PATCH v2 0/5] mm: extend memfd with ability to create "secret" memory areas Mike Rapoport
2020-07-06 17:20 ` [RFC PATCH v2 1/5] mm: make HPAGE_PxD_{SHIFT,MASK,SIZE} always available Mike Rapoport
2020-07-07  5:07   ` Hugh Dickins [this message]
2020-07-07  6:47     ` Mike Rapoport
2020-07-10 16:40     ` Andrea Arcangeli
2020-07-10 16:57       ` Matthew Wilcox
2020-07-10 17:08         ` Andrea Arcangeli
2020-07-10 17:12         ` Mike Rapoport
2020-07-06 17:20 ` [RFC PATCH v2 2/5] mmap: make mlock_future_check() global Mike Rapoport
2020-07-06 17:20 ` [RFC PATCH v2 3/5] mm: extend memfd with ability to create "secret" memory areas Mike Rapoport
2020-07-13 10:58   ` Kirill A. Shutemov
2020-07-13 15:31     ` Mike Rapoport
2020-07-06 17:20 ` [RFC PATCH v2 4/5] mm: secretmem: use PMD-size pages to amortize direct map fragmentation Mike Rapoport
2020-07-13 11:05   ` Kirill A. Shutemov
2020-07-13 15:32     ` Mike Rapoport
2020-07-06 17:20 ` [RFC PATCH v2 5/5] mm: secretmem: add ability to reserve memory at boot Mike Rapoport
2020-07-17  8:36 ` [RFC PATCH v2 0/5] mm: extend memfd with ability to create "secret" memory areas Pavel Machek
2020-07-17 14:43   ` James Bottomley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LSU.2.11.2007062153000.2793@eggly.anvils \
    --to=hughd@google.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@linux.intel.com \
    --cc=cl@linux.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=elena.reshetova@intel.com \
    --cc=idan.yaniv@ibm.com \
    --cc=jejb@linux.ibm.com \
    --cc=kirill@shutemov.name \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rppt@kernel.org \
    --cc=rppt@linux.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=tycho@tycho.ws \
    --cc=willy@infradead.org \
    --subject='Re: [RFC PATCH v2 1/5] mm: make HPAGE_PxD_{SHIFT,MASK,SIZE} always available' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).