Re: [PATCH RFC v2 4/6] KVM: change mark_page_dirty() to handle endian issues explicitly

[Avi Kivity <avi@redhat.com>]

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