Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chengming Zhou <zhouchengming@bytedance.com>
To:     corbet@lwn.net, hannes@cmpxchg.org, mingo@redhat.com,
        peterz@infradead.org, surenb@google.com, ebiggers@google.com
Cc:     linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        songmuchun@bytedance.com,
        Chengming Zhou <zhouchengming@bytedance.com>,
        Martin Steigerwald <Martin.Steigerwald@proact.de>
Subject: [PATCH] psi: remove CPU full metric at system level
Date:   Thu,  3 Mar 2022 13:58:14 +0800
Message-Id: <20220303055814.93057-1-zhouchengming@bytedance.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Martin find it confusing when look at the /proc/pressure/cpu output,
and found no hint about that CPU "full" line in psi Documentation.

% cat /proc/pressure/cpu
some avg10=0.92 avg60=0.91 avg300=0.73 total=933490489
full avg10=0.22 avg60=0.23 avg300=0.16 total=358783277

The PSI_CPU_FULL state is introduced by commit e7fcd7622823
("psi: Add PSI_CPU_FULL state"), which mainly for cgroup level,
but also counted at the system level as a side effect.

Naturally, the FULL state doesn't exist for the CPU resource at
the system level. These "full" numbers can come from CPU idle
schedule latency. For example, t1 is the time when task wakeup
on an idle CPU, t2 is the time when CPU pick and switch to it.
The delta of (t2 - t1) will be in CPU_FULL state.

Another case all processes can be stalled is when all cgroups
have been throttled at the same time, which unlikely to happen.

Anyway, CPU_FULL metric is meaningless and confusing at the
system level. So this patch removed CPU full metric at the
system level, and removed it's monitor function too. The psi
Documentation has also been updated accordingly.

Fixes: e7fcd7622823 ("psi: Add PSI_CPU_FULL state")
Reported-by: Martin Steigerwald <Martin.Steigerwald@proact.de>
Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
---
 Documentation/accounting/psi.rst | 18 +++++++++++++++---
 kernel/sched/psi.c               | 10 +++++++++-
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/Documentation/accounting/psi.rst b/Documentation/accounting/psi.rst
index 860fe651d645..519652c06d7d 100644
--- a/Documentation/accounting/psi.rst
+++ b/Documentation/accounting/psi.rst
@@ -178,8 +178,20 @@ Cgroup2 interface
 In a system with a CONFIG_CGROUP=y kernel and the cgroup2 filesystem
 mounted, pressure stall information is also tracked for tasks grouped
 into cgroups. Each subdirectory in the cgroupfs mountpoint contains
-cpu.pressure, memory.pressure, and io.pressure files; the format is
-the same as the /proc/pressure/ files.
+cpu.pressure, memory.pressure, and io.pressure files; the format of
+memory.pressure and io.pressure is the same as the /proc/pressure/ files.
+
+But the format of cpu.pressure is as such::
+	some avg10=0.00 avg60=0.00 avg300=0.00 total=0
+	full avg10=0.00 avg60=0.00 avg300=0.00 total=0
+
+The "some" line indicates the share of time in which at least some tasks
+in the cgroup are stalled on CPU resource.
+
+The "full" line indicates the share of time in which all non-idle tasks
+in the cgroup are stalled on CPU resource, which is being used by others
+outside of the cgroup or throttled by the cgroup cpu.max configuration.
 
 Per-cgroup psi monitors can be specified and used the same way as
-system-wide ones.
+system-wide ones, except that users can also monitor full pressure on
+CPU resource at the cgroup level.
diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index e14358178849..d1baeb07d08c 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -1047,6 +1047,7 @@ void cgroup_move_task(struct task_struct *task, struct css_set *to)
 
 int psi_show(struct seq_file *m, struct psi_group *group, enum psi_res res)
 {
+	int full_max = 2;
 	int full;
 	u64 now;
 
@@ -1061,7 +1062,11 @@ int psi_show(struct seq_file *m, struct psi_group *group, enum psi_res res)
 		group->avg_next_update = update_averages(group, now);
 	mutex_unlock(&group->avgs_lock);
 
-	for (full = 0; full < 2; full++) {
+	/* CPU_FULL state doesn't exist at system level */
+	if (res == PSI_CPU && group == &psi_system)
+		full_max = 1;
+
+	for (full = 0; full < full_max; full++) {
 		unsigned long avg[3];
 		u64 total;
 		int w;
@@ -1103,6 +1108,9 @@ struct psi_trigger *psi_trigger_create(struct psi_group *group,
 	if (state >= PSI_NONIDLE)
 		return ERR_PTR(-EINVAL);
 
+	if (state == PSI_CPU_FULL && group == &psi_system)
+		return ERR_PTR(-EINVAL);
+
 	if (window_us < WINDOW_MIN_US ||
 		window_us > WINDOW_MAX_US)
 		return ERR_PTR(-EINVAL);
-- 
2.20.1