On Wed, Sep 25, 2024 at 04:34:57PM +0100, David Woodhouse wrote: > On Wed, 2024-09-25 at 15:15 +0000, Suleiman Souhlal wrote: > > Yes, that's a good way to put it: The excess steal time isn't actually > > being stolen from anyone. > > And since it's not being stolen from anyone, isn't the right thing to do > > to drop it? > > It's being stolen from the system, isn't it? Just not any specific > userspace process? I guess it depends what you mean by "stolen". I would argue that it's not stolen from anyone since the time isn't actually counted anywhere. > > If we have separate "end of outgoing task" and "start of incoming task" > timestamps, surely the time between those two must be accounted > *somewhere*? Not exactly. clock_task is essentially frozen until the next update_rq_clock(), at which point we'll look at how much sched_clock_cpu advanced and subtract how much steal time advanced. The two things are done in separate spots (update_rq_clock() and update_rq_clock_task()), indepedently (which is where the race is happening). As far as I can tell, the time between the two isn't really accounted anywhere. The "end of outgoing task" and "start of incoming task" timestamps should end up being the same. > > > There might still be extra steal time that doesn't exceed the current > > 'delta' from the race between reading the two values, that would still > > be erroneously accounted to the outgoing task, which this patch doesn't > > address, but we know that any steal > delta is from this race and should > > be dropped. > > Well that's what we want the atomic paired read for :) Right, but I don't think it's that simple. We aren't only reading memory but also a clock. It might be possible to address this with a mechanism like rseq, but that would be a much bigger patch set than the current one (and I don't think anyone has ever attempted to do rseq for VMs yet). (There is also another potential issue I spotted with steal time, that has to do with reading another VCPU's steal time while it's not running, but I'll start a separate discussion about that with a different patch set.) Thanks for the discussion. -- Suleiman
