ia64 broken by transparent huge pages - other arches too?

ia64 broken by transparent huge pages - other arches too?

[Luck, Tony]

Didn't Andrew make some rash promise at kernel summit about stopping
eating if "-mm" wasn't included in linux-next by the end of November?

Must be getting pretty hungry by now.

The transparent huge page code just arrived in the merge window
without having been in linux-next.  I see this error when trying
to build for ia64 from Linus' tree this morning:

In file included from /home/aegl/generic-smp/arch/ia64/include/asm/pgtable.h:611,
                 from include/linux/mm.h:41,
                 from /home/aegl/generic-smp/arch/ia64/include/asm/uaccess.h:39,
                 from include/linux/poll.h:14,
                 from include/linux/rtc.h:117,
                 from include/linux/efi.h:19,
                 from /home/aegl/generic-smp/arch/ia64/include/asm/sal.h:40,
                 from /home/aegl/generic-smp/arch/ia64/include/asm/mca.h:20,
                 from arch/ia64/kernel/asm-offsets.c:17:
include/asm-generic/pgtable.h: In function ‘pmdp_get_and_clear’:
include/asm-generic/pgtable.h:96: error: implicit declaration of function ‘__pmd’
include/asm-generic/pgtable.h:96: error: incompatible types in return
make[1]: *** [arch/ia64/kernel/asm-offsets.s] Error 1


Looks like arch/*/include/pgtable.h needs to define __pmd() but only x86
was blessed with it.

-Tony

ia64 broken by transparent huge pages - other arches too?

[Luck, Tony]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1299 bytes --]

Didn't Andrew make some rash promise at kernel summit about stopping
eating if "-mm" wasn't included in linux-next by the end of November?

Must be getting pretty hungry by now.

The transparent huge page code just arrived in the merge window
without having been in linux-next.  I see this error when trying
to build for ia64 from Linus' tree this morning:

In file included from /home/aegl/generic-smp/arch/ia64/include/asm/pgtable.h:611,
                 from include/linux/mm.h:41,
                 from /home/aegl/generic-smp/arch/ia64/include/asm/uaccess.h:39,
                 from include/linux/poll.h:14,
                 from include/linux/rtc.h:117,
                 from include/linux/efi.h:19,
                 from /home/aegl/generic-smp/arch/ia64/include/asm/sal.h:40,
                 from /home/aegl/generic-smp/arch/ia64/include/asm/mca.h:20,
                 from arch/ia64/kernel/asm-offsets.c:17:
include/asm-generic/pgtable.h: In function ‘pmdp_get_and_clear’:
include/asm-generic/pgtable.h:96: error: implicit declaration of function ‘__pmd’
include/asm-generic/pgtable.h:96: error: incompatible types in return
make[1]: *** [arch/ia64/kernel/asm-offsets.s] Error 1


Looks like arch/*/include/pgtable.h needs to define __pmd() but only x86
was blessed with it.

-Tony

Re: ia64 broken by transparent huge pages - other arches too?

[Andrea Arcangeli]

[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]

