Re: [PATCH] KVM: x86: Include host suspended time in steal time.

Chao Gao
2024-07-31T01:03:24Z

>> >+static int
>> >+kvm_arch_resume_notifier(struct kvm *kvm)
>> >+{
>> >+ kvm->last_suspend_duration = ktime_get_boottime_ns() -
>> >+ kvm->suspended_time;
>>
>> Is it possible that a vCPU doesn't get any chance to run (i.e., update steal
>> time) between two suspends? In this case, only the second suspend would be
>> recorded.
>
>Good point. I'll address this.
>
>>
>> Maybe we need an infrastructure in the PM subsystem to record accumulated
>> suspended time. When updating steal time, KVM can add the additional suspended
>> time since the last update into steal_time (as how KVM deals with
>> current->sched_info.run_deley). This way, the scenario I mentioned above won't
>> be a problem and KVM needn't calculate the suspend duration for each guest. And
>> this approach can potentially benefit RISC-V and ARM as well, since they have
>> the same logic as x86 regarding steal_time.
>
>Thanks for the suggestion.
>I'm a bit wary of making a whole PM subsystem addition for such a counter, but
>maybe I can make a architecture-independent KVM change for it, with a PM
>notifier in kvm_main.c.

Sounds good.

>
>>
>> Additionally, it seems that if a guest migrates to another system after a suspend
>> and before updating steal time, the suspended time is lost during migration. I'm
>> not sure if this is a practical issue.
>
>The systems where the host suspends don't usually do VM migrations. Or at least
>the ones where we're encountering the problem this patch is trying to address
>don't (laptops).
>But even if they did, it doesn't seem that likely that the migration would
>happen over a host suspend.
>If it's ok with you, I'll put this issue aside for the time being.

I am fine with putting this issue aside.