sched,rt: non-isolated cores lift isolcpus throttle for CONFIG_RT_GROUP_SCHED

PATCH New Mike Galbraith 3 participants 14 messages Apr 3, 2012 – Apr 19, 2012 Scheduler

lore

Share & tools

lore

Mike Galbraith

author

2012-04-03 09:08 UTC #0 lore raw

PATCH 1/1 sched,rt: non-isolated cores lift isolcpus throttle for CONFIG_RT_GROUP_SCHED

s/patch/hack. Better ideas?

When CONFIG_RT_GROUP_SCHED is enabled, isolcpus have no replentishment timer running, and unlike !CONFIG_RT_GROUP_SCHED, are not in rd->span of the cpu running replentishment. If you trigger the throttle, you're rewarded with a dead box. Should the user reassign cpus to a domain, they become sane again, replentishment starts/stops as usual.

Signed-off-by Mike Galbraith <[email protected]>

---
 kernel/sched/core.c  |   15 ++++++++++++++-
 kernel/sched/rt.c    |   16 ++++++++++++++--
 kernel/sched/sched.h |    5 +++++
 3 files changed, 33 insertions(+), 3 deletions(-)

kernel/sched/core.c +14 -1

@@ -5877,6 +5877,14 @@ cpu_attach_domain(struct sched_domain *s
 
 	sched_domain_debug(sd, cpu);
 
+#ifdef CONFIG_RT_GROUP_SCHED
+	/* If the cpu was an isolcpu, it no longer is. */
+	if (sd) {
+		cpumask_clear_cpu(cpu, cpu_isolated_map);
+		nr_isolated_cpus = cpumask_weight(cpu_isolated_map);
+	}
+#endif
+
 	rq_attach_root(rq, rd);
 	tmp = rq->sd;
 	rcu_assign_pointer(rq->sd, sd);

@@ -5886,13 +5894,18 @@ cpu_attach_domain(struct sched_domain *s
 }
 
 /* cpus with isolated domains */
-static cpumask_var_t cpu_isolated_map;
+cpumask_var_t cpu_isolated_map;
+
+__read_mostly int nr_isolated_cpus;
 
 /* Setup the mask of cpus configured for isolated domains */
 static int __init isolated_cpu_setup(char *str)
 {
 	alloc_bootmem_cpumask_var(&cpu_isolated_map);
 	cpulist_parse(str, cpu_isolated_map);
+#ifdef CONFIG_RT_GROUP_SCHED
+	nr_isolated_cpus = cpumask_weight(cpu_isolated_map);
+#endif
 	return 1;
 }

kernel/sched/rt.c +14 -2

@@ -778,10 +778,11 @@ static inline int balance_runtime(struct
 
 static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
 {
-	int i, idle = 1, throttled = 0;
+	int i, idle = 1, throttled = 0, isol_cpus = nr_isolated_cpus;
 	const struct cpumask *span;
 
 	span = sched_rt_period_mask();
+do_isolcpus:
 	for_each_cpu(i, span) {
 		int enqueue = 0;
 		struct rt_rq *rt_rq = sched_rt_period_rt_rq(rt_b, i);

@@ -792,7 +793,7 @@ static int do_sched_rt_period_timer(stru
 			u64 runtime;
 
 			raw_spin_lock(&rt_rq->rt_runtime_lock);
-			if (rt_rq->rt_throttled)
+			if (rt_rq->rt_throttled && span != cpu_isolated_map)
 				balance_runtime(rt_rq);
 			runtime = rt_rq->rt_runtime;
 			rt_rq->rt_time -= min(rt_rq->rt_time, overrun*runtime);

@@ -823,6 +824,17 @@ static int do_sched_rt_period_timer(stru
 		raw_spin_unlock(&rq->lock);
 	}
 
+	/*
+	 * Hack: unthrottle isolcpus for RT_GROUP_SCHED.  No replentishment
+	 * timer is running on isolcpus, and unlike !RT_GROUP_SCHED, they're
+	 * not in the rd->span of the cpu running the timer.
+	 */
+	if (isol_cpus) {
+		span = cpu_isolated_map;
+		isol_cpus = 0;
+		goto do_isolcpus;
+	}
+
 	if (!throttled && (!rt_bandwidth_enabled() || rt_b->rt_runtime == RUNTIME_INF))
 		return 1;

kernel/sched/sched.h +5 -0

@@ -572,6 +572,11 @@ static inline void set_task_rq(struct ta
 #endif
 }
 
+/* cpus with isolated domains */
+extern  cpumask_var_t cpu_isolated_map;
+
+extern __read_mostly int nr_isolated_cpus;
+
 #else /* CONFIG_CGROUP_SCHED */
 
 static inline void set_task_rq(struct task_struct *p, unsigned int cpu) { }

Mike Galbraith

author

2012-04-03 09:20 UTC #1 lore raw

PATCH 1/1 sched,rt: let the user see rt queues in /proc/sched_debug

ec514c48 made rt queues invisible in sched_debug, restore visibility.

Signed-off-by Mike Galbraith <[email protected]>

---
 kernel/sched/rt.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

kernel/sched/rt.c +1 -2

@@ -2079,11 +2079,10 @@ extern void print_rt_rq(struct seq_file
 
 void print_rt_stats(struct seq_file *m, int cpu)
 {
-	rt_rq_iter_t iter;
 	struct rt_rq *rt_rq;
 
 	rcu_read_lock();
-	for_each_rt_rq(rt_rq, iter, cpu_rq(cpu))
+	for_each_leaf_rt_rq(rt_rq, cpu_rq(cpu))
 		print_rt_rq(m, cpu, rt_rq);
 	rcu_read_unlock();
 }

Mike Galbraith

author

2012-04-07 08:58 UTC #2 lore raw

PATCH 1/1 sched,cgroup_sched: fix up task_groups list buglet

Quoted 1 line · Mike Galbraith

ec514c48 made rt queues invisible in sched_debug, restore visibility.

<g> removing their cloak of invisibility might be a tad better than just making them visible while futzing with annoying RT_GROUP_SCHED throttle.

sched,cgroup_sched: fix up task_groups list

Oops happened during sched migration to kernel/sched, fix it up.

Signed-off-by Mike Galbraith <[email protected]>

Cc [email protected] # v3.3

---
 kernel/sched/core.c  |    1 +
 kernel/sched/sched.h |    2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

kernel/sched/core.c +1 -0

@@ -6871,6 +6871,7 @@ int in_sched_functions(unsigned long add
 
 #ifdef CONFIG_CGROUP_SCHED
 struct task_group root_task_group;
+LIST_HEAD(task_groups);
 #endif
 
 DECLARE_PER_CPU(cpumask_var_t, load_balance_tmpmask);

kernel/sched/sched.h +1 -1

@@ -80,7 +80,7 @@ extern struct mutex sched_domains_mutex;
 struct cfs_rq;
 struct rt_rq;
 
-static LIST_HEAD(task_groups);
+extern struct list_head task_groups;
 
 struct cfs_bandwidth {
 #ifdef CONFIG_CFS_BANDWIDTH

Mike Galbraith

author

2012-04-07 09:54 UTC #3 lore raw

RFC [patch] sched,cgroup_sched: convince RT_GROUP_SCHED throttle to work

Greetings,

I'm having trouble with RT_GROUP_SCHED throttle kicking in and staying engaged (timer troubles). Either groups execute one after the other (frob timer above you), or bandwidth is wrong, or the reason I started squabbling with this thing in the first place happens, one or the other group gets stuck, even with only two groups, with the root task group throttled, and the victim is marooned until I kill the cgroup setup. If (say) grp1 starts first, grp2 is screwed, or the other way around.

With this patch, the thing appears to work perfectly, but it doesn't look correct, since I'm futzing with ->rt_time where I should not.

Not so pretty ascii-art:

/----------/system cpu 0-2, rt 300000-----/foo cpu 2, rt 100000

    \
     \
      \----/rtcpus cpu 3, rt 300000---\---/bar cpu 3, rt 100000
                                       \
                                        \-/baz cpu 3, rt 100000

It only needs to be two groups, grp1 containing most of the system, the other rt only. With the patch, the above setup works (last setup I prodded box with), and bandwidth looked fine, twiddle budgets or not.

I just happened to notice the throttle wasn't doing it's thing right after discovering that isolcpus is busted with RT_GROUP_SCHED. Thought I should probably beat on it a little. The darn thing beat me back :)

---
 kernel/sched/rt.c |   76 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 45 insertions(+), 31 deletions(-)

kernel/sched/rt.c +45 ~~-31~~

@@ -776,55 +776,69 @@ static inline int balance_runtime(struct
 }
 #endif /* CONFIG_SMP */
 
+#ifdef CONFIG_RT_GROUP_SCHED
+#define for_each_rt_rq_up_from(rt_rq, iter, rq)			\
+	for (iter = rt_rq->tg; iter; iter = iter->parent,	\
+		rt_rq = iter ? iter->rt_rq[cpu_of(rq)] : NULL)
+#else
+#define for_each_rt_rq_up_from(rt_rq, iter, rq)			\
+	for (iter = rt_rq; iter; iter = NULL)
+#endif
+
 static int do_sched_rt_period_timer(struct rt_bandwidth *rt_b, int overrun)
 {
 	int i, idle = 1, throttled = 0;
 	const struct cpumask *span;
+	rt_rq_iter_t iter;
 
 	span = sched_rt_period_mask();
 	for_each_cpu(i, span) {
-		int enqueue = 0;
+		int enqueue = 0, depth = 0;
 		struct rt_rq *rt_rq = sched_rt_period_rt_rq(rt_b, i);
-		struct rq *rq = rq_of_rt_rq(rt_rq);
+		struct rq *rq = cpu_rq(i);
 
 		raw_spin_lock(&rq->lock);
-		if (rt_rq->rt_time) {
-			u64 runtime;
-
-			raw_spin_lock(&rt_rq->rt_runtime_lock);
-			if (rt_rq->rt_throttled)
-				balance_runtime(rt_rq);
-			runtime = rt_rq->rt_runtime;
-			rt_rq->rt_time -= min(rt_rq->rt_time, overrun*runtime);
-			if (rt_rq->rt_throttled && rt_rq->rt_time < runtime) {
-				rt_rq->rt_throttled = 0;
+		for_each_rt_rq_up_from(rt_rq, iter, rq) {
+			if (rt_rq->rt_time) {
+				u64 runtime;
+
+				raw_spin_lock(&rt_rq->rt_runtime_lock);
+				if (rt_rq->rt_throttled)
+					balance_runtime(rt_rq);
+				runtime = rt_rq->rt_runtime;
+				rt_rq->rt_time -= min(rt_rq->rt_time, overrun*runtime);
+				if (rt_rq->rt_throttled && rt_rq->rt_time < runtime) {
+					rt_rq->rt_throttled = 0;
+					enqueue = 1;
+
+					/*
+					 * Force a clock update if the CPU was idle,
+					 * lest wakeup -> unthrottle time accumulate.
+					 */
+					if (rt_rq->rt_nr_running && rq->curr == rq->idle)
+						rq->skip_clock_update = -1;
+				}
+				raw_spin_unlock(&rt_rq->rt_runtime_lock);
+			} else if (!rt_rq_throttled(rt_rq))
 				enqueue = 1;
 
-				/*
-				 * Force a clock update if the CPU was idle,
-				 * lest wakeup -> unthrottle time accumulate.
-				 */
-				if (rt_rq->rt_nr_running && rq->curr == rq->idle)
-					rq->skip_clock_update = -1;
+			if (enqueue)
+				sched_rt_rq_enqueue(rt_rq);
+
+			if (!depth++) {
+				if (rt_rq->rt_throttled) {
+					throttled = 1;
+					idle = 0;
+				} else if (rt_rq->rt_time || rt_rq->rt_nr_running)
+					idle = 0;
 			}
-			if (rt_rq->rt_time || rt_rq->rt_nr_running)
-				idle = 0;
-			raw_spin_unlock(&rt_rq->rt_runtime_lock);
-		} else if (rt_rq->rt_nr_running) {
-			idle = 0;
-			if (!rt_rq_throttled(rt_rq))
-				enqueue = 1;
-		}
-		if (rt_rq->rt_throttled)
-			throttled = 1;
 
-		if (enqueue)
-			sched_rt_rq_enqueue(rt_rq);
+		}
 		raw_spin_unlock(&rq->lock);
 	}
 
 	if (!throttled && (!rt_bandwidth_enabled() || rt_b->rt_runtime == RUNTIME_INF))
-		return 1;
+		idle = 1;
 
 	return idle;
 }

Mike Galbraith

author

2012-04-10 09:08 UTC #4 lore raw

Re: RFC [patch] sched,cgroup_sched: convince RT_GROUP_SCHED throttle to work

Quoted 2 lines · Mike Galbraith

With this patch, the thing appears to work perfectly, but it doesn't look correct, since I'm futzing with ->rt_time where I should not.

s/doesn't look/definitely isn't. This failed the "definitely works so may live on despite horrific appearance" test anyway, so how about just stop pretending root isn't global instead?

sched,rt: fix isolated CPUs leaving root_task_group indefinitely throttled

Root task group bandwidth replenishment must service all CPUs regardless of where it was last started.

Signed-off-by Mike Galbraith <[email protected]>

---
 kernel/sched/rt.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

kernel/sched/rt.c +13 -0

@@ -782,6 +782,19 @@ static int do_sched_rt_period_timer(stru
 	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;
+#endif
 	for_each_cpu(i, span) {
 		int enqueue = 0;
 		struct rt_rq *rt_rq = sched_rt_period_rt_rq(rt_b, i);

Peter Zijlstra

2012-04-14 11:10 UTC #5 lore raw

Re: RFC [patch] sched,cgroup_sched: convince RT_GROUP_SCHED throttle to work

Quoted 13 lines · Mike Galbraith

sched,rt: fix isolated CPUs leaving root_task_group indefinitely throttled
Root task group bandwidth replenishment must service all CPUs regardless of where it was last started.
Signed-off-by Mike Galbraith <[email protected]>
---
 kernel/sched/rt.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

kernel/sched/rt.c static int do_sched_rt_period_timer(stru · line 787

Quoted 6 lines

kernel/sched/rt.c +3 -0

@@ -782,6 +782,19 @@ static int do_sched_rt_period_timer(stru
 	const struct cpumask *span;
 
 	span = sched_rt_period_mask();
+#ifdef CONFIG_RT_GROUP_SCHED
+	/*
+	 * FIXME: isolated CPUs should really leave the root task group,

No no, that's the wrong fix, the right fix is to remove isolcpus :-)

kernel/sched/rt.c static int do_sched_rt_period_timer(stru · line 800

Quoted 13 lines

kernel/sched/rt.c +10 -0

+	 * 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;
+#endif
 	for_each_cpu(i, span) {
 		int enqueue = 0;
 		struct rt_rq *rt_rq = sched_rt_period_rt_rq(rt_b, i);

I guess the alternative 'fix' is to not account the rt_runtime on isolated cpus.. does something like the below actually work?

---
 kernel/sched/rt.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

kernel/sched/rt.c +3 -0

index 44af55e..dc2b5b6 100644
@@ -922,6 +922,9 @@ static void update_curr_rt(struct rq *rq)
 	if (!rt_bandwidth_enabled())
 		return;
 
+	if (cpumask_test_cpu(cpu_of(rq), cpu_isolated_map))
+		return;
+
 	for_each_sched_rt_entity(rt_se) {
 		rt_rq = rt_rq_of_se(rt_se);

Mike Galbraith

author

2012-04-15 03:37 UTC #6 lore raw

Re: RFC [patch] sched,cgroup_sched: convince RT_GROUP_SCHED throttle to work

Quoted 7 lines · Peter Zijlstra

Quoted 5 lines · Mike Galbraith
kernel/sched/rt.c +0 -0
@@ -782,6 +782,19 @@ static int do_sched_rt_period_timer(stru
 	const struct cpumask *span;
 

Quoted 6 lines

kernel/sched/rt.c static int do_sched_rt_period_timer(stru · line 787
Quoted 4 lines
kernel/sched/rt.c +3 -0
 	span = sched_rt_period_mask();
+#ifdef CONFIG_RT_GROUP_SCHED
+	/*
+	 * FIXME: isolated CPUs should really leave the root task group,
No no, that's the wrong fix, the right fix is to remove isolcpus :-)

Yeah, isolcpus needs to die, but...

Quoted 2 lines

I guess the alternative 'fix' is to not account the rt_runtime on isolated cpus.. does something like the below actually work?

I haven't tried it, because the exact same thing happens when you isolate via cpusets directly below root. One timer, two (or more) rd->span, so _somebody_ is screwed.

-Mike

Mike Galbraith

author

2012-04-15 03:44 UTC #7 lore raw

Re: RFC [patch] sched,cgroup_sched: convince RT_GROUP_SCHED throttle to work

Quoted 15 lines · Mike Galbraith

Quoted 7 lines · Peter Zijlstra
Quoted 5 lines · Mike Galbraith
kernel/sched/rt.c +0 -0
@@ -782,6 +782,19 @@ static int do_sched_rt_period_timer(stru
 	const struct cpumask *span;
 
Quoted 6 lines
kernel/sched/rt.c static int do_sched_rt_period_timer(stru · line 787
Quoted 4 lines
kernel/sched/rt.c +3 -0
 	span = sched_rt_period_mask();
+#ifdef CONFIG_RT_GROUP_SCHED
+	/*
+	 * FIXME: isolated CPUs should really leave the root task group,
No no, that's the wrong fix, the right fix is to remove isolcpus :-)

Quoted 8 lines

Yeah, isolcpus needs to die, but...
Quoted 2 lines
I guess the alternative 'fix' is to not account the rt_runtime on isolated cpus.. does something like the below actually work?
I haven't tried it, because the exact same thing happens when you isolate via cpusets directly below root. One timer, two (or more) rd->span, so _somebody_ is screwed.

You _could_ bail on !rq->sd I suppose, but the way I hacked around it, the user can keep the throttle for testing/troubleshooting their isolated setup, and turn it off in production. OTOH, auto throttle disable for all isolated sets could work just as well.

-Mike

Mike Galbraith

author

2012-04-15 04:51 UTC #8 lore raw

Re: RFC [patch] sched,cgroup_sched: convince RT_GROUP_SCHED throttle to work

Quoted 21 lines · Mike Galbraith

Quoted 20 lines · Mike Galbraith
Quoted 7 lines · Peter Zijlstra
Quoted 5 lines · Mike Galbraith
kernel/sched/rt.c +0 -0
@@ -782,6 +782,19 @@ static int do_sched_rt_period_timer(stru
 	const struct cpumask *span;
 
Quoted 6 lines
kernel/sched/rt.c static int do_sched_rt_period_timer(stru · line 787
Quoted 4 lines
kernel/sched/rt.c +3 -0
 	span = sched_rt_period_mask();
+#ifdef CONFIG_RT_GROUP_SCHED
+	/*
+	 * FIXME: isolated CPUs should really leave the root task group,
No no, that's the wrong fix, the right fix is to remove isolcpus :-)
Yeah, isolcpus needs to die, but...
Quoted 2 lines
I guess the alternative 'fix' is to not account the rt_runtime on isolated cpus.. does something like the below actually work?

Quoted 8 lines

Quoted 3 lines
I haven't tried it, because the exact same thing happens when you isolate via cpusets directly below root. One timer, two (or more) rd->span, so _somebody_ is screwed.
You _could_ bail on !rq->sd I suppose, but the way I hacked around it, the user can keep the throttle for testing/troubleshooting their isolated setup, and turn it off in production. OTOH, auto throttle disable for all isolated sets could work just as well.

Like so seems to work. I personally like 2 lines better, but whatever solves dinky but deadly problem works for me.

---
 kernel/sched/core.c  |    7 ++++++-
 kernel/sched/rt.c    |    9 +++++++++
 kernel/sched/sched.h |    3 +++
 3 files changed, 18 insertions(+), 1 deletion(-)

kernel/sched/core.c +6 -1

@@ -5875,6 +5875,11 @@ cpu_attach_domain(struct sched_domain *s
 			sd->child = NULL;
 	}
 
+	if (sd)
+		cpumask_clear_cpu(cpu, cpu_isolated_map);
+	else
+		cpumask_set_cpu(cpu, cpu_isolated_map);
+
 	sched_domain_debug(sd, cpu);
 
 	rq_attach_root(rq, rd);

@@ -5886,7 +5891,7 @@ cpu_attach_domain(struct sched_domain *s
 }
 
 /* cpus with isolated domains */
-static cpumask_var_t cpu_isolated_map;
+cpumask_var_t cpu_isolated_map;
 
 /* Setup the mask of cpus configured for isolated domains */
 static int __init isolated_cpu_setup(char *str)

kernel/sched/rt.c +9 -0

@@ -922,6 +922,9 @@ static void update_curr_rt(struct rq *rq
 	if (!rt_bandwidth_enabled())
 		return;
 
+	if (cpumask_test_cpu(cpu_of(rq), cpu_isolated_map))
+		return;
+
 	for_each_sched_rt_entity(rt_se) {
 		rt_rq = rt_rq_of_se(rt_se);

@@ -1014,6 +1017,9 @@ static inline void dec_rt_prio(struct rt
 static void
 inc_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq)
 {
+	if (cpumask_test_cpu(rq_of_rt_rq(rt_rq)->cpu, cpu_isolated_map))
+		return;
+
 	if (rt_se_boosted(rt_se))
 		rt_rq->rt_nr_boosted++;

@@ -1035,6 +1041,9 @@ dec_rt_group(struct sched_rt_entity *rt_
 static void
 inc_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq)
 {
+	if (cpumask_test_cpu(rq_of_rt_rq(rt_rq)->cpu, cpu_isolated_map))
+		return;
+
 	start_rt_bandwidth(&def_rt_bandwidth);
 }

kernel/sched/sched.h +3 -0

@@ -73,6 +73,9 @@ struct rt_bandwidth {
 
 extern struct mutex sched_domains_mutex;
 
+/* cpus with isolated domains */
+extern cpumask_var_t cpu_isolated_map;
+
 #ifdef CONFIG_CGROUP_SCHED
 
 #include <linux/cgroup.h>

Yong Zhang

2012-04-18 05:30 UTC #9 lore raw

Re: RFC [patch] sched,cgroup_sched: convince RT_GROUP_SCHED throttle to work

Quoted 12 lines · Mike Galbraith

Like so seems to work. I personally like 2 lines better, but whatever solves dinky but deadly problem works for me.
---
 kernel/sched/core.c  |    7 ++++++-
 kernel/sched/rt.c    |    9 +++++++++
 kernel/sched/sched.h |    3 +++
 3 files changed, 18 insertions(+), 1 deletion(-)

kernel/sched/core.c cpu_attach_domain(struct sched_domain *s · line 5882

Quoted 8 lines

kernel/sched/core.c +5 -0

@@ -5875,6 +5875,11 @@ cpu_attach_domain(struct sched_domain *s
 			sd->child = NULL;
 	}
 
+	if (sd)
+		cpumask_clear_cpu(cpu, cpu_isolated_map);
+	else
+		cpumask_set_cpu(cpu, cpu_isolated_map);
+

Do we allow this?
Seems sched_domain_debug() will bite !sd.

Thanks,
Yong

Quoted 63 lines

kernel/sched/core.c +1 -1

 	sched_domain_debug(sd, cpu);
 
 	rq_attach_root(rq, rd);

@@ -5886,7 +5891,7 @@ cpu_attach_domain(struct sched_domain *s
 }
 
 /* cpus with isolated domains */
-static cpumask_var_t cpu_isolated_map;
+cpumask_var_t cpu_isolated_map;
 
 /* Setup the mask of cpus configured for isolated domains */
 static int __init isolated_cpu_setup(char *str)

kernel/sched/rt.c +9 -0

@@ -922,6 +922,9 @@ static void update_curr_rt(struct rq *rq
 	if (!rt_bandwidth_enabled())
 		return;
 
+	if (cpumask_test_cpu(cpu_of(rq), cpu_isolated_map))
+		return;
+
 	for_each_sched_rt_entity(rt_se) {
 		rt_rq = rt_rq_of_se(rt_se);

@@ -1014,6 +1017,9 @@ static inline void dec_rt_prio(struct rt
 static void
 inc_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq)
 {
+	if (cpumask_test_cpu(rq_of_rt_rq(rt_rq)->cpu, cpu_isolated_map))
+		return;
+
 	if (rt_se_boosted(rt_se))
 		rt_rq->rt_nr_boosted++;

@@ -1035,6 +1041,9 @@ dec_rt_group(struct sched_rt_entity *rt_
 static void
 inc_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq)
 {
+	if (cpumask_test_cpu(rq_of_rt_rq(rt_rq)->cpu, cpu_isolated_map))
+		return;
+
 	start_rt_bandwidth(&def_rt_bandwidth);
 }

kernel/sched/sched.h +3 -1

@@ -73,6 +73,9 @@ struct rt_bandwidth {
 
 extern struct mutex sched_domains_mutex;
 
+/* cpus with isolated domains */
+extern cpumask_var_t cpu_isolated_map;
+
 #ifdef CONFIG_CGROUP_SCHED
 
 #include <linux/cgroup.h>
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 1 line

Only stand for myself

Mike Galbraith

author

2012-04-18 06:27 UTC #10 lore raw

Re: RFC [patch] sched,cgroup_sched: convince RT_GROUP_SCHED throttle to work

Quoted 6 lines · Yong Zhang

Quoted 6 lines

kernel/sched/core.c +0 -0

@@ -5875,6 +5875,11 @@ cpu_attach_domain(struct sched_domain *s
 			sd->child = NULL;
 	}

Quoted 7 lines

kernel/sched/core.c cpu_attach_domain(struct sched_domain *s · line 5882
Quoted 5 lines
kernel/sched/core.c +5 -0
+	if (sd)
+		cpumask_clear_cpu(cpu, cpu_isolated_map);
+	else
+		cpumask_set_cpu(cpu, cpu_isolated_map);
+
Do we allow this?

Yeah, isolating CPUS 2-3...

[ 3011.444345] CPU0 attaching NULL sched-domain.
[ 3011.448719] CPU1 attaching NULL sched-domain.
[ 3011.453107] CPU2 attaching NULL sched-domain.
[ 3011.457466] CPU3 attaching NULL sched-domain.
[ 3011.461892] CPU0 attaching sched-domain:
[ 3011.465813]  domain 0: span 0-1 level MC
[ 3011.469761]   groups: 0 1
[ 3011.472415] CPU1 attaching sched-domain:
[ 3011.476333]  domain 0: span 0-1 level MC
[ 3011.480266]   groups: 1 0
[ 3011.482988] CPU2 attaching sched-domain:
[ 3011.486919]  domain 0: span 2-3 level MC
[ 3011.490851]   groups: 2 3
[ 3011.493502] CPU3 attaching sched-domain:
[ 3011.497422]  domain 0: span 2-3 level MC
[ 3011.501367]   groups: 3 2
[ 3011.504214] CPU2 attaching NULL sched-domain.
[ 3011.508569] CPU3 attaching NULL sched-domain.

Yong Zhang

2012-04-18 07:49 UTC #11 lore raw

Re: RFC [patch] sched,cgroup_sched: convince RT_GROUP_SCHED throttle to work

Quoted 16 lines · Mike Galbraith

Quoted 6 lines · Yong Zhang
Quoted 6 lines
kernel/sched/core.c +0 -0
@@ -5875,6 +5875,11 @@ cpu_attach_domain(struct sched_domain *s
 			sd->child = NULL;
 	}
 
Quoted 7 lines
kernel/sched/core.c cpu_attach_domain(struct sched_domain *s · line 5882
Quoted 5 lines
kernel/sched/core.c +5 -0
+	if (sd)
+		cpumask_clear_cpu(cpu, cpu_isolated_map);
+	else
+		cpumask_set_cpu(cpu, cpu_isolated_map);
+
Do we allow this?

Quoted 1 line

Yeah, isolating CPUS 2-3...

Hmm...magic cpuset ;-)

Quoted 11 lines

[ 3011.444345] CPU0 attaching NULL sched-domain.
[ 3011.448719] CPU1 attaching NULL sched-domain.
[ 3011.453107] CPU2 attaching NULL sched-domain.
[ 3011.457466] CPU3 attaching NULL sched-domain.
[ 3011.461892] CPU0 attaching sched-domain:
[ 3011.465813]  domain 0: span 0-1 level MC
[ 3011.469761]   groups: 0 1
[ 3011.472415] CPU1 attaching sched-domain:
[ 3011.476333]  domain 0: span 0-1 level MC
[ 3011.480266]   groups: 1 0

Quoted 8 lines

[ 3011.482988] CPU2 attaching sched-domain:
[ 3011.486919]  domain 0: span 2-3 level MC
[ 3011.490851]   groups: 2 3
[ 3011.493502] CPU3 attaching sched-domain:
[ 3011.497422]  domain 0: span 2-3 level MC
[ 3011.501367]   groups: 3 2
[ 3011.504214] CPU2 attaching NULL sched-domain.
[ 3011.508569] CPU3 attaching NULL sched-domain.

But another scenario comes into head:
What will happen when a rt_rq is throttled the very CPU is attached to NULL domain?

Thanks,
Yong

Quoted 7 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 1 line

Only stand for myself

Mike Galbraith

author

2012-04-18 08:38 UTC #12 lore raw

Re: RFC [patch] sched,cgroup_sched: convince RT_GROUP_SCHED throttle to work

Quoted 17 lines · Yong Zhang

Quoted 16 lines · Mike Galbraith
Quoted 13 lines · Yong Zhang
kernel/sched/core.c cpu_attach_domain(struct sched_domain *s · line 5882
Quoted 11 lines
kernel/sched/core.c +5 -0
@@ -5875,6 +5875,11 @@ cpu_attach_domain(struct sched_domain *s
 			sd->child = NULL;
 	}
 
+	if (sd)
+		cpumask_clear_cpu(cpu, cpu_isolated_map);
+	else
+		cpumask_set_cpu(cpu, cpu_isolated_map);
+
Do we allow this?

Quoted 3 lines

Quoted 1 line
Yeah, isolating CPUS 2-3...
Hmm...magic cpuset ;-)

The _only_ cpuset if you have a low jitter requirement.

Quoted 19 lines

Quoted 10 lines

[ 3011.444345] CPU0 attaching NULL sched-domain.
[ 3011.448719] CPU1 attaching NULL sched-domain.
[ 3011.453107] CPU2 attaching NULL sched-domain.
[ 3011.457466] CPU3 attaching NULL sched-domain.
[ 3011.461892] CPU0 attaching sched-domain:
[ 3011.465813]  domain 0: span 0-1 level MC
[ 3011.469761]   groups: 0 1
[ 3011.472415] CPU1 attaching sched-domain:
[ 3011.476333]  domain 0: span 0-1 level MC
[ 3011.480266]   groups: 1 0

Quoted 8 lines

[ 3011.482988] CPU2 attaching sched-domain:
[ 3011.486919]  domain 0: span 2-3 level MC
[ 3011.490851]   groups: 2 3
[ 3011.493502] CPU3 attaching sched-domain:
[ 3011.497422]  domain 0: span 2-3 level MC
[ 3011.501367]   groups: 3 2
[ 3011.504214] CPU2 attaching NULL sched-domain.
[ 3011.508569] CPU3 attaching NULL sched-domain.

Quoted 3 lines

But another scenario comes into head:
What will happen when a rt_rq is throttled the very CPU is attached to NULL domain?

Yup, _somebody_ will hit the throttle only once :)

That's what gets fixed by either killing the throttle entirely for isolated CPUs, or making the throttle work until the guy who needed isolation turns noise maker off. I prefer reconnect the dots, because that doesn't touch the fast path.

-Mike

Yong Zhang

2012-04-19 06:34 UTC #13 lore raw

Re: RFC [patch] sched,cgroup_sched: convince RT_GROUP_SCHED throttle to work

Quoted 47 lines · Mike Galbraith

Quoted 17 lines · Yong Zhang
Quoted 16 lines · Mike Galbraith
Quoted 13 lines · Yong Zhang
kernel/sched/core.c cpu_attach_domain(struct sched_domain *s · line 5882
Quoted 11 lines
kernel/sched/core.c +5 -0
@@ -5875,6 +5875,11 @@ cpu_attach_domain(struct sched_domain *s
 			sd->child = NULL;
 	}
 
+	if (sd)
+		cpumask_clear_cpu(cpu, cpu_isolated_map);
+	else
+		cpumask_set_cpu(cpu, cpu_isolated_map);
+
Do we allow this?
Quoted 3 lines
Quoted 1 line
Yeah, isolating CPUS 2-3...
Hmm...magic cpuset ;-)
The _only_ cpuset if you have a low jitter requirement.
Quoted 19 lines
Quoted 10 lines
[ 3011.444345] CPU0 attaching NULL sched-domain.
[ 3011.448719] CPU1 attaching NULL sched-domain.
[ 3011.453107] CPU2 attaching NULL sched-domain.
[ 3011.457466] CPU3 attaching NULL sched-domain.
[ 3011.461892] CPU0 attaching sched-domain:
[ 3011.465813]  domain 0: span 0-1 level MC
[ 3011.469761]   groups: 0 1
[ 3011.472415] CPU1 attaching sched-domain:
[ 3011.476333]  domain 0: span 0-1 level MC
[ 3011.480266]   groups: 1 0
Quoted 8 lines
[ 3011.482988] CPU2 attaching sched-domain:
[ 3011.486919]  domain 0: span 2-3 level MC
[ 3011.490851]   groups: 2 3
[ 3011.493502] CPU3 attaching sched-domain:
[ 3011.497422]  domain 0: span 2-3 level MC
[ 3011.501367]   groups: 3 2
[ 3011.504214] CPU2 attaching NULL sched-domain.
[ 3011.508569] CPU3 attaching NULL sched-domain.
Quoted 3 lines
But another scenario comes into head:
What will happen when a rt_rq is throttled the very CPU is attached to NULL domain?

Quoted 6 lines

Yup, _somebody_ will hit the throttle only once :)
That's what gets fixed by either killing the throttle entirely for isolated CPUs, or making the throttle work until the guy who needed isolation turns noise maker off. I prefer reconnect the dots, because that doesn't touch the fast path.

I like it better too :)

Thanks,
Yong