On Fri, Jan 14, 2011 at 09:50:45AM -0800, Luck, Tony wrote:
> In file included from /home/aegl/generic-smp/arch/ia64/include/asm/pgtable.h:611,
>                  from include/linux/mm.h:41,
>                  from /home/aegl/generic-smp/arch/ia64/include/asm/uaccess.h:39,
>                  from include/linux/poll.h:14,
>                  from include/linux/rtc.h:117,
>                  from include/linux/efi.h:19,
>                  from /home/aegl/generic-smp/arch/ia64/include/asm/sal.h:40,
>                  from /home/aegl/generic-smp/arch/ia64/include/asm/mca.h:20,
>                  from arch/ia64/kernel/asm-offsets.c:17:
> include/asm-generic/pgtable.h: In function ‘pmdp_get_and_clear’:
> include/asm-generic/pgtable.h:96: error: implicit declaration of function ‘__pmd’
> include/asm-generic/pgtable.h:96: error: incompatible types in return
> make[1]: *** [arch/ia64/kernel/asm-offsets.s] Error 1
> 
> 
> Looks like arch/*/include/pgtable.h needs to define __pmd() but only x86
> was blessed with it.

So I fixed powerpc build and sparc but broke ia64 with this change
sorry, hard to make all archs build consistent. See the attached mails.

Would you be willing to implement __pmd for ia64 to fix this? Can you
check if this works?

Thanks a lot,
Andrea

========
Subject: fix ia64 build failure in pmdp_get_and_clear

From: Andrea Arcangeli <aarcange@redhat.com>

Implement __pmd macro for ia64 too.

Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---

diff --git a/arch/ia64/include/asm/page.h b/arch/ia64/include/asm/page.h
index 41b6d31..961a16f 100644
--- a/arch/ia64/include/asm/page.h
+++ b/arch/ia64/include/asm/page.h
@@ -189,6 +189,7 @@ get_order (unsigned long size)
 # define pgprot_val(x)	((x).pgprot)
 
 # define __pte(x)	((pte_t) { (x) } )
+# define __pmd(x)	((pmd_t) { (x) } )
 # define __pgprot(x)	((pgprot_t) { (x) } )
 
 #else /* !STRICT_MM_TYPECHECKS */

