Messages in this thread | | | Date | Fri, 16 Mar 2007 18:59:23 +0000 | From | Christoph Hellwig <> | Subject | Re: [patch 00/26] Xen-paravirt_ops: Xen guest implementation for paravirt_ops interface |
| |
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?
> -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.
> +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.
> +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.
- 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/
| |