mtrr use type bool

PATCH In review Paul Jimenez 3 participants 7 messages Oct 31, 2007 – Nov 2, 2007 x86

Share & tools

Paul Jimenez

author

2007-10-31 03:57 UTC #0 lore raw

PATCH 1/1 mtrr use type bool

This is a janitorish patch to 1) remove private TRUE/FALSE #def's in favor of using the standard enum from linux/stddef.h and 2) switch the variables holding those values to type 'bool' (from linux/types.h) since it both seems more appropriate and allows for potentially better optimization.

As a truly minor aside, I removed a couple of comments documenting a 'do_safe' parameter that seems to no longer exist.

Signed-off-by Paul Jimenez <[email protected]>

arch/x86/kernel/cpu/mtrr/amd.c +0 -2

index 0949cdb..ee2331b 100644
@@ -53,8 +53,6 @@ static void amd_set_mtrr(unsigned int reg, unsigned long base,
     <base> The base address of the region.
     <size> The size of the region. If this is 0 the region is disabled.
     <type> The type of the region.
-    <do_safe> If TRUE, do the change safely. If FALSE, safety measures should
-    be done externally.
     [RETURNS] Nothing.
 */
 {

arch/x86/kernel/cpu/mtrr/generic.c +7 -9

index 992f08d..1c331c3 100644
@@ -188,7 +188,7 @@ static inline void k8_enable_fixed_iorrs(void)
  * \param changed pointer which indicates whether the MTRR needed to be changed
  * \param msrwords pointer to the MSR values which the MSR should have
  */
-static void set_fixed_range(int msr, int * changed, unsigned int * msrwords)
+static void set_fixed_range(int msr, bool *changed, unsigned int *msrwords)
 {
 	unsigned lo, hi;

@@ -200,7 +200,7 @@ static void set_fixed_range(int msr, int * changed, unsigned int * msrwords)
 		    ((msrwords[0] | msrwords[1]) & K8_MTRR_RDMEM_WRMEM_MASK))
 			k8_enable_fixed_iorrs();
 		mtrr_wrmsr(msr, msrwords[0], msrwords[1]);
-		*changed = TRUE;
+		*changed = true;
 	}
 }