[-- Attachment #2: Type: message/rfc822, Size: 1212 bytes --]

From: Andrea Arcangeli <aarcange@redhat.com>
To: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: [PATCH -mm] fix powerpc/sparc build
Date: Mon, 10 Jan 2011 19:04:25 +0100
Message-ID: <20110110180425.GK9506@random.random>

This would become
thp-add-pmd-mangling-generic-functions-fix-pgtableh-build-for-um-2.patch

=====
Subject: thp: build fix for pmdp_get_and_clear

From: Andrea Arcangeli <aarcange@redhat.com>

__pmd should return a valid pmd_t for every arch.

Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---


diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -93,7 +93,7 @@ static inline pmd_t pmdp_get_and_clear(s
 				       pmd_t *pmdp)
 {
 	BUG();
-	return (pmd_t){ 0 };
+	return __pmd(0);
 }
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 #endif


[-- Attachment #3: Type: message/rfc822, Size: 5024 bytes --]

[-- Attachment #3.1.1: Type: text/plain, Size: 1458 bytes --]

Hi Andrew,

On Thu, 06 Jan 2011 15:41:14 -0800 akpm@linux-foundation.org wrote:
>
> The mm-of-the-moment snapshot 2011-01-06-15-41 has been uploaded to
> 
>    http://userweb.kernel.org/~akpm/mmotm/

Build results here: http://kisskb.ellerman.id.au/kisskb/head/3605/

Notably:

powerpc pmac32_defconfig:

In file included from arch/powerpc/include/asm/pgtable.h:200,
                 from include/linux/mm.h:41,
                 from include/linux/mman.h:14,
                 from arch/powerpc/kernel/asm-offsets.c:22:
include/asm-generic/pgtable.h: In function 'pmdp_get_and_clear':
include/asm-generic/pgtable.h:96: warning: missing braces around initializer
include/asm-generic/pgtable.h:96: warning: (near initialization for '(anonymous).pud')

sparc defconfig:

In file included from arch/sparc/include/asm/pgtable_32.h:456,
                 from arch/sparc/include/asm/pgtable.h:7,
                 from include/linux/mm.h:42,
                 from arch/sparc/kernel/sys_sparc_32.c:12:
include/asm-generic/pgtable.h: In function 'pmdp_get_and_clear':
include/asm-generic/pgtable.h:96: error: missing braces around initializer
include/asm-generic/pgtable.h:96: error: (near initialization for '(anonymous).pmdv')

Probably a side effect of
thp-add-pmd-mangling-generic-functions-fix-pgtableh-build-for-um.patch.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #3.1.2: Type: application/pgp-signature, Size: 490 bytes --]

Re: ia64 broken by transparent huge pages - other arches too?

[Tony Luck]

On Fri, Jan 14, 2011 at 10:30 AM, Andrea Arcangeli <aarcange@redhat.com> wrote:
> Would you be willing to implement __pmd for ia64 to fix this? Can you
> check if this works?
>
> Thanks a lot,
> Andrea
>
> ========
> Subject: fix ia64 build failure in pmdp_get_and_clear
>
> From: Andrea Arcangeli <aarcange@redhat.com>
>
> Implement __pmd macro for ia64 too.
>
> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
> ---
>
> diff --git a/arch/ia64/include/asm/page.h b/arch/ia64/include/asm/page.h
> index 41b6d31..961a16f 100644
> --- a/arch/ia64/include/asm/page.h
> +++ b/arch/ia64/include/asm/page.h
> @@ -189,6 +189,7 @@ get_order (unsigned long size)
>  # define pgprot_val(x) ((x).pgprot)
>
>  # define __pte(x)      ((pte_t) { (x) } )
> +# define __pmd(x)      ((pmd_t) { (x) } )
>  # define __pgprot(x)   ((pgprot_t) { (x) } )
>
>  #else /* !STRICT_MM_TYPECHECKS */

Yes that works. Thanks.  I'll apply to my ia64 tree and ask Linus to pull it.

-Tony

Re: ia64 broken by transparent huge pages - other arches too?

[Andrea Arcangeli]

Hi Tony,

On Fri, Jan 14, 2011 at 10:50:26AM -0800, Tony Luck wrote:
> Yes that works. Thanks.  I'll apply to my ia64 tree and ask Linus to pull it.

Awesome, thanks!
Andrea

Re: ia64 broken by transparent huge pages - other arches too?

[Benjamin Herrenschmidt]

On Fri, 2011-01-14 at 09:50 -0800, Luck, Tony wrote:
> Didn't Andrew make some rash promise at kernel summit about stopping
> eating if "-mm" wasn't included in linux-next by the end of November?
> 
> Must be getting pretty hungry by now.
> 
> The transparent huge page code just arrived in the merge window
> without having been in linux-next.  I see this error when trying
> to build for ia64 from Linus' tree this morning:

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.

Linus, wtf is going on ?

Ben.

> In file included from /home/aegl/generic-smp/arch/ia64/include/asm/pgtable.h:611,
>                  from include/linux/mm.h:41,
>                  from /home/aegl/generic-smp/arch/ia64/include/asm/uaccess.h:39,
>                  from include/linux/poll.h:14,
>                  from include/linux/rtc.h:117,
>                  from include/linux/efi.h:19,
>                  from /home/aegl/generic-smp/arch/ia64/include/asm/sal.h:40,
>                  from /home/aegl/generic-smp/arch/ia64/include/asm/mca.h:20,
>                  from arch/ia64/kernel/asm-offsets.c:17:
> include/asm-generic/pgtable.h: In function ‘pmdp_get_and_clear’:
> include/asm-generic/pgtable.h:96: error: implicit declaration of function ‘__pmd’
> include/asm-generic/pgtable.h:96: error: incompatible types in return
> make[1]: *** [arch/ia64/kernel/asm-offsets.s] Error 1
> 
> 
> Looks like arch/*/include/pgtable.h needs to define __pmd() but only x86
> was blessed with it.
> 
> -Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: ia64 broken by transparent huge pages - other arches too?

[Andrea Arcangeli]

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).

Re: ia64 broken by transparent huge pages - other arches too?

[James Bottomley]

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

Re: ia64 broken by transparent huge pages - other arches too?

[Andrea Arcangeli]

On Sat, Jan 15, 2011 at 10:47:28AM -0600, James Bottomley wrote:
> 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

Yes I've seen the arm build fix. Thanks for it! ;)

> 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.

I fully agree with you about build time regression, there linux-next
might have helped more. I'm talking about runtime regressions. If it
build it'll work because nothing relevant has changed for not-x86
archs.

By all means next times I'll try to get through linux-next too if this
is preferred, but the brainer part has been heavily tested and that's
the important thing as far as I can see.

I'm also not sure if having it in linux-next instead of -mm, would
have been better in terms of handling of the patchstream. I think
having it managed in -mm reviewed by all other -mm developers using
raw patches floating in the linux-mm and mm-commit lists, was ideal
and potentially more valuable for an increased amount of review, than
what a blind pull from linux-next could provide. For the brainer part,
maximizing the reviewing was certainly more valuable than checking if
it builds and boots on some arch not affected in any functional way.

I think the sparc/arm build issues because of cleanup code refactoring
are not worth worrying too much about, or at least they shouldn't be
the argument for lack of testing. Said that, I apologize for the
annoyance and I appreciate your help in the arm case. ia64 I fixed it
with a one liner already.

Overall I think the end result is great, perfection was the goal and
if these build issues are the only error I think we got as close as
humanly possible to it. And it's definitely thanks to an huge amount
of help and feedback from the whole Linux community (both developers,
maintainers and testers) if we could achieve this result, I could
never achieve this alone.

Sincerely thanks everyone!
Andrea

Re: ia64 broken by transparent huge pages - other arches too?

[Geert Uytterhoeven]

On Sat, Jan 15, 2011 at 18:23, Andrea Arcangeli <aarcange@redhat.com> wrote:
>> 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.
>
> I fully agree with you about build time regression, there linux-next
> might have helped more. I'm talking about runtime regressions. If it
> build it'll work because nothing relevant has changed for not-x86
> archs.
>
> By all means next times I'll try to get through linux-next too if this
> is preferred, but the brainer part has been heavily tested and that's
> the important thing as far as I can see.
>
> I'm also not sure if having it in linux-next instead of -mm, would
> have been better in terms of handling of the patchstream. I think
> having it managed in -mm reviewed by all other -mm developers using
> raw patches floating in the linux-mm and mm-commit lists, was ideal
> and potentially more valuable for an increased amount of review, than
> what a blind pull from linux-next could provide. For the brainer part,
> maximizing the reviewing was certainly more valuable than checking if
> it builds and boots on some arch not affected in any functional way.

These are not mutually exclusive options: we want to (a) have everything
reviewed and discussed _and_ (b) have everything build (and optionally run)
tested in linux-next. If perfection was the goal, we don't settle for anything
less.

Furthermore, even stupid and trivial to fix build issues may mask other
more severe issues, and delay the fixing of those.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: ia64 broken by transparent huge pages - other arches too?

[Benjamin Herrenschmidt]

On Sat, 2011-01-15 at 18:23 +0100, Andrea Arcangeli wrote:
> 
> 
> By all means next times I'll try to get through linux-next too if this
> is preferred, but the brainer part has been heavily tested and that's
> the important thing as far as I can see.

Linux-next is the integration testing essentially. That's where we find
such build regression and to a lesser extent maybe, runtime regressions.

I think you under estimate the pain caused by build breakage. The main
problem is that it makes bisection difficult, and that's a pretty big
deal in a merge window. If everybody stops caring about build breakage,
bisection would essentially become unusable accross merge windows.

> I'm also not sure if having it in linux-next instead of -mm, would
> have been better in terms of handling of the patchstream. I think
> having it managed in -mm reviewed by all other -mm developers using
> raw patches floating in the linux-mm and mm-commit lists, was ideal
> and potentially more valuable for an increased amount of review, than
> what a blind pull from linux-next could provide. For the brainer part,
> maximizing the reviewing was certainly more valuable than checking if
> it builds and boots on some arch not affected in any functional way.

It's not a matter of -mm vs. -next. You should not have a patch set that
is still a work in progress in -next. The later is for things that are
essentially ready to merge, to simmer there for a few days to find out
typically bad patch collisions (more than simple fixups), such build
breakages, major runtime breakages, etc... Ideally, things in -next
don't need a respin before going upstream but at least there's a last
chance to do so. 

The question becomes should -mm itself go into -next, and that I'm less
certain of. It depends on what criterias Andrew applies to things that
go into -mm I suppose, but if they qualify as "mature stuff ready to go
upstream" then by all means.

> I think the sparc/arm build issues because of cleanup code refactoring
> are not worth worrying too much about, or at least they shouldn't be
> the argument for lack of testing. Said that, I apologize for the
> annoyance and I appreciate your help in the arm case. ia64 I fixed it
> with a one liner already.

But that's the whole point... all those "little issues" have actually
broken build on 3 architectures so far, and this is -bad-. Yes, none of
them is major, all of them are easily fixed ... and all of them have
been a pain in the neck for some people somewhere and have broken
bisection accross a portion of the merge window.

Having a bit of time in -next allows to easily avoid most of this.

> Overall I think the end result is great, perfection was the goal and
> if these build issues are the only error I think we got as close as
> humanly possible to it. And it's definitely thanks to an huge amount
> of help and feedback from the whole Linux community (both developers,
> maintainers and testers) if we could achieve this result, I could
> never achieve this alone.

Cheers,
Ben.

Re: ia64 broken by transparent huge pages - other arches too?

[Linus Torvalds]

On Fri, Jan 14, 2011 at 11:21 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> 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.
>
> Linus, wtf is going on ?

I pretty much take anything from Andrew, unless I hate it (and the
latest version of the transparent huge-page was ok).

That said, I did think that Andrew's -mm tree was in -next, and there
clearly is some serious problem wrt -mm and -next if this wasn't
caught earlier there.

I was also expecting the fixups to come through Andrew, and they
haven't. I can apply whatever people agree is sane, but right now I
don't even know what the final patch for this problem is.

                        Linus

Re: ia64 broken by transparent huge pages - other arches too?

[Andrew Morton]

On Sun, 16 Jan 2011 13:05:15 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Fri, Jan 14, 2011 at 11:21 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> 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.
> >
> > Linus, wtf is going on ?
> 
> I pretty much take anything from Andrew, unless I hate it (and the
> latest version of the transparent huge-page was ok).
> 
> That said, I did think that Andrew's -mm tree was in -next, and there
> clearly is some serious problem wrt -mm and -next if this wasn't
> caught earlier there.

It's taking a while - Stephen and I are discussing a plan.

These problems were caused by a last-minute fixup which was more of a
breakup.

> I was also expecting the fixups to come through Andrew, and they
> haven't.

You've applied a couple directly, but this one is still outstanding.

> I can apply whatever people agree is sane, but right now I
> don't even know what the final patch for this problem is.

I believe it's the below.  I haven't tested it yet and nor has anyone
else apart from Andrea, afaik.


From: Andrea Arcangeli <aarcange@redhat.com>

pmdp_get_and_clear/pmdp_clear_flush/pmdp_splitting_flush were trapped as
BUG() and they were defined only to diminish the risk of build issues on
not-x86 archs and to be consistent with the generic pte methods previously
defined in include/asm-generic/pgtable.h.

But they are causing more trouble than they were supposed to solve, so
it's simpler not to define them when THP is off.

This is also correcting the export of pmdp_splitting_flush which is
currently unused (x86 isn't using the generic implementation in
mm/pgtable-generic.c and no other arch needs that [yet]).

Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
Sam Ravnborg <sam@ravnborg.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: "Luck, Tony" <tony.luck@intel.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/asm-generic/pgtable.h |   14 +++-----------
 mm/pgtable-generic.c          |   11 ++++-------
 2 files changed, 7 insertions(+), 18 deletions(-)

diff -puN include/asm-generic/pgtable.h~mm-thp-remove-pmdp_get_and_clear-pmdp_clear_flush-pmdp_splitting_flush-methods-when-thp=n include/asm-generic/pgtable.h
--- a/include/asm-generic/pgtable.h~mm-thp-remove-pmdp_get_and_clear-pmdp_clear_flush-pmdp_splitting_flush-methods-when-thp=n
+++ a/include/asm-generic/pgtable.h
@@ -87,14 +87,6 @@ static inline pmd_t pmdp_get_and_clear(s
 	pmd_clear(mm, address, pmdp);
 	return pmd;
 })
-#else /* CONFIG_TRANSPARENT_HUGEPAGE */
-static inline pmd_t pmdp_get_and_clear(struct mm_struct *mm,
-				       unsigned long address,
-				       pmd_t *pmdp)
-{
-	BUG();
-	return __pmd(0);
-}
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 #endif
 
@@ -163,9 +155,9 @@ static inline void pmdp_set_wrprotect(st
 #endif
 
 #ifndef __HAVE_ARCH_PMDP_SPLITTING_FLUSH
-extern pmd_t pmdp_clear_flush(struct vm_area_struct *vma,
-			      unsigned long address,
-			      pmd_t *pmdp);
+extern pmd_t pmdp_splitting_flush(struct vm_area_struct *vma,
+				  unsigned long address,
+				  pmd_t *pmdp);
 #endif
 
 #ifndef __HAVE_ARCH_PTE_SAME
diff -puN mm/pgtable-generic.c~mm-thp-remove-pmdp_get_and_clear-pmdp_clear_flush-pmdp_splitting_flush-methods-when-thp=n mm/pgtable-generic.c
--- a/mm/pgtable-generic.c~mm-thp-remove-pmdp_get_and_clear-pmdp_clear_flush-pmdp_splitting_flush-methods-when-thp=n
+++ a/mm/pgtable-generic.c
@@ -92,32 +92,29 @@ pte_t ptep_clear_flush(struct vm_area_st
 #endif
 
 #ifndef __HAVE_ARCH_PMDP_CLEAR_FLUSH
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
 pmd_t pmdp_clear_flush(struct vm_area_struct *vma, unsigned long address,
 		       pmd_t *pmdp)
 {
 	pmd_t pmd;
-#ifndef CONFIG_TRANSPARENT_HUGEPAGE
-	BUG();
-#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
 	pmd = pmdp_get_and_clear(vma->vm_mm, address, pmdp);
 	flush_tlb_range(vma, address, address + HPAGE_PMD_SIZE);
 	return pmd;
 }
+#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 #endif
 
 #ifndef __HAVE_ARCH_PMDP_SPLITTING_FLUSH
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
 pmd_t pmdp_splitting_flush(struct vm_area_struct *vma, unsigned long address,
 			   pmd_t *pmdp)
 {
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	pmd_t pmd = pmd_mksplitting(*pmdp);
 	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
 	set_pmd_at(vma->vm_mm, address, pmdp, pmd);
 	/* tlb flush only to serialize against gup-fast */
 	flush_tlb_range(vma, address, address + HPAGE_PMD_SIZE);
-#else /* CONFIG_TRANSPARENT_HUGEPAGE */
-	BUG();
-#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 }
+#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 #endif
_

Re: ia64 broken by transparent huge pages - other arches too?

[Andrea Arcangeli]

On Sun, Jan 16, 2011 at 01:10:39PM -0800, Andrew Morton wrote:
> I believe it's the below.  I haven't tested it yet and nor has anyone
> else apart from Andrea, afaik.

Correct. I tested it with sparc64 cross compiler using ARCH=sparc32 in
addition to x86_32/64 with THP on and off. It should solve the
problem.

James' fix for arm still needed in addition to it, I'm appending it.

Thanks and sorry again for the arch build trouble.
Andrea

===
Subject: [PATCH] parisc: fix compile breakage caused by inlining maybe_mkwrite

From: James Bottomley <James.Bottomley@HansenPartnership.com>

on Parisc, we have an include of linux/mm.h inside our asm/pgtable.h, so
this patch 

commit 14fd403f2146f740942d78af4e0ee59396ad8eab
Author: Andrea Arcangeli <aarcange@redhat.com>
Date:   Thu Jan 13 15:46:37 2011 -0800

    thp: export maybe_mkwrite

Causes us an unsatisfiable use of pte_mkwrite in linux/mm.h

The fix is obviously not to include linux/mm.h in our pgtable.h, which
unbreaks the build.

James

---

diff --git a/arch/parisc/include/asm/pgtable.h b/arch/parisc/include/asm/pgtable.h
index f3c0973..5d7b8ce 100644
--- a/arch/parisc/include/asm/pgtable.h
+++ b/arch/parisc/include/asm/pgtable.h
@@ -10,11 +10,13 @@
  * we simulate an x86-style page table for the linux mm code
  */
 
-#include <linux/mm.h>		/* for vm_area_struct */
 #include <linux/bitops.h>
+#include <linux/spinlock.h>
 #include <asm/processor.h>
 #include <asm/cache.h>
 
+struct vm_area_struct;
+
 /*
  * kern_addr_valid(ADDR) tests if ADDR is pointing to valid kernel
  * memory.  For the return value to be meaningful, ADDR must be >=