2007-10-31 03:57:28

by Paul Jimenez

[permalink] [raw]
Subject: [PATCH] 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]>


diff --git a/arch/x86/kernel/cpu/mtrr/amd.c b/arch/x86/kernel/cpu/mtrr/amd.c
index 0949cdb..ee2331b 100644
--- a/arch/x86/kernel/cpu/mtrr/amd.c
+++ b/arch/x86/kernel/cpu/mtrr/amd.c
@@ -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.
*/
{
diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index 992f08d..1c331c3 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -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.
*/
{
diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
index c7d8f17..1453568 100644
--- a/arch/x86/kernel/cpu/mtrr/if.c
+++ b/arch/x86/kernel/cpu/mtrr/if.c
@@ -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
diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 9abbdf7..591bd96 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -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;
diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.h b/arch/x86/kernel/cpu/mtrr/mtrr.h
index 289dfe6..54347e9 100644
--- a/arch/x86/kernel/cpu/mtrr/mtrr.h
+++ b/arch/x86/kernel/cpu/mtrr/mtrr.h
@@ -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
diff --git a/include/asm-x86/mtrr.h b/include/asm-x86/mtrr.h
index e8320e4..262670e 100644
--- a/include/asm-x86/mtrr.h
+++ b/include/asm-x86/mtrr.h
@@ -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;
}


2007-10-31 15:34:34

by Richard Knutsson

[permalink] [raw]
Subject: Re: [PATCH] mtrr use type bool

Paul Jimenez wrote:
> 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>
> diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
> index 9abbdf7..591bd96 100644
> --- a/arch/x86/kernel/cpu/mtrr/main.c
> +++ b/arch/x86/kernel/cpu/mtrr/main.c
> @@ -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;
>
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;
?

> 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;
> diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.h b/arch/x86/kernel/cpu/mtrr/mtrr.h
> index 289dfe6..54347e9 100644
> --- a/arch/x86/kernel/cpu/mtrr/mtrr.h
> +++ b/arch/x86/kernel/cpu/mtrr/mtrr.h
> @@ -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?
>
> #define MTRRcap_MSR 0x0fe
> #define MTRRdefType_MSR 0x2ff
> diff --git a/include/asm-x86/mtrr.h b/include/asm-x86/mtrr.h
> index e8320e4..262670e 100644
> --- a/include/asm-x86/mtrr.h
> +++ b/include/asm-x86/mtrr.h
> @@ -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;
> }
> -
The rest looks good :)

Richard Knutsson

2007-10-31 16:20:27

by Lennart Sorensen

[permalink] [raw]
Subject: Re: [PATCH] mtrr use type bool

On Wed, Oct 31, 2007 at 04:27:31PM +0100, Richard Knutsson wrote:
> Paul Jimenez wrote:
....
> >- 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;

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

2007-10-31 16:28:10

by Paul Jimenez

[permalink] [raw]
Subject: Re: [PATCH] mtrr use type bool

On Wednesday, Oct 31, 2007, Richard Knutsson writes:
>> 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;
>?

I've got no strong feelings either way.

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

...would work just fine too.

>> index 289dfe6..54347e9 100644
>> --- a/arch/x86/kernel/cpu/mtrr/mtrr.h
>> +++ b/arch/x86/kernel/cpu/mtrr/mtrr.h
>> @@ -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?

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

>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

2007-10-31 20:32:21

by Richard Knutsson

[permalink] [raw]
Subject: Re: [PATCH] mtrr use type bool

Lennart Sorensen wrote:
> On Wed, Oct 31, 2007 at 04:27:31PM +0100, Richard Knutsson wrote:
>
>> Paul Jimenez wrote:
>>
> ....
>
>>> - 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;
>>
>
> 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).
>
I kind of like them :)
But you are right, since the 'else' doesn't do anything, an if() is more
clear.
> 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

2007-10-31 20:56:44

by Richard Knutsson

[permalink] [raw]
Subject: Re: [PATCH] mtrr use type bool

Paul Jimenez wrote:
> On Wednesday, Oct 31, 2007, Richard Knutsson writes:
>
>>> 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;
>> ?
>>
>
> I've got no strong feelings either way.
>
> 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 ;) )
>
>>> index 289dfe6..54347e9 100644
>>> --- a/arch/x86/kernel/cpu/mtrr/mtrr.h
>>> +++ b/arch/x86/kernel/cpu/mtrr/mtrr.h
>>> @@ -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?
>>
>
> 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.
>
>> 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

2007-11-02 20:14:18

by Paul Jimenez

[permalink] [raw]
Subject: [PATCH] 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.)

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


diff --git a/arch/x86/kernel/cpu/mtrr/amd.c b/arch/x86/kernel/cpu/mtrr/amd.c
index 0949cdb..ee2331b 100644
--- a/arch/x86/kernel/cpu/mtrr/amd.c
+++ b/arch/x86/kernel/cpu/mtrr/amd.c
@@ -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.
*/
{
diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index 992f08d..1c331c3 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -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.
*/
{
diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
index c7d8f17..1453568 100644
--- a/arch/x86/kernel/cpu/mtrr/if.c
+++ b/arch/x86/kernel/cpu/mtrr/if.c
@@ -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
diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 9abbdf7..f6fb863 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -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;
diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.h b/arch/x86/kernel/cpu/mtrr/mtrr.h
index 289dfe6..54347e9 100644
--- a/arch/x86/kernel/cpu/mtrr/mtrr.h
+++ b/arch/x86/kernel/cpu/mtrr/mtrr.h
@@ -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
diff --git a/include/asm-x86/mtrr.h b/include/asm-x86/mtrr.h
index e8320e4..262670e 100644
--- a/include/asm-x86/mtrr.h
+++ b/include/asm-x86/mtrr.h
@@ -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;
}