linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Andrea Arcangeli <aarcange@redhat.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	"Luck, Tony" <tony.luck@intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-arch@vger.kernel.org
Subject: Re: ia64 broken by transparent huge pages - other arches too?
Date: Sat, 15 Jan 2011 10:47:28 -0600	[thread overview]
Message-ID: <1295110048.5973.36.camel@mulgrave.site> (raw)
In-Reply-To: <20110115155925.GT9506@random.random>

On Sat, 2011-01-15 at 16:59 +0100, Andrea Arcangeli wrote:
> On Sat, Jan 15, 2011 at 06:21:36PM +1100, Benjamin Herrenschmidt wrote:
> > This is insane. Having such a massively invasive change to the whole mm,
> > barely tested on most architecture, and last I heard still generally
> > controversial being merged like that without even some integration
> > testing via -next makes no sense.
> 
> This is 99% a noop for all archs but x86.. Really if you worry about
> the testing you should worry about x86 only! Only x86 is affected by
> the brainer part of the code, and only if TRANSPARENT_HUGEPAGE=y
> (which is set to N by default).
> 
> Not x86 archs can't possibly have a regression because of this. The
> reason there's this compile trouble is that I cleaned up some bad code
> in include/asm-generic/pgtable.h while adding the pmd methods, and I
> tried to keep everything as a static inline as requested by Mel for
> better gcc compile time validation than what the preprocessor can
> do. Otherwise if it was a macro I may not have had to return
> anything and I could just BUG() in this pmd method that requires the
> __pmd macro to be implemented by all archs (I think it's better off if
> __pmd is available considering __pte seems already available).
> 
> The below can't introduce regressions, if it builds it'll work, so the
> testing on -next for the other archs isn't really necessary at all. I
> don't think you can worry about a one liner change to make ia64 build,
> when the brainer part of the code is a noop for the other archs
> (including x86 when the config option is off).

Andrea, what you say above isn't true, you're not thinking broadly
enough: the kernel is a complex set of code interactions.  For instance,
you caused this build break on parisc (which is a regression even though
parisc has no transparent huge pages at all):

http://marc.info/?l=linux-arch&m=129504371532124

That was just from a simple code rearrangement (independent of any of
the config options).

One of the points of getting stuff through linux-next is that all of
these problems get sorted out long before the code hits mainline.  This
happens because linux-next gets a wide range of config randomisation
testing plus quite a few different architecture builds and runs.

The problem is very often not in the actual code, but in the side
effects the code causes.  This is what linux-next integration helps
mitigate. So, please use it next time.  Just testing on x86 in your own
configuration isn't sufficient to pick up the problems.

James

  reply	other threads:[~2011-01-15 16:47 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-14 17:50 ia64 broken by transparent huge pages - other arches too? Luck, Tony
2011-01-14 17:50 ` Luck, Tony
2011-01-14 18:30 ` Andrea Arcangeli
2011-01-14 18:50   ` Tony Luck
2011-01-14 19:03     ` Andrea Arcangeli
2011-01-15  7:21 ` Benjamin Herrenschmidt
2011-01-15 15:59   ` Andrea Arcangeli
2011-01-15 16:47     ` James Bottomley [this message]
2011-01-15 17:23       ` Andrea Arcangeli
2011-01-15 19:02         ` Geert Uytterhoeven
2011-01-15 21:31         ` Benjamin Herrenschmidt
2011-01-16 21:05   ` Linus Torvalds
2011-01-16 21:10     ` Andrew Morton
2011-01-16 22:06       ` Andrea Arcangeli

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=1295110048.5973.36.camel@mulgrave.site \
    --to=james.bottomley@hansenpartnership.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tony.luck@intel.com \
    --cc=torvalds@linux-foundation.org \
    /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;
as well as URLs for NNTP newsgroup(s).