Messages in this thread | | | Date | Wed, 31 Oct 2007 21:49:56 +0100 | From | Richard Knutsson <> | 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
- 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/
| |