On 15.01.2018 18:03, Claudio Imbrenda wrote:
> This is a fix for several issues that were found in the original code
> for storage attributes migration.
>
> Now no bitmap is allocated to keep track of dirty storage attributes;
> the extra bits of the per-memslot bitmap that are always present anyway
> are now used for this purpose.
>
> The code has also been refactored a little to improve readability.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxxxxxxx>
> Fixes: 190df4a212a ("KVM: s390: CMMA tracking, ESSA emulation, migration mode")
> Fixes: 4036e3874a1 ("KVM: s390: ioctls to get and set guest storage attributes")
Did Christians patches go into stable, and should yours too?
> ---
> static int kvm_s390_vm_start_migration(struct kvm *kvm)
> {
[...]
> if (kvm->arch.use_cmma) {
[...]
> -
> - mgs->bitmap_size = ram_pages;
> - atomic64_set(&mgs->dirty_pages, ram_pages);
> /* mark all the pages in active slots as dirty */
> for (slotnr = 0; slotnr < slots->used_slots; slotnr++) {
> ms = slots->memslots + slotnr;
> - bitmap_set(mgs->pgste_bitmap, ms->base_gfn, ms->npages);
> + /*
> + * The second half of the bitmap is only used on x86,
> + * and would be wasted otherwise, so we put it to good
> + * use here to keep track of the state of the storage
> + * attributes.
> + */
> + memset(_cmma_bitmap(ms), 0xFF,
> + kvm_dirty_bitmap_bytes(ms));
On your branch, the second line is not indented properly.
Also s/0xFF/0xff/
> + ram_pages += ms->npages;
> }
>
> + atomic64_set(&kvm->arch.cmma_dirty_pages, ram_pages);
> + atomic_set(&kvm->arch.migration_mode, 1);
> kvm_s390_sync_request_broadcast(kvm, KVM_REQ_START_MIGRATION);
> + } else {
> + atomic_set(&kvm->arch.migration_mode, 1);
For stop migration, you do that unconditionally, why don't you move this
in front of the if?
> }
> return 0;
> }
> @@ -834,19 +820,12 @@ static int kvm_s390_vm_start_migration(struct kvm *kvm)
> */
> static int kvm_s390_vm_stop_migration(struct kvm *kvm)
> {
> - struct kvm_s390_migration_state *mgs;
> -
> /* migration mode already disabled */
> - if (!kvm->arch.migration_state)
> + if (!atomic_read(&kvm->arch.migration_mode))
> return 0;
> - mgs = kvm->arch.migration_state;
> - kvm->arch.migration_state = NULL;
> -
> - if (kvm->arch.use_cmma) {
> + atomic_set(&kvm->arch.migration_mode, 0);
> + if (kvm->arch.use_cmma)
> kvm_s390_sync_request_broadcast(kvm, KVM_REQ_STOP_MIGRATION);
> - vfree(mgs->pgste_bitmap);
> - }
> - kfree(mgs);
> return 0;
> }
>
The rest below will take me some time.
However, it is much more readable than before!
Attachment:
signature.asc
Description: OpenPGP digital signature
