On 04/03/2024 23:44, Sean Christopherson wrote:
On Tue, Feb 27, 2024, David Woodhouse wrote:
+ /* Xen has a 'Linux workaround' in do_set_timer_op() which
+ * checks for negative absolute timeout values (caused by
+ * integer overflow), and for values about 13 days in the
+ * future (2^50ns) which would be caused by jiffies
+ * overflow. For those cases, it sets the timeout 100ms in
+ * the future (not *too* soon, since if a guest really did
+ * set a long timeout on purpose we don't want to keep
+ * churning CPU time by waking it up).
+ */
I'm going to massage this slightly, partly to take advantage of reduced indentation,
but also to call out when the workaround is applied. Though in all honesty, the
extra context may just be in response to a PEBKAC on my end, as I misread the diff
multiple times.
+ if (linux_wa) {
+ if ((unlikely((int64_t)guest_abs < 0 ||
No need for a second set of parantheses around the unlikely.
+ (delta > 0 && (uint32_t) (delta >> 50) != 0)))) {
And this can all easily fit into one if-statement.
+ delta = 100 * NSEC_PER_MSEC;
+ guest_abs = guest_now + delta;
+ }
+ }
This is what I'm going to commit, holler if it looks wrong (disclaimer: I've only
compile tested at this point).
/*
* Xen has a 'Linux workaround' in do_set_timer_op() which checks for
* negative absolute timeout values (caused by integer overflow), and
* for values about 13 days in the future (2^50ns) which would be
* caused by jiffies overflow. For those cases, Xen sets the timeout
* 100ms in the future (not *too* soon, since if a guest really did
* set a long timeout on purpose we don't want to keep churning CPU
* time by waking it up). Emulate Xen's workaround when starting the
* timer in response to __HYPERVISOR_set_timer_op.
*/
if (linux_wa &&
unlikely((int64_t)guest_abs < 0 ||
(delta > 0 && (uint32_t) (delta >> 50) != 0))) {
Now that I look at it again, since the last test is simply a '!= 0', I
don't really see why the case is necessary. Perhaps lose that too.
Otherwise LGTM.
delta = 100 * NSEC_PER_MSEC;
guest_abs = guest_now + delta;
}
