When working with timestamps, psi is careful to only compare timestamps from cpu_clock(cpu) with the same cpu argument. However, on systems with a stable clock, that cpu argument is ignored. This means that despite psi's best efforts, there will be some cross-CPU cpu_clock() timestamp comparisons. The cross-CPU skew in cpu_clock() timestamps is generally orders of magnitude smaller than psi measurement intervals, so the cyclic times counters naturally absorb the jitter. However, if get_recent_times() executes immediately after a task on another CPU enters an active stall state, then negative skew can cause the u32 active state duration calculation to underflow. If that state had not been active since the previous execution of get_recent_times(), then times and times_prev will be equal and the very large underflow value will be passed out to collect_percpu_times(). There, it will be zero extended and folded into the u64 total accumulator as a very large delta of about 4 seconds scaled by that CPU's share of nonidle time. That large jump can lead to spurious wakeups for the PSI_POLL aggregator or incorrect values for the PSI_AVGS aggregator. Spurious triggers or nonsense psi values (e.g. full>some, psi values >100%) can cause userspace to take unnecessary corrective action, such as a userspace OOM daemon killing processes to relieve (non-existent) memory pressure. When running browser workloads on an Intel N100 with a sustained psi.mem.some of 2-3%, this underflow is observed once every couple of hours. Clamping elapsed time to >= 0 prevents this jump from happening. It does result in the total accumulator being slightly elevated compared to what it should actually be, but that error is bounded by the skew and is in practice always smaller than what is added today. Note that expanding times to u64 to prevent the u32->u64 conversion from causing issues would result in userspace-facing total counters no longer being monotonic, which could break consumers that derive rates via successive total values. Clock skew can cause issues in two more places. First, negative clock skew in record_times() can propagate through the times accumulator to the delta calculation against times_prev in get_recent_times() and cause it to underflow. Second, the elapsed time values calculated by successive executions of get_recent_times() can appear to go backwards if the first execution has positive skew and the second has negative skew. This can cause the delta calculation to underflow, since elapsed is propagated in the times_prev value. However, both of these require that the growth in stall time between iterations be non-zero but smaller than the inter-CPU skew. Neither issue has been observed in practice and they are not addressed here. Cc: stable@vger.kernel.org Fixes: eb414681d5a0 ("psi: pressure stall information for CPU, memory, and IO") Signed-off-by: David Stevens --- kernel/sched/psi.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c index 4e152410653d..893ad16ff42c 100644 --- a/kernel/sched/psi.c +++ b/kernel/sched/psi.c @@ -305,8 +305,24 @@ static void get_recent_times(struct psi_group *group, int cpu, * (u32) and our reported pressure close to what's * actually happening. */ - if (state_mask & (1 << s)) - times[s] += now - state_start; + if (state_mask & (1 << s)) { + s64 elapsed = now - state_start; + + /* + * When sched_clock_stable(), cpu_clock(cpu) ignores + * the cpu argument, so we can end up doing cross-CPU + * comparisons. A negative clock skew can result in + * underflow to a very large u32. + * + * While the u32 cyclic accumulators in groupc could + * handle a very large u32 from underflow, folding it + * into the u64 totals in collect_percpu_times() + * would result in huge jumps. Clamp to avoid that. + */ + if (elapsed < 0) + elapsed = 0; + times[s] += elapsed; + } delta = times[s] - groupc->times_prev[aggregator][s]; groupc->times_prev[aggregator][s] = times[s]; base-commit: 1fb28c664a19df8d45a6afa04d28d102b04ea680 -- 2.56.0.rc1.315.gc6ed9934b7-goog