lkml.org 
[lkml]   [2007]   [Oct]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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 <pj@place.org>
>
>
>
<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

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

\
 
 \ /
  Last update: 2007-10-31 16:37    [W:0.101 / U:0.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site