From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH RFC v2 4/6] KVM: change mark_page_dirty() to handle endian issues explicitly Date: Wed, 21 Apr 2010 14:15:13 +0300 Message-ID: <4BCEDE41.1070100@redhat.com> References: <20100420195349.dab60b1d.yoshikawa.takuya@oss.ntt.co.jp> <20100420200043.956302db.yoshikawa.takuya@oss.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org, kvm-ia64@vger.kernel.org, kvm-ppc@vger.kernel.org, fernando@oss.ntt.co.jp To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:41666 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751373Ab0DULPR (ORCPT ); Wed, 21 Apr 2010 07:15:17 -0400 In-Reply-To: <20100420200043.956302db.yoshikawa.takuya@oss.ntt.co.jp> Sender: kvm-owner@vger.kernel.org List-ID: On 04/20/2010 02:00 PM, Takuya Yoshikawa wrote: > We are now using generic___set_le_bit() to make dirty bitmaps le. > Though this works well, we have to replace __set_bit() to appropriate > uaccess function to move dirty bitmaps to user space. So this patch > splits generic___set_le_bit() and prepares for that. > > > +static int __mark_page_dirty(unsigned long nr, > + unsigned long *dirty_bitmap) > +{ > +#ifdef __BIG_ENDIAN > + nr = nr ^ BITOP_LE_SWIZZLE; > +#endif > + __set_bit(nr, dirty_bitmap); > Why not introduce __set_le_bit_user() along with __set_bit_user()? kvm shouldn't do endian corrections when they can be done in generic code. > +} > + > void mark_page_dirty(struct kvm *kvm, gfn_t gfn) > { > struct kvm_memory_slot *memslot; > @@ -1203,11 +1212,8 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn) > if (memslot&& memslot->dirty_bitmap) { > unsigned long rel_gfn = gfn - memslot->base_gfn; > > - /* avoid RMW */ > - if (!generic_test_le_bit(rel_gfn, memslot->dirty_bitmap)) { > - generic___set_le_bit(rel_gfn, memslot->dirty_bitmap); > - memslot->is_dirty = true; > - } > You're also dropping the RMW avoidance. This is fine, but deserves at least a note in the changelog (or a separate patch). > + __mark_page_dirty(rel_gfn, memslot->dirty_bitmap); > + memslot->is_dirty = true; > } > } > > -- error compiling committee.c: too many arguments to function