Messages in this thread | | | Date | Fri, 16 Mar 2007 12:26:27 -0700 | From | Jeremy Fitzhardinge <> | Subject | Re: [patch 00/26] Xen-paravirt_ops: Xen guest implementation for paravirt_ops interface |
| |
Christoph Hellwig wrote: > On Fri, Mar 16, 2007 at 10:26:55AM -0700, Jeremy Fitzhardinge wrote: > >> +#ifdef CONFIG_HIGHPTE >> + .kmap_atomic_pte = native_kmap_atomic_pte, >> +#else >> + .kmap_atomic_pte = paravirt_nop, >> +#endif >> > > This is ifdefing is quite ugly. Shouldn't native_kmap_atomic_pte > just be a noop in the !CONFIG_HIGHPTE case? >
Yes, but the trouble is that asm/highmem.h simply isn't included in the !HIGHMEM case, so I can't put anything in there, and putting anything pv_ops related into linux/highmem.h isn't appropriate either.
>> -void *kmap_atomic(struct page *page, enum km_type type) >> +void *_kmap_atomic(struct page *page, enum km_type type, pgprot_t prot) >> > > We normally call our "secial" function __foo, not _foo. But in this > case it really should have a more meaningfull name like > kmap_atomic_prot anyway. >
OK.
>> +void *kmap_atomic(struct page *page, enum km_type type) >> +{ >> + return _kmap_atomic(page, type, kmap_prot); >> > > And this one should probably be an inline. >
OK, if you think it makes a difference.
>> +static inline void *native_kmap_atomic_pte(struct page *page, enum km_type type) >> +{ >> + return kmap_atomic(page, type); >> +} >> + >> +#ifndef CONFIG_PARAVIRT >> +#define kmap_atomic_pte(page, type) kmap_atomic(page, type) >> +#endif >> > > This is all getting rather ugly just for your pagetable hackery. >
Well, I could promote kmap_atomic_pte to a first-class interface, but it seems like overkill.
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/
| |