* [PATCH 1/5] sched, autogroup, sysctl: use proc_dointvec_minmax instead.
2011-02-20 7:08 [PATCH 0/5] autogroup runtime enable/disable tuning fix Yong Zhang
@ 2011-02-20 7:08 ` Yong Zhang
2011-02-23 12:05 ` [tip:sched/core] sched, autogroup, sysctl: Use proc_dointvec_minmax() instead tip-bot for Yong Zhang
2011-02-20 7:08 ` [PATCH 2/5] sched: hide cfs_rq of autogroup if autogroup is disabled Yong Zhang
` (3 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Yong Zhang @ 2011-02-20 7:08 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Peter Zijlstra, Mike Galbraith
sched_autogroup_enabled has min/max value, proc_dointvec_minmax()
is be used for this case.
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
---
kernel/sysctl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 0f1bd83..22e07ee 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -374,7 +374,7 @@ static struct ctl_table kern_table[] = {
.data = &sysctl_sched_autogroup_enabled,
.maxlen = sizeof(unsigned int),
.mode = 0644,
- .proc_handler = proc_dointvec,
+ .proc_handler = proc_dointvec_minmax,
.extra1 = &zero,
.extra2 = &one,
},
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [tip:sched/core] sched, autogroup, sysctl: Use proc_dointvec_minmax() instead
2011-02-20 7:08 ` [PATCH 1/5] sched, autogroup, sysctl: use proc_dointvec_minmax instead Yong Zhang
@ 2011-02-23 12:05 ` tip-bot for Yong Zhang
0 siblings, 0 replies; 21+ messages in thread
From: tip-bot for Yong Zhang @ 2011-02-23 12:05 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, a.p.zijlstra, efault, yong.zhang0, tglx,
mingo
Commit-ID: 1747b21fecbfb63fbf6b9624e8b92707960d5a97
Gitweb: http://git.kernel.org/tip/1747b21fecbfb63fbf6b9624e8b92707960d5a97
Author: Yong Zhang <yong.zhang0@gmail.com>
AuthorDate: Sun, 20 Feb 2011 15:08:12 +0800
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 23 Feb 2011 11:33:58 +0100
sched, autogroup, sysctl: Use proc_dointvec_minmax() instead
sched_autogroup_enabled has min/max value, proc_dointvec_minmax() is
be used for this case.
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1298185696-4403-2-git-send-email-yong.zhang0@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/sysctl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index cb7c830..7b5eead 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -367,7 +367,7 @@ static struct ctl_table kern_table[] = {
.data = &sysctl_sched_autogroup_enabled,
.maxlen = sizeof(unsigned int),
.mode = 0644,
- .proc_handler = proc_dointvec,
+ .proc_handler = proc_dointvec_minmax,
.extra1 = &zero,
.extra2 = &one,
},
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/5] sched: hide cfs_rq of autogroup if autogroup is disabled
2011-02-20 7:08 [PATCH 0/5] autogroup runtime enable/disable tuning fix Yong Zhang
2011-02-20 7:08 ` [PATCH 1/5] sched, autogroup, sysctl: use proc_dointvec_minmax instead Yong Zhang
@ 2011-02-20 7:08 ` Yong Zhang
2011-02-20 7:08 ` [PATCH 3/5] sched, autogroup: stop going ahead " Yong Zhang
` (2 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Yong Zhang @ 2011-02-20 7:08 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Peter Zijlstra, Mike Galbraith
When autogroup is disabled, sched_debug will show
all of the cfs_rq of autogroup by name "cfs_rq[0]:"
Actually that doesn't make sense and not necessary
to be showed.
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
---
kernel/sched_debug.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/kernel/sched_debug.c b/kernel/sched_debug.c
index eb6cb8e..121e2a2 100644
--- a/kernel/sched_debug.c
+++ b/kernel/sched_debug.c
@@ -169,6 +169,12 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
struct sched_entity *last;
unsigned long flags;
+#ifdef CONFIG_SCHED_AUTOGROUP
+ int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
+
+ if (task_group_is_autogroup(cfs_rq->tg) && !enabled)
+ return;
+#endif
#ifdef CONFIG_FAIR_GROUP_SCHED
SEQ_printf(m, "\ncfs_rq[%d]:%s\n", cpu, task_group_path(cfs_rq->tg));
#else
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 3/5] sched, autogroup: stop going ahead if autogroup is disabled
2011-02-20 7:08 [PATCH 0/5] autogroup runtime enable/disable tuning fix Yong Zhang
2011-02-20 7:08 ` [PATCH 1/5] sched, autogroup, sysctl: use proc_dointvec_minmax instead Yong Zhang
2011-02-20 7:08 ` [PATCH 2/5] sched: hide cfs_rq of autogroup if autogroup is disabled Yong Zhang
@ 2011-02-20 7:08 ` Yong Zhang
2011-02-23 12:05 ` [tip:sched/core] sched, autogroup: Stop " tip-bot for Yong Zhang
2011-02-20 7:08 ` [PATCH 4/5] sched: refactor task_group() Yong Zhang
2011-02-20 7:08 ` [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix Yong Zhang
4 siblings, 1 reply; 21+ messages in thread
From: Yong Zhang @ 2011-02-20 7:08 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Peter Zijlstra, Mike Galbraith
when autogroup is disable from the beginning,
sched_autogroup_create_attach()
autogroup_move_group() <== 1
sched_move_task() <== 2
task_move_group_fair()
set_task_rq()
task_group()
autogroup_task_group()
We go the whole path without doing anything useful.
Then stop going further if autogroup is disabled.
But there will be a race window between 1 and 2, in which
sysctl_sched_autogroup_enabled is enabled. This issue
will be toke by following patch.
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
---
kernel/sched_autogroup.c | 4 ++++
kernel/sched_autogroup.h | 5 +++++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 9fb6562..137a096 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -161,11 +161,15 @@ autogroup_move_group(struct task_struct *p, struct autogroup *ag)
p->signal->autogroup = autogroup_kref_get(ag);
+ if (!ACCESS_ONCE(sysctl_sched_autogroup_enabled))
+ goto out;
+
t = p;
do {
sched_move_task(t);
} while_each_thread(p, t);
+out:
unlock_task_sighand(p, &flags);
autogroup_kref_put(prev);
}
diff --git a/kernel/sched_autogroup.h b/kernel/sched_autogroup.h
index 7b859ff..0557705 100644
--- a/kernel/sched_autogroup.h
+++ b/kernel/sched_autogroup.h
@@ -1,6 +1,11 @@
#ifdef CONFIG_SCHED_AUTOGROUP
struct autogroup {
+ /*
+ * reference doesn't mean how many thread attach to this
+ * autogroup now. It just stands for the number of task
+ * could use this autogroup.
+ */
struct kref kref;
struct task_group *tg;
struct rw_semaphore lock;
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [tip:sched/core] sched, autogroup: Stop going ahead if autogroup is disabled
2011-02-20 7:08 ` [PATCH 3/5] sched, autogroup: stop going ahead " Yong Zhang
@ 2011-02-23 12:05 ` tip-bot for Yong Zhang
0 siblings, 0 replies; 21+ messages in thread
From: tip-bot for Yong Zhang @ 2011-02-23 12:05 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, a.p.zijlstra, efault, yong.zhang0, tglx,
mingo
Commit-ID: 800d4d30c8f20bd728e5741a3b77c4859a613f7c
Gitweb: http://git.kernel.org/tip/800d4d30c8f20bd728e5741a3b77c4859a613f7c
Author: Yong Zhang <yong.zhang0@gmail.com>
AuthorDate: Sun, 20 Feb 2011 15:08:14 +0800
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 23 Feb 2011 11:33:59 +0100
sched, autogroup: Stop going ahead if autogroup is disabled
when autogroup is disable from the beginning,
sched_autogroup_create_attach()
autogroup_move_group() <== 1
sched_move_task() <== 2
task_move_group_fair()
set_task_rq()
task_group()
autogroup_task_group()
We go the whole path without doing anything useful.
Then stop going further if autogroup is disabled.
But there will be a race window between 1 and 2, in which
sysctl_sched_autogroup_enabled is enabled. This issue
will be toke by following patch.
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
LKML-Reference: <1298185696-4403-4-git-send-email-yong.zhang0@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/sched_autogroup.c | 4 ++++
kernel/sched_autogroup.h | 5 +++++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 9fb6562..137a096 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -161,11 +161,15 @@ autogroup_move_group(struct task_struct *p, struct autogroup *ag)
p->signal->autogroup = autogroup_kref_get(ag);
+ if (!ACCESS_ONCE(sysctl_sched_autogroup_enabled))
+ goto out;
+
t = p;
do {
sched_move_task(t);
} while_each_thread(p, t);
+out:
unlock_task_sighand(p, &flags);
autogroup_kref_put(prev);
}
diff --git a/kernel/sched_autogroup.h b/kernel/sched_autogroup.h
index 7b859ff..0557705 100644
--- a/kernel/sched_autogroup.h
+++ b/kernel/sched_autogroup.h
@@ -1,6 +1,11 @@
#ifdef CONFIG_SCHED_AUTOGROUP
struct autogroup {
+ /*
+ * reference doesn't mean how many thread attach to this
+ * autogroup now. It just stands for the number of task
+ * could use this autogroup.
+ */
struct kref kref;
struct task_group *tg;
struct rw_semaphore lock;
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/5] sched: refactor task_group()
2011-02-20 7:08 [PATCH 0/5] autogroup runtime enable/disable tuning fix Yong Zhang
` (2 preceding siblings ...)
2011-02-20 7:08 ` [PATCH 3/5] sched, autogroup: stop going ahead " Yong Zhang
@ 2011-02-20 7:08 ` Yong Zhang
2011-02-20 7:08 ` [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix Yong Zhang
4 siblings, 0 replies; 21+ messages in thread
From: Yong Zhang @ 2011-02-20 7:08 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Peter Zijlstra, Mike Galbraith
move cgroup-related task group finding to cgroup_task_group().
No function change.
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
---
kernel/sched.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/kernel/sched.c b/kernel/sched.c
index 18d38e4..5c74f79 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -601,7 +601,7 @@ static inline int cpu_of(struct rq *rq)
* holds that lock for each task it moves into the cgroup. Therefore
* by holding that lock, we pin the task to the current cgroup.
*/
-static inline struct task_group *task_group(struct task_struct *p)
+static inline struct task_group *cgroup_task_group(struct task_struct *p)
{
struct task_group *tg;
struct cgroup_subsys_state *css;
@@ -613,6 +613,14 @@ static inline struct task_group *task_group(struct task_struct *p)
lockdep_is_held(&task_rq(p)->lock));
tg = container_of(css, struct task_group, css);
+ return tg;
+}
+
+static inline struct task_group *task_group(struct task_struct *p)
+{
+ struct task_group *tg;
+
+ tg = cgroup_task_group(p);
return autogroup_task_group(p, tg);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix
2011-02-20 7:08 [PATCH 0/5] autogroup runtime enable/disable tuning fix Yong Zhang
` (3 preceding siblings ...)
2011-02-20 7:08 ` [PATCH 4/5] sched: refactor task_group() Yong Zhang
@ 2011-02-20 7:08 ` Yong Zhang
2011-02-20 13:09 ` Mike Galbraith
4 siblings, 1 reply; 21+ messages in thread
From: Yong Zhang @ 2011-02-20 7:08 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Peter Zijlstra, Mike Galbraith
Now enable/disable autogroup at runtime is just a flag,
it doesn't take effect on current process of the system.
when disable autogroup:
vanilla:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
1046 test1 20 0 25544 844 276 R 50.0 0.0 0:23.30 bash
1044 test2 20 0 25544 860 280 R 25.2 0.0 0:20.55 bash
1045 test2 20 0 25544 864 284 R 24.8 0.0 0:17.48 bash
patched:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
991 test1 20 0 25544 844 276 R 31.7 0.0 2:05.93 bash
992 test2 20 0 25544 848 276 R 31.7 0.0 1:16.84 bash
994 test2 20 0 25544 856 284 R 29.8 0.0 1:02.67 bash
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
---
include/linux/sched.h | 3 +++
kernel/sched_autogroup.c | 34 ++++++++++++++++++++++++++++++++++
kernel/sysctl.c | 2 +-
3 files changed, 38 insertions(+), 1 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d747f94..00791ce 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1949,6 +1949,9 @@ extern unsigned int sysctl_sched_compat_yield;
#ifdef CONFIG_SCHED_AUTOGROUP
extern unsigned int sysctl_sched_autogroup_enabled;
+extern int sysctl_sched_autogroup_handler(struct ctl_table *table, int write,
+ void __user *buffer, size_t *lenp,
+ loff_t *ppos);
extern void sched_autogroup_create_attach(struct task_struct *p);
extern void sched_autogroup_detach(struct task_struct *p);
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 137a096..b1ad946 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -211,6 +211,40 @@ static int __init setup_autogroup(char *str)
__setup("noautogroup", setup_autogroup);
+static DEFINE_MUTEX(autogroup_sysctl_mutex);
+
+int sysctl_sched_autogroup_handler(struct ctl_table *table, int write,
+ void __user *buffer, size_t *lenp,
+ loff_t *ppos)
+{
+ int old_value, ret;
+ struct task_struct *p, *n;
+
+ mutex_lock(&autogroup_sysctl_mutex);
+
+ old_value = sysctl_sched_autogroup_enabled;
+ ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+
+ if (ret || !write || (old_value == sysctl_sched_autogroup_enabled))
+ goto out_unlock;
+
+ read_lock_irq(&tasklist_lock);
+ rcu_read_lock();
+
+ do_each_thread(n, p) {
+ if (cgroup_task_group(p) == &root_task_group)
+ sched_move_task(p);
+ } while_each_thread(n, p);
+
+ rcu_read_unlock();
+ read_unlock_irq(&tasklist_lock);
+
+out_unlock:
+ mutex_unlock(&autogroup_sysctl_mutex);
+
+ return ret;
+}
+
#ifdef CONFIG_PROC_FS
int proc_sched_autogroup_set_nice(struct task_struct *p, int *nice)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 22e07ee..2b12c65 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -374,7 +374,7 @@ static struct ctl_table kern_table[] = {
.data = &sysctl_sched_autogroup_enabled,
.maxlen = sizeof(unsigned int),
.mode = 0644,
- .proc_handler = proc_dointvec_minmax,
+ .proc_handler = sysctl_sched_autogroup_handler,
.extra1 = &zero,
.extra2 = &one,
},
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix
2011-02-20 7:08 ` [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix Yong Zhang
@ 2011-02-20 13:09 ` Mike Galbraith
2011-02-20 14:10 ` Yong Zhang
0 siblings, 1 reply; 21+ messages in thread
From: Mike Galbraith @ 2011-02-20 13:09 UTC (permalink / raw)
To: Yong Zhang; +Cc: linux-kernel, Ingo Molnar, Peter Zijlstra
On Sun, 2011-02-20 at 15:08 +0800, Yong Zhang wrote:
> Now enable/disable autogroup at runtime is just a flag,
> it doesn't take effect on current process of the system.
Yup, they only move upon migration, which can take up to forever. Linus
didn't like the idea of an active on/off switch, so it got whacked.
-Mike
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix
2011-02-20 13:09 ` Mike Galbraith
@ 2011-02-20 14:10 ` Yong Zhang
2011-02-20 17:22 ` Mike Galbraith
0 siblings, 1 reply; 21+ messages in thread
From: Yong Zhang @ 2011-02-20 14:10 UTC (permalink / raw)
To: Mike Galbraith; +Cc: linux-kernel, Ingo Molnar, Peter Zijlstra
On Sun, Feb 20, 2011 at 02:09:39PM +0100, Mike Galbraith wrote:
> On Sun, 2011-02-20 at 15:08 +0800, Yong Zhang wrote:
> > Now enable/disable autogroup at runtime is just a flag,
> > it doesn't take effect on current process of the system.
>
> Yup, they only move upon migration, which can take up to forever.
Yeah. so on UP, the user can have autogroup forever or not, and
sched_autogroup_enable is a nop.
> Linus
> didn't like the idea of an active on/off switch, so it got whacked.
Seems I missed that very thread.
Apart from runtime enable/disable issue, I think patch1~3 could
still be applied beause they really fix some problem or bring
some improvement.
Thanks,
Yong
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix
2011-02-20 14:10 ` Yong Zhang
@ 2011-02-20 17:22 ` Mike Galbraith
2011-02-21 3:13 ` Mike Galbraith
2011-02-21 16:36 ` Peter Zijlstra
0 siblings, 2 replies; 21+ messages in thread
From: Mike Galbraith @ 2011-02-20 17:22 UTC (permalink / raw)
To: Yong Zhang; +Cc: linux-kernel, Ingo Molnar, Peter Zijlstra
On Sun, 2011-02-20 at 22:10 +0800, Yong Zhang wrote:
> On Sun, Feb 20, 2011 at 02:09:39PM +0100, Mike Galbraith wrote:
> > On Sun, 2011-02-20 at 15:08 +0800, Yong Zhang wrote:
> > > Now enable/disable autogroup at runtime is just a flag,
> > > it doesn't take effect on current process of the system.
> >
> > Yup, they only move upon migration, which can take up to forever.
>
> Yeah. so on UP, the user can have autogroup forever or not, and
> sched_autogroup_enable is a nop.
Yup, and that's the best argument for doing the active switch.
> Apart from runtime enable/disable issue, I think patch1~3 could
> still be applied beause they really fix some problem or bring
> some improvement.
Yeah.
Your whole series looked fine to me at a glance (sunday;), with the
exception of #2, that one is maybe iffy, depending on point of view.
It's iffy only in that the proc interface is there to show the group
(whether used or not, it's not about tasks really, just group), so the
user can easily twiddle it if he so desires, which has nada to do with
the group being active or not.
That said, I'd agree to whacking the proc interface wholesale in a
heartbeat. It's really a wart. If it were frequently used, it'd be
indicative of failure for the whole concept.
-Mike
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix
2011-02-20 17:22 ` Mike Galbraith
@ 2011-02-21 3:13 ` Mike Galbraith
2011-02-21 9:05 ` Yong Zhang
2011-02-21 16:36 ` Peter Zijlstra
1 sibling, 1 reply; 21+ messages in thread
From: Mike Galbraith @ 2011-02-21 3:13 UTC (permalink / raw)
To: Yong Zhang; +Cc: linux-kernel, Ingo Molnar, Peter Zijlstra
On Sun, 2011-02-20 at 18:22 +0100, Mike Galbraith wrote:
> On Sun, 2011-02-20 at 22:10 +0800, Yong Zhang wrote:
> > On Sun, Feb 20, 2011 at 02:09:39PM +0100, Mike Galbraith wrote:
> > > On Sun, 2011-02-20 at 15:08 +0800, Yong Zhang wrote:
> > > > Now enable/disable autogroup at runtime is just a flag,
> > > > it doesn't take effect on current process of the system.
> > >
> > > Yup, they only move upon migration, which can take up to forever.
> >
> > Yeah. so on UP, the user can have autogroup forever or not, and
> > sched_autogroup_enable is a nop.
>
> Yup, and that's the best argument for doing the active switch.
>
> > Apart from runtime enable/disable issue, I think patch1~3 could
> > still be applied beause they really fix some problem or bring
> > some improvement.
>
> Yeah.
>
> Your whole series looked fine to me at a glance (sunday;), with the
> exception of #2, that one is maybe iffy, depending on point of view.
Hm, my mind wandered off to different file somewhere along the line :)
-Mike
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix
2011-02-21 3:13 ` Mike Galbraith
@ 2011-02-21 9:05 ` Yong Zhang
0 siblings, 0 replies; 21+ messages in thread
From: Yong Zhang @ 2011-02-21 9:05 UTC (permalink / raw)
To: Mike Galbraith; +Cc: linux-kernel, Ingo Molnar, Peter Zijlstra
On Mon, Feb 21, 2011 at 11:13 AM, Mike Galbraith <efault@gmx.de> wrote:
>> Your whole series looked fine to me at a glance (sunday;), with the
>> exception of #2, that one is maybe iffy, depending on point of view.
>
> Hm, my mind wandered off to different file somewhere along the line :)
Never mind :)
If we relax the runtime autogroup switch, maybe patch-2 is a little
overkill, because after autogroup is disabled there still could have
some tasks staying on it's autogroup.
So if we wan to keep current autogroup switch behavior, I think
we can make autogroup_path() independent on
sysctl_sched_autogroup_enabled, thus user will not be confused
by lots of groups named "cfs_rq[*]:"
But if this patch is accepted, IMHO, patch-2 is ok.
Thanks,
Yong
--
Only stand for myself
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix
2011-02-20 17:22 ` Mike Galbraith
2011-02-21 3:13 ` Mike Galbraith
@ 2011-02-21 16:36 ` Peter Zijlstra
2011-02-21 21:12 ` Ingo Molnar
2011-02-22 3:13 ` Yong Zhang
1 sibling, 2 replies; 21+ messages in thread
From: Peter Zijlstra @ 2011-02-21 16:36 UTC (permalink / raw)
To: Mike Galbraith; +Cc: Yong Zhang, linux-kernel, Ingo Molnar
On Sun, 2011-02-20 at 18:22 +0100, Mike Galbraith wrote:
>
> Your whole series looked fine to me at a glance (sunday;), with the
> exception of #2, that one is maybe iffy, depending on point of view.
Right, so I've queued 1 and 3, 2 is like you said iffy, if there's an
autogroup it should display it.
As to 5, I really don't care, Linus did seem to care initially, don't
know how he feels about it now.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix
2011-02-21 16:36 ` Peter Zijlstra
@ 2011-02-21 21:12 ` Ingo Molnar
2011-02-22 3:13 ` Yong Zhang
1 sibling, 0 replies; 21+ messages in thread
From: Ingo Molnar @ 2011-02-21 21:12 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Mike Galbraith, Yong Zhang, linux-kernel, Linus Torvalds
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Sun, 2011-02-20 at 18:22 +0100, Mike Galbraith wrote:
> >
> > Your whole series looked fine to me at a glance (sunday;), with the
> > exception of #2, that one is maybe iffy, depending on point of view.
>
> Right, so I've queued 1 and 3, 2 is like you said iffy, if there's an
> autogroup it should display it.
>
> As to 5, I really don't care, Linus did seem to care initially, don't
> know how he feels about it now.
I've Cc:-ed him.
Since Yong Zhang has noticed the difference and has written and tested the patch we
should consider it i think, it's a QoI improvement. The knob is root-only, right?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix
2011-02-21 16:36 ` Peter Zijlstra
2011-02-21 21:12 ` Ingo Molnar
@ 2011-02-22 3:13 ` Yong Zhang
2011-02-22 9:56 ` Peter Zijlstra
1 sibling, 1 reply; 21+ messages in thread
From: Yong Zhang @ 2011-02-22 3:13 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Mike Galbraith, linux-kernel, Ingo Molnar
On Tue, Feb 22, 2011 at 12:36 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Sun, 2011-02-20 at 18:22 +0100, Mike Galbraith wrote:
>>
>> Your whole series looked fine to me at a glance (sunday;), with the
>> exception of #2, that one is maybe iffy, depending on point of view.
>
> Right, so I've queued 1 and 3, 2 is like you said iffy, if there's an
> autogroup it should display it.
So how about replacing patch-2 by below one for now?
---
From: Yong Zhang <yong.zhang0@gmail.com>
Subject: [PATCH] sched, autogroup: always show autogroup name in sched_debug
When autogroup is disabled, there will be lots of group
named "cfs_rq[cpu]:", thus will lead to confusion for
who read it.
And for now, autogroup runtime disable/enable will
not take effect immediately on current live processes,
so there maybe still have some processes attaching to
its autogroup.
So show autogroup name always. But for the root group,
its name will dance between "cfs_rq[cpu]:" and
"cfs_rq[cpu]:/autogroup-0" according to
sysctl_sched_autogroup_enabled.
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
---
kernel/sched_autogroup.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 64919dc..6114e3e 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -301,7 +301,7 @@ static inline int autogroup_path(struct task_group
*tg, char *buf, int buflen)
{
int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
- if (!enabled || !tg->autogroup)
+ if (!tg->autogroup || (!tg->autogroup->id && !enabled))
return 0;
return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
--
1.7.1
--
Only stand for myself
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 5/5] sched, autogroup: runtime enable/disable tuning fix
2011-02-22 3:13 ` Yong Zhang
@ 2011-02-22 9:56 ` Peter Zijlstra
2011-02-22 12:10 ` [PATCH V2] sched, autogroup: always show autogroup name in sched_debug Yong Zhang
0 siblings, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2011-02-22 9:56 UTC (permalink / raw)
To: Yong Zhang; +Cc: Mike Galbraith, linux-kernel, Ingo Molnar
On Tue, 2011-02-22 at 11:13 +0800, Yong Zhang wrote:
> So show autogroup name always. But for the root group,
> its name will dance between "cfs_rq[cpu]:" and
> "cfs_rq[cpu]:/autogroup-0" according to
> sysctl_sched_autogroup_enabled.
Would it make sense to force the root group to always be a !autogroup?
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V2] sched, autogroup: always show autogroup name in sched_debug
2011-02-22 9:56 ` Peter Zijlstra
@ 2011-02-22 12:10 ` Yong Zhang
2011-02-22 14:02 ` Mike Galbraith
0 siblings, 1 reply; 21+ messages in thread
From: Yong Zhang @ 2011-02-22 12:10 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Mike Galbraith, linux-kernel, Ingo Molnar
On Tue, Feb 22, 2011 at 10:56:06AM +0100, Peter Zijlstra wrote:
> On Tue, 2011-02-22 at 11:13 +0800, Yong Zhang wrote:
> > So show autogroup name always. But for the root group,
> > its name will dance between "cfs_rq[cpu]:" and
> > "cfs_rq[cpu]:/autogroup-0" according to
> > sysctl_sched_autogroup_enabled.
>
> Would it make sense to force the root group to always be a !autogroup?
Hmmm, I think it's ok.
---
From: Yong Zhang <yong.zhang0@gmail.com>
Subject: [PATCH] sched, autogroup: always show autogroup name in sched_debug
When autogroup is disabled, there will be lots of group
named "cfs_rq[cpu]:", thus will lead to confusion for
who read it.
And for now, autogroup runtime disable/enable will
not take effect immediately on current live processes,
so there maybe still have some processes attaching to
its autogroup.
So show autogroup name always. But for the root group,
its name will always be "cfs_rq[cpu]:/".
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
---
kernel/sched_autogroup.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 9fb6562..5b9a317 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -258,9 +258,7 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
#ifdef CONFIG_SCHED_DEBUG
static inline int autogroup_path(struct task_group *tg, char *buf, int buflen)
{
- int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
-
- if (!enabled || !tg->autogroup)
+ if (!tg->autogroup || !tg->autogroup->id)
return 0;
return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH V2] sched, autogroup: always show autogroup name in sched_debug
2011-02-22 12:10 ` [PATCH V2] sched, autogroup: always show autogroup name in sched_debug Yong Zhang
@ 2011-02-22 14:02 ` Mike Galbraith
2011-02-22 14:32 ` Yong Zhang
2011-02-23 12:06 ` [tip:sched/core] sched, autogroup: Stop claiming ownership of the root task group tip-bot for Mike Galbraith
0 siblings, 2 replies; 21+ messages in thread
From: Mike Galbraith @ 2011-02-22 14:02 UTC (permalink / raw)
To: Yong Zhang; +Cc: Peter Zijlstra, linux-kernel, Ingo Molnar
On Tue, 2011-02-22 at 20:10 +0800, Yong Zhang wrote:
> On Tue, Feb 22, 2011 at 10:56:06AM +0100, Peter Zijlstra wrote:
> > On Tue, 2011-02-22 at 11:13 +0800, Yong Zhang wrote:
> > > So show autogroup name always. But for the root group,
> > > its name will dance between "cfs_rq[cpu]:" and
> > > "cfs_rq[cpu]:/autogroup-0" according to
> > > sysctl_sched_autogroup_enabled.
> >
> > Would it make sense to force the root group to always be a !autogroup?
>
> Hmmm, I think it's ok.
Yup, it's not an autogroup, so let's stop claiming it.
sched, autogroup: stop claiming ownership of the root task group.
Disown it, and only display autogroup association if one exists.
Signed-off-by: Mike Galbraith <efault@gmx.de>
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 9fb6562..506faa0 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -12,7 +12,6 @@ static atomic_t autogroup_seq_nr;
static void __init autogroup_init(struct task_struct *init_task)
{
autogroup_default.tg = &root_task_group;
- root_task_group.autogroup = &autogroup_default;
kref_init(&autogroup_default.kref);
init_rwsem(&autogroup_default.lock);
init_task->signal->autogroup = &autogroup_default;
@@ -130,7 +129,7 @@ task_wants_autogroup(struct task_struct *p, struct task_group *tg)
static inline bool task_group_is_autogroup(struct task_group *tg)
{
- return tg != &root_task_group && tg->autogroup;
+ return !!tg->autogroup;
}
static inline struct task_group *
@@ -247,10 +246,14 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
{
struct autogroup *ag = autogroup_task_get(p);
+ if (!task_group_is_autogroup(ag->tg))
+ goto out;
+
down_read(&ag->lock);
seq_printf(m, "/autogroup-%ld nice %d\n", ag->id, ag->nice);
up_read(&ag->lock);
+out:
autogroup_kref_put(ag);
}
#endif /* CONFIG_PROC_FS */
@@ -258,9 +261,7 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
#ifdef CONFIG_SCHED_DEBUG
static inline int autogroup_path(struct task_group *tg, char *buf, int buflen)
{
- int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
-
- if (!enabled || !tg->autogroup)
+ if (!task_group_is_autogroup(tg))
return 0;
return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH V2] sched, autogroup: always show autogroup name in sched_debug
2011-02-22 14:02 ` Mike Galbraith
@ 2011-02-22 14:32 ` Yong Zhang
2011-02-23 12:06 ` [tip:sched/core] sched, autogroup: Stop claiming ownership of the root task group tip-bot for Mike Galbraith
1 sibling, 0 replies; 21+ messages in thread
From: Yong Zhang @ 2011-02-22 14:32 UTC (permalink / raw)
To: Mike Galbraith; +Cc: Peter Zijlstra, linux-kernel, Ingo Molnar
On Tue, Feb 22, 2011 at 03:02:00PM +0100, Mike Galbraith wrote:
> On Tue, 2011-02-22 at 20:10 +0800, Yong Zhang wrote:
> > On Tue, Feb 22, 2011 at 10:56:06AM +0100, Peter Zijlstra wrote:
> > > On Tue, 2011-02-22 at 11:13 +0800, Yong Zhang wrote:
> > > > So show autogroup name always. But for the root group,
> > > > its name will dance between "cfs_rq[cpu]:" and
> > > > "cfs_rq[cpu]:/autogroup-0" according to
> > > > sysctl_sched_autogroup_enabled.
> > >
> > > Would it make sense to force the root group to always be a !autogroup?
> >
> > Hmmm, I think it's ok.
>
> Yup, it's not an autogroup, so let's stop claiming it.
>
> sched, autogroup: stop claiming ownership of the root task group.
>
> Disown it, and only display autogroup association if one exists.
>
> Signed-off-by: Mike Galbraith <efault@gmx.de>
Ah, I like this one better :)
Actually I misunderstanded what Peter mean.
Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>
Thanks,
Yong
>
> diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
> index 9fb6562..506faa0 100644
> --- a/kernel/sched_autogroup.c
> +++ b/kernel/sched_autogroup.c
> @@ -12,7 +12,6 @@ static atomic_t autogroup_seq_nr;
> static void __init autogroup_init(struct task_struct *init_task)
> {
> autogroup_default.tg = &root_task_group;
> - root_task_group.autogroup = &autogroup_default;
> kref_init(&autogroup_default.kref);
> init_rwsem(&autogroup_default.lock);
> init_task->signal->autogroup = &autogroup_default;
> @@ -130,7 +129,7 @@ task_wants_autogroup(struct task_struct *p, struct task_group *tg)
>
> static inline bool task_group_is_autogroup(struct task_group *tg)
> {
> - return tg != &root_task_group && tg->autogroup;
> + return !!tg->autogroup;
> }
>
> static inline struct task_group *
> @@ -247,10 +246,14 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
> {
> struct autogroup *ag = autogroup_task_get(p);
>
> + if (!task_group_is_autogroup(ag->tg))
> + goto out;
> +
> down_read(&ag->lock);
> seq_printf(m, "/autogroup-%ld nice %d\n", ag->id, ag->nice);
> up_read(&ag->lock);
>
> +out:
> autogroup_kref_put(ag);
> }
> #endif /* CONFIG_PROC_FS */
> @@ -258,9 +261,7 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
> #ifdef CONFIG_SCHED_DEBUG
> static inline int autogroup_path(struct task_group *tg, char *buf, int buflen)
> {
> - int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
> -
> - if (!enabled || !tg->autogroup)
> + if (!task_group_is_autogroup(tg))
> return 0;
>
> return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
>
^ permalink raw reply [flat|nested] 21+ messages in thread* [tip:sched/core] sched, autogroup: Stop claiming ownership of the root task group
2011-02-22 14:02 ` Mike Galbraith
2011-02-22 14:32 ` Yong Zhang
@ 2011-02-23 12:06 ` tip-bot for Mike Galbraith
1 sibling, 0 replies; 21+ messages in thread
From: tip-bot for Mike Galbraith @ 2011-02-23 12:06 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, a.p.zijlstra, efault, yong.zhang0, tglx,
mingo
Commit-ID: 511f67a5997c4967c69a3961e2fc9f04d8d244ac
Gitweb: http://git.kernel.org/tip/511f67a5997c4967c69a3961e2fc9f04d8d244ac
Author: Mike Galbraith <efault@gmx.de>
AuthorDate: Tue, 22 Feb 2011 15:02:00 +0100
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 23 Feb 2011 11:34:03 +0100
sched, autogroup: Stop claiming ownership of the root task group
Disown it, and only display autogroup association if one exists.
Signed-off-by: Mike Galbraith <efault@gmx.de>
Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1298383320.8036.5.camel@marge.simson.net>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/sched_autogroup.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 137a096..5946ac5 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -12,7 +12,6 @@ static atomic_t autogroup_seq_nr;
static void __init autogroup_init(struct task_struct *init_task)
{
autogroup_default.tg = &root_task_group;
- root_task_group.autogroup = &autogroup_default;
kref_init(&autogroup_default.kref);
init_rwsem(&autogroup_default.lock);
init_task->signal->autogroup = &autogroup_default;
@@ -130,7 +129,7 @@ task_wants_autogroup(struct task_struct *p, struct task_group *tg)
static inline bool task_group_is_autogroup(struct task_group *tg)
{
- return tg != &root_task_group && tg->autogroup;
+ return !!tg->autogroup;
}
static inline struct task_group *
@@ -251,10 +250,14 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
{
struct autogroup *ag = autogroup_task_get(p);
+ if (!task_group_is_autogroup(ag->tg))
+ goto out;
+
down_read(&ag->lock);
seq_printf(m, "/autogroup-%ld nice %d\n", ag->id, ag->nice);
up_read(&ag->lock);
+out:
autogroup_kref_put(ag);
}
#endif /* CONFIG_PROC_FS */
@@ -262,9 +265,7 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
#ifdef CONFIG_SCHED_DEBUG
static inline int autogroup_path(struct task_group *tg, char *buf, int buflen)
{
- int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
-
- if (!enabled || !tg->autogroup)
+ if (!task_group_is_autogroup(tg))
return 0;
return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
^ permalink raw reply related [flat|nested] 21+ messages in thread