Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   carver4lio@163.com
To:     mingo@redhat.com
Cc:     juri.lelli@redhat.com, vincent.guittot@linaro.org,
        dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com,
        mgorman@suse.de, bristot@redhat.com, linux-kernel@vger.kernel.org,
        carver4lio@163.com, Hailong Liu <liu.hailong6@zte.com.cn>
Subject: [PATCH] sched/rt:fix the missing of rt_rq runtime check in rt-period timer
Date:   Sat,  5 Dec 2020 11:38:01 +0800
Message-Id: <20201205033801.6924-1-carver4lio@163.com>
Precedence: bulk

From: Hailong Liu <liu.hailong6@zte.com.cn>

The rq->rd->span of a cpu in a system with isolated cpus splited into two
different parts: one is for isolated cpus, another for non-isolated cpus.

When CONFIG_RT_GROUP_SCHED enabled, the handler of sched_rt_period_timer
updates rt_time and rt_runtime for every cpus in rq(this_cpu)->rd->span.

It means that other parts cpus out of this_cpu's rd->span will be missed
by sched_rt_period_timer handler, when CONFIG_RT_GROUP_SCHED enabled and
isolated cpus presents in system.

E.g problem will be triggered as follows on my 8 cores machine:
1 enable  CONFIG_RT_GROUP_SCHED=y, and boot kernel with command-line
  "isolcpus=4-7"
2 create a child group and init it:
  mount -t cgroup -o cpu cpu /sys/fs/cgruop
  mkdir /sys/fs/cgroup/child0
  echo 950000 > /sys/fs/cgroup/child0/cpu.rt_runtime_us
3 run two rt-loop tasks, assume their pids are $pid1 and $pid2
4 affinity a rt task to the isolated cpu-sets
  taskset -p 0xf0 $pid2
5 add tasks created above into child cpu-group
  echo $pid1 > /sys/fs/cgroup/child0/tasks
  echo $pid2 > /sys/fs/cgroup/child0/tasks
6 check wat happened:
  "top": one of the task will fail to has cpu usage, but its stat is "R"
  "kill": the task on the problem rt_rq can't be killed

This patch will fix this problem.

Signed-off-by: Hailong Liu <liu.hailong6@zte.com.cn>
---
 kernel/sched/rt.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 49ec096a8..c5c39695c 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -855,19 +855,10 @@ static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
 	int i, idle = 1, throttled = 0;
 	const struct cpumask *span;
 
-	span = sched_rt_period_mask();
 #ifdef CONFIG_RT_GROUP_SCHED
-	/*
-	 * FIXME: isolated CPUs should really leave the root task group,
-	 * whether they are isolcpus or were isolated via cpusets, lest
-	 * the timer run on a CPU which does not service all runqueues,
-	 * potentially leaving other CPUs indefinitely throttled.  If
-	 * isolation is really required, the user will turn the throttle
-	 * off to kill the perturbations it causes anyway.  Meanwhile,
-	 * this maintains functionality for boot and/or troubleshooting.
-	 */
-	if (rt_b == &root_task_group.rt_bandwidth)
-		span = cpu_online_mask;
+	span = cpu_online_mask;
+#else
+	span = sched_rt_period_mask();
 #endif
 	for_each_cpu(i, span) {
 		int enqueue = 0;
-- 
2.17.1