This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [patch 00/26] Xen-paravirt_ops: Xen guest implementation

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Jeremy Fitzhardinge <jeremy@xxxxxxxx>, Ingo Molnar <mingo@xxxxxxx>, Andi Kleen <ak@xxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, virtualization@xxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxx, Chris Wright <chrisw@xxxxxxxxxxxx>, Zachary Amsden <zach@xxxxxxxxxx>, Rusty Russell <rusty@xxxxxxxxxxxxxxx>
Subject: [Xen-devel] Re: [patch 00/26] Xen-paravirt_ops: Xen guest implementation for paravirt_ops interface
From: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Date: Fri, 16 Mar 2007 12:26:27 -0700
Delivery-date: Fri, 16 Mar 2007 12:25:35 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20070316185923.GA15209@xxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20070301232443.195603797@xxxxxxxx> <20070316092137.GL23174@xxxxxxx> <45FAD35F.8040404@xxxxxxxx> <20070316185923.GA15209@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird (X11/20070302)
Christoph Hellwig wrote:
> On Fri, Mar 16, 2007 at 10:26:55AM -0700, Jeremy Fitzhardinge wrote:
>> +    .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.


>> +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);
>> +}
>> +
>> +#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.


Xen-devel mailing list