Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fine-grained dirty region tracking optimization issue #2

Open
PJTPJT opened this issue Dec 7, 2017 · 2 comments
Open

Fine-grained dirty region tracking optimization issue #2

PJTPJT opened this issue Dec 7, 2017 · 2 comments

Comments

@PJTPJT
Copy link
Collaborator

PJTPJT commented Dec 7, 2017

In fine-grained dirty region tracking optimization
Some diff page size is 0, it means three possibilities:

  1. Page content is from A to B and change back to A
  2. Page content is not dirty but we mark it dirty
  3. Backup too late

If in case 1 or 2, we can ignore it.
In case 3, we will transfer all page, otherwise it will missing logging some dirty page.

But the correct solution is find what dirty page we backup too late
This issue is correspond to 60623ee

@coldfunction
Copy link
Collaborator

I guess I may find a mistake in KVM module. In the old version, the kvm_guest_time_update() function gives the code as below:

kvmft_page_dirty(v->kvm, vcpu->time >> PAGE_SHIFT, shared_kaddr, 0, NULL);

memcpy(shared_kaddr + vcpu->time_offset, &vcpu->hv_clock,
	       sizeof(vcpu->hv_clock));

But in the latest version, the kvm_guest_time_update() only execute one of the statements as below:

kvmft_page_dirty(v->kvm, vcpu->time >> PAGE_SHIFT, shared_kaddr, 0, NULL);
return 0;

and return, that's all.

The old version we can find that the shared_kaddr will change after we back up the page via kvmft_page_dirty. However, the shared_kaddr doesn't assign any value to the latest version.

I think there exists other code like this smell, using kvmft_page_dirty but don't consider the address is correct or not.

@PJTPJT
Copy link
Collaborator Author

PJTPJT commented Aug 31, 2020

I guess I may find a mistake in KVM module. In the old version, the kvm_guest_time_update() function gives the code as below:

kvmft_page_dirty(v->kvm, vcpu->time >> PAGE_SHIFT, shared_kaddr, 0, NULL);

memcpy(shared_kaddr + vcpu->time_offset, &vcpu->hv_clock,
	       sizeof(vcpu->hv_clock));

But in the latest version, the kvm_guest_time_update() only execute one of the statements as below:

kvmft_page_dirty(v->kvm, vcpu->time >> PAGE_SHIFT, shared_kaddr, 0, NULL);
return 0;

and return, that's all.

The old version we can find that the shared_kaddr will change after we back up the page via kvmft_page_dirty. However, the shared_kaddr doesn't assign any value to the latest version.

I think there exists other code like this smell, using kvmft_page_dirty but don't consider the address is correct or not.

In latest kvm_guest_time_update()
It has no kvmft_page_dirty
Actually, the kvmft_page_dirty in KVM all insert on kvm_write_guest_cached or kvm_write_guest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants