linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Alexandre Ghiti <aghiti@upmem.com>
Cc: Michal Hocko <mhocko@kernel.org>,
	linux-mm@kvack.org, kirill.shutemov@linux.intel.com,
	dan.j.williams@intel.com, zi.yan@cs.rutgers.edu,
	gregkh@linuxfoundation.org, n-horiguchi@ah.jp.nec.com,
	willy@linux.intel.com, mark.rutland@arm.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm, THP: vmf_insert_pfn_pud depends on CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
Date: Thu, 11 Jan 2018 16:28:25 -0800	[thread overview]
Message-ID: <20180111162825.4cdaba2a21d8f15b21c45c75@linux-foundation.org> (raw)
In-Reply-To: <71853228-0beb-1e69-df47-59fa1bc5bd2f@upmem.com>

On Thu, 11 Jan 2018 14:05:34 +0100 Alexandre Ghiti <aghiti@upmem.com> wrote:

> On 11/01/2018 11:06, Michal Hocko wrote:
> > On Thu 11-01-18 09:53:31, Alexandre Ghiti wrote:
> >> The only definition of vmf_insert_pfn_pud depends on
> >> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD being defined. Then its declaration in
> >> include/linux/huge_mm.h should have the same restriction so that we do
> >> not expose this function if CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is
> >> not defined.
> > Why is this a problem? Compiler should simply throw away any
> > declarations which are not used?
> It is not a big problem but surrounding the declaration with the #ifdef 
> makes the compilation of external modules fail with an "error: implicit 
> declaration of function vmf_insert_pfn_pud" if 
> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is not defined. I think it is 
> cleaner than generating a .ko which would not load anyway.

Disagree.  We'd have to put an absolutely vast amount of complex and
hard-to-maintain ifdefs in headers if we were to ensure that such
errors were to be detected at compile time.

Whereas if we defer the detection of the errors until link time (or
depmod or modprobe time) then yes, a handful of people will detect
their mistake a minute or three later but that's a small cost compared
to permanently and badly messing up the header files.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2018-01-12  0:28 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-11  8:53 Alexandre Ghiti
2018-01-11 10:06 ` Michal Hocko
2018-01-11 13:05   ` Alexandre Ghiti
2018-01-12  0:28     ` Andrew Morton [this message]
2018-01-12 15:26       ` Alexandre Ghiti

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=20180111162825.4cdaba2a21d8f15b21c45c75@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=aghiti@upmem.com \
    --cc=dan.j.williams@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mark.rutland@arm.com \
    --cc=mhocko@kernel.org \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=willy@linux.intel.com \
    --cc=zi.yan@cs.rutgers.edu \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox