[RFC:PATCH 00/06] Strong Access Ordering page attributes for POWER7

[RFC:PATCH 00/06] Strong Access Ordering page attributes for POWER7

[Dave Kleikamp]

Allow an application to enable Strong Access Ordering on specific pages of
memory on Power 7 hardware. Currently, power has a weaker memory model than
x86. Implementing a stronger memory model allows an emulator to more
efficiently translate x86 code into power code, resulting in faster code
execution.

On Power 7 hardware, storing 0b1110 in the WIMG bits of the hpte enables
strong access ordering mode for the memory page.  This patchset allows a
user to specify which pages are thus enabled by passing a new protection
bit through mmap() and mprotect().  I have tentatively defined this bit,
PROT_SAO, as 0x10.

In order to accomplish this, I had to modify the architecture-independent
code to allow the architecture to deal with additional protection bits.

Patches built against 2.6.26-rc5.

Any and all suggestions, complaints, flames, insults, etc. are appreciated.

Thanks,
Shaggy

[RFC:PATCH 01/06] powerpc: hash_huge_page() should get the WIMG bits from the lpte

[Dave Kleikamp]

powerpc: hash_huge_page() should get the WIMG bits from the lpte

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
---

 arch/powerpc/mm/hugetlbpage.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff -Nurp linux000/arch/powerpc/mm/hugetlbpage.c linux001/arch/powerpc/mm/hugetlbpage.c
--- linux000/arch/powerpc/mm/hugetlbpage.c	2008-04-16 21:49:44.000000000 -0500
+++ linux001/arch/powerpc/mm/hugetlbpage.c	2008-06-10 16:48:59.000000000 -0500
@@ -502,9 +502,8 @@ repeat:
 		new_pte = (new_pte & ~_PAGE_HPTEFLAGS) | _PAGE_HASHPTE;
 
 		/* Add in WIMG bits */
-		/* XXX We should store these in the pte */
-		/* --BenH: I think they are ... */
-		rflags |= _PAGE_COHERENT;
+		rflags |= (new_pte & (_PAGE_WRITETHRU | _PAGE_NO_CACHE |
+				      _PAGE_COHERENT | _PAGE_GUARDED));
 
 		/* Insert into the hash table, primary slot */
 		slot = ppc_md.hpte_insert(hpte_group, va, pa, rflags, 0,

[RFC:PATCH 02/06] mm: Allow architectures to define additional protection bits

[Dave Kleikamp]

mm: Allow architectures to define additional protection bits

This patch allows architectures to define functions to deal with
additional protections bits for mmap() and mprotect().

arch_calc_vm_prot_bits() maps additonal protection bits to vm_flags
arch_vm_get_page_prot() maps additional vm_flags to the vma's vm_page_prot
arch_validate_prot() checks for valid values of the protection bits

Note: vm_get_page_prot() is now pretty ugly.  Suggestions?

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
---

 include/linux/mman.h |   23 ++++++++++++++++++++++-
 mm/mmap.c            |    5 +++--
 mm/mprotect.c        |    2 +-
 3 files changed, 26 insertions(+), 4 deletions(-)

diff -Nurp linux001/include/linux/mman.h linux002/include/linux/mman.h
--- linux001/include/linux/mman.h	2008-06-05 10:08:01.000000000 -0500
+++ linux002/include/linux/mman.h	2008-06-10 16:48:59.000000000 -0500
@@ -34,6 +34,26 @@ static inline void vm_unacct_memory(long
 }
 
 /*
+ * Allow architectures to handle additional protection bits
+ */
+
+#ifndef HAVE_ARCH_PROT_BITS
+#define arch_calc_vm_prot_bits(prot) 0
+#define arch_vm_get_page_prot(vm_flags) __pgprot(0)
+
+/*
+ * This is called from mprotect().  PROT_GROWSDOWN and PROT_GROWSUP have
+ * already been masked out.
+ *
+ * Returns true if the prot flags are valid
+ */
+static inline int arch_validate_prot(unsigned long prot)
+{
+	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
+}
+#endif /* HAVE_ARCH_PROT_BITS */
+
+/*
  * Optimisation macro.  It is equivalent to:
  *      (x & bit1) ? bit2 : 0
  * but this version is faster.
@@ -51,7 +71,8 @@ calc_vm_prot_bits(unsigned long prot)
 {
 	return _calc_vm_trans(prot, PROT_READ,  VM_READ ) |
 	       _calc_vm_trans(prot, PROT_WRITE, VM_WRITE) |
-	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC );
+	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC) |
+	       arch_calc_vm_prot_bits(prot);
 }
 
 /*
diff -Nurp linux001/mm/mmap.c linux002/mm/mmap.c
--- linux001/mm/mmap.c	2008-06-05 10:08:03.000000000 -0500
+++ linux002/mm/mmap.c	2008-06-10 16:48:59.000000000 -0500
@@ -72,8 +72,9 @@ pgprot_t protection_map[16] = {
 
 pgprot_t vm_get_page_prot(unsigned long vm_flags)
 {
-	return protection_map[vm_flags &
-				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
+	return __pgprot(pgprot_val(protection_map[vm_flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
+			pgprot_val(arch_vm_get_page_prot(vm_flags)));
 }
 EXPORT_SYMBOL(vm_get_page_prot);
 
diff -Nurp linux001/mm/mprotect.c linux002/mm/mprotect.c
--- linux001/mm/mprotect.c	2008-06-05 10:08:03.000000000 -0500
+++ linux002/mm/mprotect.c	2008-06-10 16:48:59.000000000 -0500
@@ -239,7 +239,7 @@ sys_mprotect(unsigned long start, size_t
 	end = start + len;
 	if (end <= start)
 		return -ENOMEM;
-	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM))
+	if (!arch_validate_prot(prot))
 		return -EINVAL;
 
 	reqprot = prot;

Re: [RFC:PATCH 02/06] mm: Allow architectures to define additional protection bits

[Andrew Morton]

On Tue, 10 Jun 2008 18:01:07 -0400
Dave Kleikamp <shaggy@linux.vnet.ibm.com> wrote:

> mm: Allow architectures to define additional protection bits
> 
> This patch allows architectures to define functions to deal with
> additional protections bits for mmap() and mprotect().
> 
> arch_calc_vm_prot_bits() maps additonal protection bits to vm_flags
> arch_vm_get_page_prot() maps additional vm_flags to the vma's vm_page_prot
> arch_validate_prot() checks for valid values of the protection bits
> 
> Note: vm_get_page_prot() is now pretty ugly.  Suggestions?

It didn't get any better, no ;)

I wonder if we can do the ORing after doing the protection_map[]
lookup.  I guess that's illogical even if it happens to work.

> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> ---
> 
>  include/linux/mman.h |   23 ++++++++++++++++++++++-
>  mm/mmap.c            |    5 +++--
>  mm/mprotect.c        |    2 +-
>  3 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff -Nurp linux001/include/linux/mman.h linux002/include/linux/mman.h
> --- linux001/include/linux/mman.h	2008-06-05 10:08:01.000000000 -0500
> +++ linux002/include/linux/mman.h	2008-06-10 16:48:59.000000000 -0500
> @@ -34,6 +34,26 @@ static inline void vm_unacct_memory(long
>  }
>  
>  /*
> + * Allow architectures to handle additional protection bits
> + */
> +
> +#ifndef HAVE_ARCH_PROT_BITS
> +#define arch_calc_vm_prot_bits(prot) 0
> +#define arch_vm_get_page_prot(vm_flags) __pgprot(0)
> +
> +/*
> + * This is called from mprotect().  PROT_GROWSDOWN and PROT_GROWSUP have
> + * already been masked out.
> + *
> + * Returns true if the prot flags are valid
> + */
> +static inline int arch_validate_prot(unsigned long prot)
> +{
> +	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
> +}
> +#endif /* HAVE_ARCH_PROT_BITS */

argh, another HAVE_ARCH_foo.

A good (but verbose) way of doing this is to nuke the ifdefs and just
go and define these three things for each architecture.  That can be
done via copy-n-paste into include/asm-*/mman.h or #include
<asm-generic/arch-mman.h>(?) within each asm/mman.h.

Another way would be

#ifndef arch_calc_vm_prot_bits
#define arch_calc_vm_prot_bits(prot) ...


> +/*
>   * Optimisation macro.  It is equivalent to:
>   *      (x & bit1) ? bit2 : 0
>   * but this version is faster.
> @@ -51,7 +71,8 @@ calc_vm_prot_bits(unsigned long prot)
>  {
>  	return _calc_vm_trans(prot, PROT_READ,  VM_READ ) |
>  	       _calc_vm_trans(prot, PROT_WRITE, VM_WRITE) |
> -	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC );
> +	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC) |
> +	       arch_calc_vm_prot_bits(prot);
>  }
>  
>  /*
> diff -Nurp linux001/mm/mmap.c linux002/mm/mmap.c
> --- linux001/mm/mmap.c	2008-06-05 10:08:03.000000000 -0500
> +++ linux002/mm/mmap.c	2008-06-10 16:48:59.000000000 -0500
> @@ -72,8 +72,9 @@ pgprot_t protection_map[16] = {
>  
>  pgprot_t vm_get_page_prot(unsigned long vm_flags)
>  {
> -	return protection_map[vm_flags &
> -				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
> +	return __pgprot(pgprot_val(protection_map[vm_flags &
> +				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
> +			pgprot_val(arch_vm_get_page_prot(vm_flags)));
>  }
>  EXPORT_SYMBOL(vm_get_page_prot);
>  
> diff -Nurp linux001/mm/mprotect.c linux002/mm/mprotect.c
> --- linux001/mm/mprotect.c	2008-06-05 10:08:03.000000000 -0500
> +++ linux002/mm/mprotect.c	2008-06-10 16:48:59.000000000 -0500
> @@ -239,7 +239,7 @@ sys_mprotect(unsigned long start, size_t
>  	end = start + len;
>  	if (end <= start)
>  		return -ENOMEM;
> -	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM))
> +	if (!arch_validate_prot(prot))
>  		return -EINVAL;
>  
>  	reqprot = prot;

Re: [RFC:PATCH 02/06] mm: Allow architectures to define additional protection bits

[Dave Kleikamp]

On Tue, 2008-06-10 at 15:14 -0700, Andrew Morton wrote:
> On Tue, 10 Jun 2008 18:01:07 -0400
> Dave Kleikamp <shaggy@linux.vnet.ibm.com> wrote:
> 
> > mm: Allow architectures to define additional protection bits
> > 
> > This patch allows architectures to define functions to deal with
> > additional protections bits for mmap() and mprotect().
> > 
> > arch_calc_vm_prot_bits() maps additonal protection bits to vm_flags
> > arch_vm_get_page_prot() maps additional vm_flags to the vma's vm_page_prot
> > arch_validate_prot() checks for valid values of the protection bits
> > 
> > Note: vm_get_page_prot() is now pretty ugly.  Suggestions?
> 
> It didn't get any better, no ;)
> 
> I wonder if we can do the ORing after doing the protection_map[]
> lookup.  I guess that's illogical even if it happens to work.

I guess we can live with it.  Just holding out hope that someone might
see a nicer way to do it.

> > diff -Nurp linux001/include/linux/mman.h linux002/include/linux/mman.h
> > --- linux001/include/linux/mman.h	2008-06-05 10:08:01.000000000 -0500
> > +++ linux002/include/linux/mman.h	2008-06-10 16:48:59.000000000 -0500
> > @@ -34,6 +34,26 @@ static inline void vm_unacct_memory(long
> >  }
> >  
> >  /*
> > + * Allow architectures to handle additional protection bits
> > + */
> > +
> > +#ifndef HAVE_ARCH_PROT_BITS
> > +#define arch_calc_vm_prot_bits(prot) 0
> > +#define arch_vm_get_page_prot(vm_flags) __pgprot(0)
> > +
> > +/*
> > + * This is called from mprotect().  PROT_GROWSDOWN and PROT_GROWSUP have
> > + * already been masked out.
> > + *
> > + * Returns true if the prot flags are valid
> > + */
> > +static inline int arch_validate_prot(unsigned long prot)
> > +{
> > +	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
> > +}
> > +#endif /* HAVE_ARCH_PROT_BITS */
> 
> argh, another HAVE_ARCH_foo.

Sorry.  I didn't realize HAVE_ARCH_foo was so evil.

> A good (but verbose) way of doing this is to nuke the ifdefs and just
> go and define these three things for each architecture.  That can be
> done via copy-n-paste into include/asm-*/mman.h or #include
> <asm-generic/arch-mman.h>(?) within each asm/mman.h.
> 
> Another way would be
> 
> #ifndef arch_calc_vm_prot_bits
> #define arch_calc_vm_prot_bits(prot) ...

I think I prefer this method.  I'll get rid of HAVE_ARCH_PROT_BITS.

Thanks,
Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

[RFC:PATCH 03/06] powerpc: Define flags for Strong Access Ordering

[Dave Kleikamp]

powerpc: Define flags for Strong Access Ordering

This patch defines:

- PROT_SAO, which is passed into mmap() and mprotect() in the prot field
- VM_SAO in vma->vm_flags, and
- _PAGE_SAO, the combination of WIMG bits in the pte that enables strong
access ordering for the page.

NOTE: There doesn't seem to be a precedent for architecture-dependent vm_flags.
It may be better to define VM_SAO somewhere in include/asm-powerpc/.  Since
vm_flags is a long, defining it in the high-order word would help prevent a
collision with any newly added values in architecture-independent code.

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
---

 include/asm-powerpc/mman.h          |    2 ++
 include/asm-powerpc/pgtable-ppc64.h |    3 +++
 include/linux/mm.h                  |    1 +
 3 files changed, 6 insertions(+)

diff -Nurp linux002/include/asm-powerpc/mman.h linux003/include/asm-powerpc/mman.h
--- linux002/include/asm-powerpc/mman.h	2008-04-16 21:49:44.000000000 -0500
+++ linux003/include/asm-powerpc/mman.h	2008-06-10 16:48:59.000000000 -0500
@@ -10,6 +10,8 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#define PROT_SAO	0x10		/* Strong Access Ordering */
+
 #define MAP_RENAME      MAP_ANONYMOUS   /* In SunOS terminology */
 #define MAP_NORESERVE   0x40            /* don't reserve swap pages */
 #define MAP_LOCKED	0x80
diff -Nurp linux002/include/asm-powerpc/pgtable-ppc64.h linux003/include/asm-powerpc/pgtable-ppc64.h
--- linux002/include/asm-powerpc/pgtable-ppc64.h	2008-06-05 10:07:56.000000000 -0500
+++ linux003/include/asm-powerpc/pgtable-ppc64.h	2008-06-10 16:48:59.000000000 -0500
@@ -94,6 +94,9 @@
 #define _PAGE_HASHPTE	0x0400 /* software: pte has an associated HPTE */
 #define _PAGE_BUSY	0x0800 /* software: PTE & hash are busy */
 
+/* Strong Access Ordering */
+#define _PAGE_SAO	(_PAGE_WRITETHRU | _PAGE_NO_CACHE | _PAGE_COHERENT)
+
 #define _PAGE_BASE	(_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_COHERENT)
 
 #define _PAGE_WRENABLE	(_PAGE_RW | _PAGE_DIRTY)
diff -Nurp linux002/include/linux/mm.h linux003/include/linux/mm.h
--- linux002/include/linux/mm.h	2008-06-05 10:08:01.000000000 -0500
+++ linux003/include/linux/mm.h	2008-06-10 16:48:59.000000000 -0500
@@ -108,6 +108,7 @@ extern unsigned int kobjsize(const void 
 
 #define VM_CAN_NONLINEAR 0x08000000	/* Has ->fault & does nonlinear pages */
 #define VM_MIXEDMAP	0x10000000	/* Can contain "struct page" and pure PFN pages */
+#define VM_SAO		0x20000000	/* Strong Access Ordering (powerpc) */
 
 #ifndef VM_STACK_DEFAULT_FLAGS		/* arch can override this */
 #define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS

[RFC:PATCH 04/06] powerpc: Define CPU_FTR_SAO

[Dave Kleikamp]

powerpc: Define CPU_FTR_SAO

This is just a placeholder to make the patchset compilable.

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
---

 include/asm-powerpc/cputable.h |    1 +
 1 file changed, 1 insertion(+)

diff -Nurp linux003/include/asm-powerpc/cputable.h linux004/include/asm-powerpc/cputable.h
--- linux003/include/asm-powerpc/cputable.h	2008-04-16 21:49:44.000000000 -0500
+++ linux004/include/asm-powerpc/cputable.h	2008-06-10 16:48:59.000000000 -0500
@@ -180,6 +180,7 @@ extern void do_feature_fixups(unsigned l
 #define CPU_FTR_DSCR			LONG_ASM_CONST(0x0002000000000000)
 #define CPU_FTR_1T_SEGMENT		LONG_ASM_CONST(0x0004000000000000)
 #define CPU_FTR_NO_SLBIE_B		LONG_ASM_CONST(0x0008000000000000)
+#define CPU_FTR_SAO			LONG_ASM_CONST(0x0010000000000000)
 
 #ifndef __ASSEMBLY__
 

[RFC:PATCH 05/06] powerpc: Add Strong Access Ordering

[Dave Kleikamp]

powerpc: Add Strong Access Ordering

Things I don't like about this patch:

1. All the includes I added to asm-powerpc/mman.h
2. It doesn't look like mmap() used to validate prot.  Now instead of
ignoring invalid values, it will return -EINVAL.  Could this be a problem?
3. Are these new functions in any hot paths that the extra instructions will
add any significant overhead?

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
---

 arch/powerpc/kernel/syscalls.c |    3 +++
 include/asm-powerpc/mman.h     |   26 ++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff -Nurp linux004/arch/powerpc/kernel/syscalls.c linux005/arch/powerpc/kernel/syscalls.c
--- linux004/arch/powerpc/kernel/syscalls.c	2008-06-05 10:07:32.000000000 -0500
+++ linux005/arch/powerpc/kernel/syscalls.c	2008-06-10 16:48:59.000000000 -0500
@@ -143,6 +143,9 @@ static inline unsigned long do_mmap2(uns
 	struct file * file = NULL;
 	unsigned long ret = -EINVAL;
 
+	if (!arch_validate_prot(prot))
+		goto out;
+
 	if (shift) {
 		if (off & ((1 << shift) - 1))
 			goto out;
diff -Nurp linux004/include/asm-powerpc/mman.h linux005/include/asm-powerpc/mman.h
--- linux004/include/asm-powerpc/mman.h	2008-06-10 16:48:59.000000000 -0500
+++ linux005/include/asm-powerpc/mman.h	2008-06-10 16:48:59.000000000 -0500
@@ -1,7 +1,9 @@
 #ifndef _ASM_POWERPC_MMAN_H
 #define _ASM_POWERPC_MMAN_H
 
+#include <asm/cputable.h>
 #include <asm-generic/mman.h>
+#include <linux/mm.h>
 
 /*
  * This program is free software; you can redistribute it and/or
@@ -26,4 +28,28 @@
 #define MAP_POPULATE	0x8000		/* populate (prefault) pagetables */
 #define MAP_NONBLOCK	0x10000		/* do not block on IO */
 
+#define HAVE_ARCH_PROT_BITS
+
+/*
+ * This file is included by linux/mman.h, so we can't use cacl_vm_prot_bits()
+ * here.  How important is the optimization?
+ */
+static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot)
+{
+	return (prot & PROT_SAO) ? VM_SAO : 0;
+}
+
+static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
+{
+	return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : 0;
+}
+
+static inline int arch_validate_prot(unsigned long prot)
+{
+	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
+		return 1;
+	if ((prot & PROT_SAO) && !cpu_has_feature(CPU_FTR_SAO))
+		return 1;
+	return 0;
+}
 #endif	/* _ASM_POWERPC_MMAN_H */

[RFC:PATCH 06/06] powerpc: Don't clear _PAGE_COHERENT when _PAGE_SAO is set

[Dave Kleikamp]

powerpc: Don't clear _PAGE_COHERENT when _PAGE_SAO is set

This is a placeholder.  Benh tells me that he will come up with a better fix.

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
---

 arch/powerpc/platforms/pseries/lpar.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff -Nurp linux005/arch/powerpc/platforms/pseries/lpar.c linux006/arch/powerpc/platforms/pseries/lpar.c
--- linux005/arch/powerpc/platforms/pseries/lpar.c	2008-06-05 10:07:34.000000000 -0500
+++ linux006/arch/powerpc/platforms/pseries/lpar.c	2008-06-10 16:48:59.000000000 -0500
@@ -305,7 +305,8 @@ static long pSeries_lpar_hpte_insert(uns
 	flags = 0;
 
 	/* Make pHyp happy */
-	if (rflags & (_PAGE_GUARDED|_PAGE_NO_CACHE))
+	if ((rflags & _PAGE_GUARDED) ||
+	    ((rflags & _PAGE_NO_CACHE) & !(rflags & _PAGE_WRITETHRU)))
 		hpte_r &= ~_PAGE_COHERENT;
 
 	lpar_rc = plpar_pte_enter(flags, hpte_group, hpte_v, hpte_r, &slot);

Re: [RFC:PATCH 06/06] powerpc: Don't clear _PAGE_COHERENT when _PAGE_SAO is set

[Sergei Shtylyov]

Hello.

Dave Kleikamp wrote:
> powerpc: Don't clear _PAGE_COHERENT when _PAGE_SAO is set
>
> This is a placeholder.  Benh tells me that he will come up with a better fix.
>
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> ---
>
>  arch/powerpc/platforms/pseries/lpar.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff -Nurp linux005/arch/powerpc/platforms/pseries/lpar.c linux006/arch/powerpc/platforms/pseries/lpar.c
> --- linux005/arch/powerpc/platforms/pseries/lpar.c	2008-06-05 10:07:34.000000000 -0500
> +++ linux006/arch/powerpc/platforms/pseries/lpar.c	2008-06-10 16:48:59.000000000 -0500
> @@ -305,7 +305,8 @@ static long pSeries_lpar_hpte_insert(uns
>  	flags = 0;
>  
>  	/* Make pHyp happy */
> -	if (rflags & (_PAGE_GUARDED|_PAGE_NO_CACHE))
> +	if ((rflags & _PAGE_GUARDED) ||
> +	    ((rflags & _PAGE_NO_CACHE) & !(rflags & _PAGE_WRITETHRU)))
>   
   I don't think you really meant bitwise AND here. I suppose the second 
expression just will never be true.

WBR, Sergei

Re: [RFC:PATCH 06/06] powerpc: Don't clear _PAGE_COHERENT when _PAGE_SAO is set

[Dave Kleikamp]

On Wed, 2008-06-11 at 02:26 +0400, Sergei Shtylyov wrote:
> Hello.
> 
> Dave Kleikamp wrote:
> > powerpc: Don't clear _PAGE_COHERENT when _PAGE_SAO is set
> >
> > This is a placeholder.  Benh tells me that he will come up with a better fix.
> >
> > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> > ---
> >
> >  arch/powerpc/platforms/pseries/lpar.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff -Nurp linux005/arch/powerpc/platforms/pseries/lpar.c linux006/arch/powerpc/platforms/pseries/lpar.c
> > --- linux005/arch/powerpc/platforms/pseries/lpar.c	2008-06-05 10:07:34.000000000 -0500
> > +++ linux006/arch/powerpc/platforms/pseries/lpar.c	2008-06-10 16:48:59.000000000 -0500
> > @@ -305,7 +305,8 @@ static long pSeries_lpar_hpte_insert(uns
> >  	flags = 0;
> >  
> >  	/* Make pHyp happy */
> > -	if (rflags & (_PAGE_GUARDED|_PAGE_NO_CACHE))
> > +	if ((rflags & _PAGE_GUARDED) ||
> > +	    ((rflags & _PAGE_NO_CACHE) & !(rflags & _PAGE_WRITETHRU)))
> >   
>    I don't think you really meant bitwise AND here. I suppose the second 
> expression just will never be true.

You're right.  That should be &&.  Thanks.

> WBR, Sergei
> 
> 
-- 
David Kleikamp
IBM Linux Technology Center