Messages in this thread | | | Date | Fri, 16 Mar 2007 11:42:19 -0700 | From | Jeremy Fitzhardinge <> | Subject | Re: [patch 04/26] Xen-paravirt_ops: Add pagetable accessors to pack and unpack pagetable entries |
| |
Ingo Molnar wrote: >> +static inline pmd_t native_make_pmd(unsigned long long val) >> +{ >> + return (pmd_t) { val }; >> +} >> +static inline pte_t native_make_pte(unsigned long long val) >> +{ >> + return (pte_t) { .pte_low = val, .pte_high = (val >> 32) } ; >> +} >> > > missing newlines between inline functions. >
OK.
>> +#ifndef CONFIG_PARAVIRT >> +#define pmd_val(x) native_pmd_val(x) >> +#define __pmd(x) native_make_pmd(x) >> +#endif /* !CONFIG_PARAVIRT */ >> > > no need for the closing !CONFIG_PARAVIRT comment: this define is 2 lines > long so it's not that hard to find the start of the block. We typically > do the /* !CONFIG_XX */ comment only for larger blocks, and when > multiple #endif's intermix. >
Yeah, I tend to put them there reflexively. Its so easy for an #endif to drift away over time, and suddenly you have no idea what's going on. I agree its overkill in this case.
>> #define HPAGE_SHIFT 22 >> #include <asm-generic/pgtable-nopmd.h> >> -#endif >> +#endif /* CONFIG_X86_PAE */ >> > > (for example here the #endif comment is justified.) >
Yeah, and it probably started life much closer to the #ifdef...
J - 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/
| |