@@ -260,7 +260,7 @@ static void generic_get_mtrr(unsigned int reg, unsigned long *base,
 static int set_fixed_ranges(mtrr_type * frs)
 {
 	unsigned long long *saved = (unsigned long long *) frs;
-	int changed = FALSE;
+	bool changed = false;
 	int block=-1, range;
 
 	while (fixed_range_blocks[++block].ranges)

@@ -273,17 +273,17 @@ static int set_fixed_ranges(mtrr_type * frs)
 
 /*  Set the MSR pair relating to a var range. Returns TRUE if
     changes are made  */
-static int set_mtrr_var_ranges(unsigned int index, struct mtrr_var_range *vr)
+static bool set_mtrr_var_ranges(unsigned int index, struct mtrr_var_range *vr)
 {
 	unsigned int lo, hi;
-	int changed = FALSE;
+	bool changed = false;
 
 	rdmsr(MTRRphysBase_MSR(index), lo, hi);
 	if ((vr->base_lo & 0xfffff0ffUL) != (lo & 0xfffff0ffUL)
 	    || (vr->base_hi & (size_and_mask >> (32 - PAGE_SHIFT))) !=
 		(hi & (size_and_mask >> (32 - PAGE_SHIFT)))) {
 		mtrr_wrmsr(MTRRphysBase_MSR(index), vr->base_lo, vr->base_hi);
-		changed = TRUE;
+		changed = true;
 	}
 
 	rdmsr(MTRRphysMask_MSR(index), lo, hi);

@@ -292,7 +292,7 @@ static int set_mtrr_var_ranges(unsigned int index, struct mtrr_var_range *vr)
 	    || (vr->mask_hi & (size_and_mask >> (32 - PAGE_SHIFT))) !=
 		(hi & (size_and_mask >> (32 - PAGE_SHIFT)))) {
 		mtrr_wrmsr(MTRRphysMask_MSR(index), vr->mask_lo, vr->mask_hi);
-		changed = TRUE;
+		changed = true;
 	}
 	return changed;
 }

@@ -417,8 +417,6 @@ static void generic_set_mtrr(unsigned int reg, unsigned long base,
     <base> The base address of the region.
     <size> The size of the region. If this is 0 the region is disabled.
     <type> The type of the region.
-    <do_safe> If TRUE, do the change safely. If FALSE, safety measures should
-    be done externally.
     [RETURNS] Nothing.
 */
 {

arch/x86/kernel/cpu/mtrr/if.c +8 -7

index c7d8f17..1453568 100644
@@ -37,7 +37,7 @@ const char *mtrr_attrib_to_str(int x)
 
 static int
 mtrr_file_add(unsigned long base, unsigned long size,
-	      unsigned int type, char increment, struct file *file, int page)
+	      unsigned int type, bool increment, struct file *file, int page)
 {
 	int reg, max;
 	unsigned int *fcount = FILE_FCOUNT(file);

@@ -55,7 +55,7 @@ mtrr_file_add(unsigned long base, unsigned long size,
 		base >>= PAGE_SHIFT;
 		size >>= PAGE_SHIFT;
 	}
-	reg = mtrr_add_page(base, size, type, 1);
+	reg = mtrr_add_page(base, size, type, true);
 	if (reg >= 0)
 		++fcount[reg];
 	return reg;

@@ -141,7 +141,7 @@ mtrr_write(struct file *file, const char __user *buf, size_t len, loff_t * ppos)
 		size >>= PAGE_SHIFT;
 		err =
 		    mtrr_add_page((unsigned long) base, (unsigned long) size, i,
-				  1);
+				  true);
 		if (err < 0)
 			return err;
 		return len;

@@ -217,7 +217,7 @@ mtrr_ioctl(struct file *file, unsigned int cmd, unsigned long __arg)
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
 		err =
-		    mtrr_file_add(sentry.base, sentry.size, sentry.type, 1,
+		    mtrr_file_add(sentry.base, sentry.size, sentry.type, true,
 				  file, 0);
 		break;
 	case MTRRIOC_SET_ENTRY:

@@ -226,7 +226,7 @@ mtrr_ioctl(struct file *file, unsigned int cmd, unsigned long __arg)
 #endif
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
-		err = mtrr_add(sentry.base, sentry.size, sentry.type, 0);
+		err = mtrr_add(sentry.base, sentry.size, sentry.type, false);
 		break;
 	case MTRRIOC_DEL_ENTRY:
 #ifdef CONFIG_COMPAT

@@ -270,7 +270,7 @@ mtrr_ioctl(struct file *file, unsigned int cmd, unsigned long __arg)
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
 		err =
-		    mtrr_file_add(sentry.base, sentry.size, sentry.type, 1,
+		    mtrr_file_add(sentry.base, sentry.size, sentry.type, true,
 				  file, 1);
 		break;
 	case MTRRIOC_SET_PAGE_ENTRY:

@@ -279,7 +279,8 @@ mtrr_ioctl(struct file *file, unsigned int cmd, unsigned long __arg)
 #endif
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
-		err = mtrr_add_page(sentry.base, sentry.size, sentry.type, 0);
+		err =
+		    mtrr_add_page(sentry.base, sentry.size, sentry.type, false);
 		break;
 	case MTRRIOC_DEL_PAGE_ENTRY:
 #ifdef CONFIG_COMPAT

arch/x86/kernel/cpu/mtrr/main.c +3 -3

index 9abbdf7..591bd96 100644
@@ -313,7 +313,7 @@ static void set_mtrr(unsigned int reg, unsigned long base,
  */
 
 int mtrr_add_page(unsigned long base, unsigned long size, 
-		  unsigned int type, char increment)
+		  unsigned int type, bool increment)
 {
 	int i, replace, error;
 	mtrr_type ltype;

@@ -396,7 +396,7 @@ int mtrr_add_page(unsigned long base, unsigned long size,
 		if (likely(replace < 0))
 			usage_table[i] = 1;
 		else {
-			usage_table[i] = usage_table[replace] + !!increment;
+			usage_table[i] = usage_table[replace] + increment;
 			if (unlikely(replace != i)) {
 				set_mtrr(replace, 0, 0, 0);
 				usage_table[replace] = 0;

@@ -462,7 +462,7 @@ static int mtrr_check(unsigned long base, unsigned long size)
 
 int
 mtrr_add(unsigned long base, unsigned long size, unsigned int type,
-	 char increment)
+	 bool increment)
 {
 	if (mtrr_check(base, size))
 		return -EINVAL;

arch/x86/kernel/cpu/mtrr/mtrr.h +2 -4

index 289dfe6..54347e9 100644
@@ -2,10 +2,8 @@
  * local mtrr defines.
  */
 
-#ifndef TRUE
-#define TRUE  1
-#define FALSE 0
-#endif
+#include <linux/types.h>
+#include <linux/stddef.h>
 
 #define MTRRcap_MSR     0x0fe
 #define MTRRdefType_MSR 0x2ff

include/asm-x86/mtrr.h +4 -4

index e8320e4..262670e 100644
@@ -89,9 +89,9 @@ struct mtrr_gentry
 extern void mtrr_save_fixed_ranges(void *);
 extern void mtrr_save_state(void);
 extern int mtrr_add (unsigned long base, unsigned long size,
-		     unsigned int type, char increment);
+		     unsigned int type, bool increment);
 extern int mtrr_add_page (unsigned long base, unsigned long size,
-		     unsigned int type, char increment);
+		     unsigned int type, bool increment);
 extern int mtrr_del (int reg, unsigned long base, unsigned long size);
 extern int mtrr_del_page (int reg, unsigned long base, unsigned long size);
 extern void mtrr_centaur_report_mcr(int mcr, u32 lo, u32 hi);

@@ -101,12 +101,12 @@ extern void mtrr_bp_init(void);
 #define mtrr_save_fixed_ranges(arg) do {} while (0)
 #define mtrr_save_state() do {} while (0)
 static __inline__ int mtrr_add (unsigned long base, unsigned long size,
-				unsigned int type, char increment)
+				unsigned int type, bool increment)
 {
     return -ENODEV;
 }
 static __inline__ int mtrr_add_page (unsigned long base, unsigned long size,
-				unsigned int type, char increment)
+				unsigned int type, bool increment)
 {
     return -ENODEV;
 }

Richard Knutsson

2007-10-31 15:34 UTC #1 lore raw

Quoted 13 lines · Paul Jimenez

This is a janitorish patch to 1) remove private TRUE/FALSE #def's in favor of using the standard enum from linux/stddef.h and 2) switch the variables holding those values to type 'bool' (from linux/types.h) since it both seems more appropriate and allows for potentially better optimization.
As a truly minor aside, I removed a couple of comments documenting a 'do_safe' parameter that seems to no longer exist.
Signed-off-by Paul Jimenez <[email protected]>

<snip>

Quoted 14 lines

arch/x86/kernel/cpu/mtrr/main.c +1 -1

index 9abbdf7..591bd96 100644
@@ -313,7 +313,7 @@ static void set_mtrr(unsigned int reg, unsigned long base,
  */
 
 int mtrr_add_page(unsigned long base, unsigned long size, 
-		  unsigned int type, char increment)
+		  unsigned int type, bool increment)
 {
 	int i, replace, error;
 	mtrr_type ltype;

arch/x86/kernel/cpu/mtrr/main.c int mtrr_add_page(unsigned long base, unsigned long size, · line 400

Quoted 6 lines

arch/x86/kernel/cpu/mtrr/main.c +1 -1

@@ -396,7 +396,7 @@ int mtrr_add_page(unsigned long base, unsigned long size,
 		if (likely(replace < 0))
 			usage_table[i] = 1;
 		else {
-			usage_table[i] = usage_table[replace] + !!increment;
+			usage_table[i] = usage_table[replace] + increment;

This seems a bit strange, using a boolean as an integer (yes I know, it works but semantically...). What about:

+ usage_table[i] = usage_table[replace];
+ usage_table[i] += increment ? 1 : 0;
?

Quoted 17 lines

arch/x86/kernel/cpu/mtrr/main.c +0 -0

 			if (unlikely(replace != i)) {
 				set_mtrr(replace, 0, 0, 0);

arch/x86/kernel/cpu/mtrr/main.c +1 -1

 				usage_table[replace] = 0;

@@ -462,7 +462,7 @@ static int mtrr_check(unsigned long base, unsigned long size)
 
 int
 mtrr_add(unsigned long base, unsigned long size, unsigned int type,
-	 char increment)
+	 bool increment)
 {
 	if (mtrr_check(base, size))
 		return -EINVAL;

arch/x86/kernel/cpu/mtrr/mtrr.h +0 -0

index 289dfe6..54347e9 100644

arch/x86/kernel/cpu/mtrr/mtrr.h line 7

Quoted 10 lines

arch/x86/kernel/cpu/mtrr/mtrr.h +2 -4

@@ -2,10 +2,8 @@
  * local mtrr defines.
  */
 
-#ifndef TRUE
-#define TRUE  1
-#define FALSE 0
-#endif
+#include <linux/types.h>
+#include <linux/stddef.h>

Isn't those included by default?

Quoted 25 lines

arch/x86/kernel/cpu/mtrr/mtrr.h +0 -0

 #define MTRRcap_MSR     0x0fe
 #define MTRRdefType_MSR 0x2ff

include/asm-x86/mtrr.h +3 -3

index e8320e4..262670e 100644
@@ -89,9 +89,9 @@ struct mtrr_gentry
 extern void mtrr_save_fixed_ranges(void *);
 extern void mtrr_save_state(void);
 extern int mtrr_add (unsigned long base, unsigned long size,
-		     unsigned int type, char increment);
+		     unsigned int type, bool increment);
 extern int mtrr_add_page (unsigned long base, unsigned long size,
-		     unsigned int type, char increment);
+		     unsigned int type, bool increment);
 extern int mtrr_del (int reg, unsigned long base, unsigned long size);
 extern int mtrr_del_page (int reg, unsigned long base, unsigned long size);
 extern void mtrr_centaur_report_mcr(int mcr, u32 lo, u32 hi);

@@ -101,12 +101,12 @@ extern void mtrr_bp_init(void);
 #define mtrr_save_fixed_ranges(arg) do {} while (0)
 #define mtrr_save_state() do {} while (0)
 static __inline__ int mtrr_add (unsigned long base, unsigned long size,
-				unsigned int type, char increment)
+				unsigned int type, bool increment)

include/asm-x86/mtrr.h extern void mtrr_bp_init(void);

Quoted 10 lines

include/asm-x86/mtrr.h +1 -2

 {
     return -ENODEV;
 }
 static __inline__ int mtrr_add_page (unsigned long base, unsigned long size,
-				unsigned int type, char increment)
+				unsigned int type, bool increment)
 {
     return -ENODEV;
 }
-

The rest looks good :)

Richard Knutsson

Lennart Sorensen

2007-10-31 16:20 UTC #2 lore raw

Quoted 1 line · Richard Knutsson

Paul Jimenez wrote:

....

Review

Quoted 8 lines

arch/x86/kernel/cpu/mtrr/main.c int mtrr_add_page(unsigned long base, unsigned long size, · line 399
Quoted 3 lines
arch/x86/kernel/cpu/mtrr/main.c +1 -1
-			usage_table[i] = usage_table[replace] + !!increment;
+			usage_table[i] = usage_table[replace] + increment;
This seems a bit strange, using a boolean as an integer (yes I know, it works but semantically...). What about:
(quoted diff) +2 -0
+			usage_table[i] = usage_table[replace];
+			usage_table[i] += increment ? 1 : 0;

What is wrong with:
usage_table[i] = usage_table[replace];
if (increment)

	usage_table[i]++;

I hate code with the '?' operator in general. It's a conditional either way, and at least this way you wouldn't even have to do a store in one of the two cases 9although if the compiler can't figure out that is the case already, then it really sucks).

Pauls code at least didn't involve a conditional at all, but on the other hand is less clear to read.

--
Len Sorensen

Richard Knutsson

2007-10-31 20:32 UTC #3 lore raw

Quoted 19 lines · Lennart Sorensen

Quoted 2 lines · Richard Knutsson
Paul Jimenez wrote:
....
Review
Quoted 9 lines
arch/x86/kernel/cpu/mtrr/main.c int mtrr_add_page(unsigned long base, unsigned long size, · line 399
Quoted 4 lines
arch/x86/kernel/cpu/mtrr/main.c +1 -1
-			usage_table[i] = usage_table[replace] + !!increment;
+			usage_table[i] = usage_table[replace] + increment;
This seems a bit strange, using a boolean as an integer (yes I know, it works but semantically...). What about:
(quoted diff) +2 -0
+			usage_table[i] = usage_table[replace];
+			usage_table[i] += increment ? 1 : 0;
What is wrong with:
usage_table[i] = usage_table[replace];
if (increment)

Quoted 7 lines

	usage_table[i]++;
I hate code with the '?' operator in general. It's a conditional either way, and at least this way you wouldn't even have to do a store in one of the two cases 9although if the compiler can't figure out that is the case already, then it really sucks).

I kind of like them :)
But you are right, since the 'else' doesn't do anything, an if() is more clear.

Quoted 3 lines

Pauls code at least didn't involve a conditional at all, but on the other hand is less clear to read.

Mm, I know. But on the upside, many places had/have some sort of a = b ? TRUE : FALSE, which are getting cleaned out.

Richard Knutsson

Paul Jimenez

author

2007-10-31 16:28 UTC #4 lore raw

In reply to Richard Knutsson

Quoted 11 lines · Richard Knutsson

arch/x86/kernel/cpu/mtrr/main.c int mtrr_add_page(unsigned long base, unsigned long size, · line 399
Quoted 6 lines
arch/x86/kernel/cpu/mtrr/main.c +1 -1
 		if (likely(replace < 0))
 			usage_table[i] = 1;
 		else {
-			usage_table[i] = usage_table[replace] + !!increment;
+			usage_table[i] = usage_table[replace] + increment;
This seems a bit strange, using a boolean as an integer (yes I know, it works but semantically...). What about:
(quoted diff) +2 -0
+			usage_table[i] = usage_table[replace];
+			usage_table[i] += increment ? 1 : 0;

Quoted 1 line

?

I've got no strong feelings either way.

  usage_table[i] = usage_table[replace];
  if (increment) usage_table[i]++;

...would work just fine too.

Quoted 7 lines

Quoted 7 lines
index 289dfe6..54347e9 100644
arch/x86/kernel/cpu/mtrr/mtrr.h +0 -0
@@ -2,10 +2,8 @@
  * local mtrr defines.
  */
 

Quoted 8 lines

arch/x86/kernel/cpu/mtrr/mtrr.h line 7
Quoted 7 lines
arch/x86/kernel/cpu/mtrr/mtrr.h +2 -4
-#ifndef TRUE
-#define TRUE  1
-#define FALSE 0
-#endif
+#include <linux/types.h>
+#include <linux/stddef.h>
 
Isn't those included by default?

Are they? I couldn't find the #includes so I put them in, figuring that at worst cpp will strip them.

Quoted 3 lines

The rest looks good :)
Richard Knutsson

Cool. Any idea what maintainer I should send this to? I'm not sure Mr. Gooch is still actively the mtrr maintainer. Maybe the new x86 maintainers could apply it?

--pj

Richard Knutsson

2007-10-31 20:56 UTC #5 lore raw

Quoted 18 lines · Paul Jimenez

Quoted 11 lines · Richard Knutsson
arch/x86/kernel/cpu/mtrr/main.c int mtrr_add_page(unsigned long base, unsigned long size, · line 399
Quoted 7 lines
arch/x86/kernel/cpu/mtrr/main.c +1 -1
 		if (likely(replace < 0))
 			usage_table[i] = 1;
 		else {
-			usage_table[i] = usage_table[replace] + !!increment;
+			usage_table[i] = usage_table[replace] + increment;
This seems a bit strange, using a boolean as an integer (yes I know, it works but semantically...). What about:
(quoted diff) +2 -0
+			usage_table[i] = usage_table[replace];
+			usage_table[i] += increment ? 1 : 0;
Quoted 2 lines
?
I've got no strong feelings either way.

Quoted 5 lines

  usage_table[i] = usage_table[replace];
  if (increment) usage_table[i]++;

...would work just fine too.

Mm, looks even better IMO. (with the usage_table[i]++; on its own line ;) )

Quoted 19 lines

Quoted 8 lines
Quoted 8 lines
index 289dfe6..54347e9 100644
arch/x86/kernel/cpu/mtrr/mtrr.h +0 -0
@@ -2,10 +2,8 @@
  * local mtrr defines.
  */
 
Quoted 10 lines
arch/x86/kernel/cpu/mtrr/mtrr.h
Quoted 8 lines
arch/x86/kernel/cpu/mtrr/mtrr.h +2 -4
-#ifndef TRUE
-#define TRUE  1
-#define FALSE 0
-#endif
+#include <linux/types.h>
+#include <linux/stddef.h>
 
 
Isn't those included by default?

Quoted 2 lines

Are they? I couldn't find the #includes so I put them in, figuring that at worst cpp will strip them.

They are #included by linux/kernel.h... But it may be as well to include them, avoiding the dependence on the overgrown kernel.h.

Quoted 9 lines

Quoted 5 lines
The rest looks good :)
Richard Knutsson
Cool. Any idea what maintainer I should send this to? I'm not sure Mr. Gooch is still actively the mtrr maintainer. Maybe the new x86 maintainers could apply it?

Sorry, not really. Did a 'git log arch/x86/kernel/cpu/mtrr/', but it seems there is no loges before the move to arch/x86/... Anyway, you may like to add Thomas Gleixner and Ingo Molnar, they have signed-off 3 out of the 4 remaining commits (otherwise they should know where to send it).

Richard Knutsson

Paul Jimenez

author

2007-11-02 20:14 UTC #6 lore raw

PATCH 1/1 mtrr use type bool [RESEND]

(Resending and copying the x86 maintainers since I'm not sure that Richard Gooch is still actively the mtrr maintainer. Also incorporated feedback from Richard Knutsson and Lennart Sorensen.)

As a truly minor aside, I removed a couple of comments documenting a 'do_safe' parameter that seems to no longer exist.

Signed-off-by Paul Jimenez <[email protected]>

arch/x86/kernel/cpu/mtrr/amd.c +0 -2

index 0949cdb..ee2331b 100644
@@ -53,8 +53,6 @@ static void amd_set_mtrr(unsigned int reg, unsigned long base,
     <base> The base address of the region.
     <size> The size of the region. If this is 0 the region is disabled.
     <type> The type of the region.
-    <do_safe> If TRUE, do the change safely. If FALSE, safety measures should
-    be done externally.
     [RETURNS] Nothing.
 */
 {

arch/x86/kernel/cpu/mtrr/generic.c +7 -9

index 992f08d..1c331c3 100644
@@ -188,7 +188,7 @@ static inline void k8_enable_fixed_iorrs(void)
  * \param changed pointer which indicates whether the MTRR needed to be changed
  * \param msrwords pointer to the MSR values which the MSR should have
  */
-static void set_fixed_range(int msr, int * changed, unsigned int * msrwords)
+static void set_fixed_range(int msr, bool *changed, unsigned int *msrwords)
 {
 	unsigned lo, hi;

@@ -200,7 +200,7 @@ static void set_fixed_range(int msr, int * changed, unsigned int * msrwords)
 		    ((msrwords[0] | msrwords[1]) & K8_MTRR_RDMEM_WRMEM_MASK))
 			k8_enable_fixed_iorrs();
 		mtrr_wrmsr(msr, msrwords[0], msrwords[1]);
-		*changed = TRUE;
+		*changed = true;
 	}
 }

@@ -260,7 +260,7 @@ static void generic_get_mtrr(unsigned int reg, unsigned long *base,
 static int set_fixed_ranges(mtrr_type * frs)
 {
 	unsigned long long *saved = (unsigned long long *) frs;
-	int changed = FALSE;
+	bool changed = false;
 	int block=-1, range;
 
 	while (fixed_range_blocks[++block].ranges)

@@ -273,17 +273,17 @@ static int set_fixed_ranges(mtrr_type * frs)
 
 /*  Set the MSR pair relating to a var range. Returns TRUE if
     changes are made  */
-static int set_mtrr_var_ranges(unsigned int index, struct mtrr_var_range *vr)
+static bool set_mtrr_var_ranges(unsigned int index, struct mtrr_var_range *vr)
 {
 	unsigned int lo, hi;
-	int changed = FALSE;
+	bool changed = false;
 
 	rdmsr(MTRRphysBase_MSR(index), lo, hi);
 	if ((vr->base_lo & 0xfffff0ffUL) != (lo & 0xfffff0ffUL)
 	    || (vr->base_hi & (size_and_mask >> (32 - PAGE_SHIFT))) !=
 		(hi & (size_and_mask >> (32 - PAGE_SHIFT)))) {
 		mtrr_wrmsr(MTRRphysBase_MSR(index), vr->base_lo, vr->base_hi);
-		changed = TRUE;
+		changed = true;
 	}
 
 	rdmsr(MTRRphysMask_MSR(index), lo, hi);

@@ -292,7 +292,7 @@ static int set_mtrr_var_ranges(unsigned int index, struct mtrr_var_range *vr)
 	    || (vr->mask_hi & (size_and_mask >> (32 - PAGE_SHIFT))) !=
 		(hi & (size_and_mask >> (32 - PAGE_SHIFT)))) {
 		mtrr_wrmsr(MTRRphysMask_MSR(index), vr->mask_lo, vr->mask_hi);
-		changed = TRUE;
+		changed = true;
 	}
 	return changed;
 }

@@ -417,8 +417,6 @@ static void generic_set_mtrr(unsigned int reg, unsigned long base,
     <base> The base address of the region.
     <size> The size of the region. If this is 0 the region is disabled.
     <type> The type of the region.
-    <do_safe> If TRUE, do the change safely. If FALSE, safety measures should
-    be done externally.
     [RETURNS] Nothing.
 */
 {

arch/x86/kernel/cpu/mtrr/if.c +8 -7

index c7d8f17..1453568 100644
@@ -37,7 +37,7 @@ const char *mtrr_attrib_to_str(int x)
 
 static int
 mtrr_file_add(unsigned long base, unsigned long size,
-	      unsigned int type, char increment, struct file *file, int page)
+	      unsigned int type, bool increment, struct file *file, int page)
 {
 	int reg, max;
 	unsigned int *fcount = FILE_FCOUNT(file);

@@ -55,7 +55,7 @@ mtrr_file_add(unsigned long base, unsigned long size,
 		base >>= PAGE_SHIFT;
 		size >>= PAGE_SHIFT;
 	}
-	reg = mtrr_add_page(base, size, type, 1);
+	reg = mtrr_add_page(base, size, type, true);
 	if (reg >= 0)
 		++fcount[reg];
 	return reg;

@@ -141,7 +141,7 @@ mtrr_write(struct file *file, const char __user *buf, size_t len, loff_t * ppos)
 		size >>= PAGE_SHIFT;
 		err =
 		    mtrr_add_page((unsigned long) base, (unsigned long) size, i,
-				  1);
+				  true);
 		if (err < 0)
 			return err;
 		return len;

@@ -217,7 +217,7 @@ mtrr_ioctl(struct file *file, unsigned int cmd, unsigned long __arg)
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
 		err =
-		    mtrr_file_add(sentry.base, sentry.size, sentry.type, 1,
+		    mtrr_file_add(sentry.base, sentry.size, sentry.type, true,
 				  file, 0);
 		break;
 	case MTRRIOC_SET_ENTRY:

@@ -226,7 +226,7 @@ mtrr_ioctl(struct file *file, unsigned int cmd, unsigned long __arg)
 #endif
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
-		err = mtrr_add(sentry.base, sentry.size, sentry.type, 0);
+		err = mtrr_add(sentry.base, sentry.size, sentry.type, false);
 		break;
 	case MTRRIOC_DEL_ENTRY:
 #ifdef CONFIG_COMPAT

@@ -270,7 +270,7 @@ mtrr_ioctl(struct file *file, unsigned int cmd, unsigned long __arg)
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
 		err =
-		    mtrr_file_add(sentry.base, sentry.size, sentry.type, 1,
+		    mtrr_file_add(sentry.base, sentry.size, sentry.type, true,
 				  file, 1);
 		break;
 	case MTRRIOC_SET_PAGE_ENTRY:

@@ -279,7 +279,8 @@ mtrr_ioctl(struct file *file, unsigned int cmd, unsigned long __arg)
 #endif
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
-		err = mtrr_add_page(sentry.base, sentry.size, sentry.type, 0);
+		err =
+		    mtrr_add_page(sentry.base, sentry.size, sentry.type, false);
 		break;
 	case MTRRIOC_DEL_PAGE_ENTRY:
 #ifdef CONFIG_COMPAT

arch/x86/kernel/cpu/mtrr/main.c +5 -3

index 9abbdf7..f6fb863 100644
@@ -313,7 +313,7 @@ static void set_mtrr(unsigned int reg, unsigned long base,
  */
 
 int mtrr_add_page(unsigned long base, unsigned long size, 
-		  unsigned int type, char increment)
+		  unsigned int type, bool increment)
 {
 	int i, replace, error;
 	mtrr_type ltype;

@@ -396,7 +396,9 @@ int mtrr_add_page(unsigned long base, unsigned long size,
 		if (likely(replace < 0))
 			usage_table[i] = 1;
 		else {
-			usage_table[i] = usage_table[replace] + !!increment;
+			usage_table[i] = usage_table[replace];
+			if (increment) 
+				usage_table[i]++;
 			if (unlikely(replace != i)) {
 				set_mtrr(replace, 0, 0, 0);
 				usage_table[replace] = 0;

@@ -462,7 +464,7 @@ static int mtrr_check(unsigned long base, unsigned long size)
 
 int
 mtrr_add(unsigned long base, unsigned long size, unsigned int type,
-	 char increment)
+	 bool increment)
 {
 	if (mtrr_check(base, size))
 		return -EINVAL;

arch/x86/kernel/cpu/mtrr/mtrr.h +2 -4

index 289dfe6..54347e9 100644
@@ -2,10 +2,8 @@
  * local mtrr defines.
  */
 
-#ifndef TRUE
-#define TRUE  1
-#define FALSE 0
-#endif
+#include <linux/types.h>
+#include <linux/stddef.h>
 
 #define MTRRcap_MSR     0x0fe
 #define MTRRdefType_MSR 0x2ff

include/asm-x86/mtrr.h +4 -4

index e8320e4..262670e 100644
@@ -89,9 +89,9 @@ struct mtrr_gentry
 extern void mtrr_save_fixed_ranges(void *);
 extern void mtrr_save_state(void);
 extern int mtrr_add (unsigned long base, unsigned long size,
-		     unsigned int type, char increment);
+		     unsigned int type, bool increment);
 extern int mtrr_add_page (unsigned long base, unsigned long size,
-		     unsigned int type, char increment);
+		     unsigned int type, bool increment);
 extern int mtrr_del (int reg, unsigned long base, unsigned long size);
 extern int mtrr_del_page (int reg, unsigned long base, unsigned long size);
 extern void mtrr_centaur_report_mcr(int mcr, u32 lo, u32 hi);

@@ -101,12 +101,12 @@ extern void mtrr_bp_init(void);
 #define mtrr_save_fixed_ranges(arg) do {} while (0)
 #define mtrr_save_state() do {} while (0)
 static __inline__ int mtrr_add (unsigned long base, unsigned long size,
-				unsigned int type, char increment)
+				unsigned int type, bool increment)
 {
     return -ENODEV;
 }
 static __inline__ int mtrr_add_page (unsigned long base, unsigned long size,
-				unsigned int type, char increment)
+				unsigned int type, bool increment)
 {
     return -ENODEV;
 }