From: Johannes Weiner <hannes@cmpxchg.org> stable inclusion from stable-v6.6.55 commit 1f997b1d13e0b9819468e577622e747b546516fe category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IB6YDK Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=... -------------------------------- [ Upstream commit 3840cbe24cf060ea05a585ca497814609f5d47d1 ] Brandon reports sporadic, non-sensical spikes in cumulative pressure time (total=) when reading cpu.pressure at a high rate. This is due to a race condition between reader aggregation and tasks changing states. While it affects all states and all resources captured by PSI, in practice it most likely triggers with CPU pressure, since scheduling events are so frequent compared to other resource events. The race context is the live snooping of ongoing stalls during a pressure read. The read aggregates per-cpu records for stalls that have concluded, but will also incorporate ad-hoc the duration of any active state that hasn't been recorded yet. This is important to get timely measurements of ongoing stalls. Those ad-hoc samples are calculated on-the-fly up to the current time on that CPU; since the stall hasn't concluded, it's expected that this is the minimum amount of stall time that will enter the per-cpu records once it does. The problem is that the path that concludes the state uses a CPU clock read that is not synchronized against aggregators; the clock is read outside of the seqlock protection. This allows aggregators to race and snoop a stall with a longer duration than will actually be recorded. With the recorded stall time being less than the last snapshot remembered by the aggregator, a subsequent sample will underflow and observe a bogus delta value, resulting in an erratic jump in pressure. Fix this by moving the clock read of the state change into the seqlock protection. This ensures no aggregation can snoop live stalls past the time that's recorded when the state concludes. Reported-by: Brandon Duffany <brandon@buildbuddy.io> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219194 Link: https://lore.kernel.org/lkml/20240827121851.GB438928@cmpxchg.org/ Fixes: d67515514ca5 ("psi: Reduce calls to sched_clock() in psi") Cc: stable@vger.kernel.org Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org> Conflicts: kernel/sched/psi.c [ Context conflicts and psi_cgroup_restart(re-enable) is not merged. Move the update_psi_stat_delta into the psi_group_change functions ] Signed-off-by: Chen Ridong <chenridong@huawei.com> --- kernel/sched/psi.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c index 977c973d1e70..b0068145b185 100644 --- a/kernel/sched/psi.c +++ b/kernel/sched/psi.c @@ -962,13 +962,14 @@ static void record_times(struct psi_group_cpu *groupc, u64 now) } static void psi_group_change(struct psi_group *group, int cpu, - unsigned int clear, unsigned int set, u64 now, + unsigned int clear, unsigned int set, bool wake_clock) { struct psi_group_cpu *groupc; unsigned int t, m; enum psi_states s; u32 state_mask; + u64 now; groupc = per_cpu_ptr(group->pcpu, cpu); @@ -981,7 +982,9 @@ static void psi_group_change(struct psi_group *group, int cpu, * change requested through the @clear and @set bits. */ write_seqcount_begin(&groupc->seq); + now = cpu_clock(cpu); + update_psi_stat_delta(group, cpu, now); record_times(groupc, now); record_cpu_stat_times(group, cpu); @@ -1110,7 +1113,6 @@ void psi_task_change(struct task_struct *task, int clear, int set) int cpu = task_cpu(task); struct psi_group *group; void *iter = NULL; - u64 now; int stat_set = 0; int stat_clear = 0; @@ -1120,11 +1122,8 @@ void psi_task_change(struct task_struct *task, int clear, int set) psi_flags_change(task, clear, set); psi_stat_flags_change(task, &stat_set, &stat_clear, set, clear); - now = cpu_clock(cpu); - while ((group = iterate_groups(task, &iter))) { - update_psi_stat_delta(group, cpu, now); - psi_group_change(group, cpu, clear, set, now, true); + psi_group_change(group, cpu, clear, set, true); psi_group_stat_change(group, cpu, stat_clear, stat_set); } } @@ -1135,7 +1134,6 @@ void psi_task_switch(struct task_struct *prev, struct task_struct *next, struct psi_group *group, *common = NULL; int cpu = task_cpu(prev); void *iter; - u64 now = cpu_clock(cpu); if (next->pid) { update_throttle_type(next, cpu, true); @@ -1153,8 +1151,7 @@ void psi_task_switch(struct task_struct *prev, struct task_struct *next, break; } - update_psi_stat_delta(group, cpu, now); - psi_group_change(group, cpu, 0, TSK_ONCPU, now, true); + psi_group_change(group, cpu, 0, TSK_ONCPU, true); psi_group_stat_change(group, cpu, 0, 0); } } @@ -1196,8 +1193,7 @@ void psi_task_switch(struct task_struct *prev, struct task_struct *next, iter = NULL; while ((group = iterate_groups(prev, &iter)) && group != common) { - update_psi_stat_delta(group, cpu, now); - psi_group_change(group, cpu, clear, set, now, wake_clock); + psi_group_change(group, cpu, clear, set, wake_clock); psi_group_stat_change(group, cpu, stat_clear, stat_set); } #ifdef CONFIG_PSI_FINE_GRAINED @@ -1214,8 +1210,7 @@ void psi_task_switch(struct task_struct *prev, struct task_struct *next, memstall_type_change) { clear &= ~TSK_ONCPU; for (; group; group = iterate_groups(prev, &iter)) { - update_psi_stat_delta(group, cpu, now); - psi_group_change(group, cpu, clear, set, now, wake_clock); + psi_group_change(group, cpu, clear, set, wake_clock); psi_group_stat_change(group, cpu, stat_clear, stat_set); } @@ -1238,12 +1233,11 @@ void psi_account_irqtime(struct task_struct *task, u32 delta) if (!task->pid) return; - now = cpu_clock(cpu); - while ((group = iterate_groups(task, &iter))) { groupc = per_cpu_ptr(group->pcpu, cpu); write_seqcount_begin(&groupc->seq); + now = cpu_clock(cpu); update_psi_stat_delta(group, cpu, now); record_stat_times(to_psi_group_ext(group), cpu); -- 2.34.1