Re: [PATCH RFC v2 5/6] KVM: moving dirty bitmaps to user space

[Takuya Yoshikawa <yoshikawa.takuya-gVGce1chcLdL9jVzuh4AOg@public.gmane.org>]

(2010/04/21 20:26), Avi Kivity wrote:

>> r = 0;
>> @@ -1858,7 +1866,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
>> if (memslot->is_dirty) {
>> kvm_flush_remote_tlbs(kvm);
>> n = kvm_dirty_bitmap_bytes(memslot);
>> - memset(memslot->dirty_bitmap, 0, n);
>> + clear_user(memslot->dirty_bitmap, n);
>> memslot->is_dirty = false;
>
> Does this need an error check?

Yes, sorry I forgot.

>
>
>> @@ -468,8 +480,12 @@ void kvm_free_physmem(struct kvm *kvm)
>> int i;
>> struct kvm_memslots *slots = kvm->memslots;
>>
>> - for (i = 0; i< slots->nmemslots; ++i)
>> + for (i = 0; i< slots->nmemslots; ++i) {
>> + /* We don't munmap dirty bitmaps by ourselves. */
>
> Why not? If we allocated them, we have to free them.

In the case of VM destruction, when qemu process exits, it conflicts
with the work of the usual (process) destructor's job.

I struggled with this problem before sending the first version.

So I have to differentiate the slot release and qemu process exit.



>
>> + slots->memslots[i].dirty_bitmap = NULL;
>> + slots->memslots[i].dirty_bitmap_old = NULL;
>> kvm_free_physmem_slot(&slots->memslots[i], NULL);
>> + }
>>
>>
>> +/*
>> + * Please use generic *_user bitops once they become available.
>> + * Be careful setting the bit won't be done atomically.
>> + */
>
> Please introduce the user bitops as part of this patchset.
>

OK, I will do in the next version. In this RFC, I would be happy if I can
know the overall design is right or not.