On Tue, 2022-03-08 at 18:13 +0100, Paolo Bonzini wrote: > On 3/8/22 17:59, David Woodhouse wrote: > > > > Incremental diff to the 'oneshot timers' patch looks like the first > > > > hunk of this. I'm also pondering the second hunk which actively > > > > *cancels* the pending timer on serialization. > > > > > > Hmm, why so? > > > > Don't know yet. But as I added the save/restore support to Joao's patch > > I had *assumed* that it would fail when the delta was negative, and was > > kind of surprised when it worked in the first place. So I'm sticking > > with "Don't Do That Then" as my initial response to fix it. > > Yes, I'm just talking about the second hunk. The first is clear(ish). Oh, I see. When the oneshot timer has expired and hasn't been re-armed by the guest, we should return zero as 'expires_ns' so that it doesn't get re- armed in the past (and, hopefully, immediately retriggered) when the guest is restored. Also, we don't really want the timer firing *after* the guest vCPU state has been serialized, since the newly-injected interrupt might not get migrated. Hence using hrtimer_cancel() as our check for whether it's still pending or not. > > After a kexec, the deadline for the timer is past, and that's why it > > ends up getting restored with a negative delta. After a *few* cycles of > > this it usually ends up with the timer callback never triggering. > > > > I'll stick a negative delta into the KVM selftest included in the patch > > series, instead of the nice polite '100ms in the future' that it uses > > at the moment. That ought to trigger it too, and I can instrument the > > hrtimer code to work out what's going on. Either way, I think 'Don't Do > > That Then' will continue to be the right answer:) > > Yep, let's keep both testcases through. Ultimately I think the negative one only ends up testing the kernel's hrtimer behaviour rather than KVM functionality, but I'll play with that some more and make sure I understand it.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
